Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[16.0][ADD] edi_utm_oca #102

Merged
merged 1 commit into from
Sep 12, 2024
Merged

Conversation

chaule97
Copy link
Contributor

@chaule97 chaule97 commented Jul 29, 2024

  • this module edi_utm_oca automatically sets the EDI source on models that have a source_id field when they are created.

@chaule97 chaule97 force-pushed the 16.0-add_edi_sale_utm_oca branch 4 times, most recently from d48eb79 to 8f08d7c Compare July 30, 2024 04:51
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As reported on the base PR I don't think we need yet another source discriminator.
We already have endpoint and storage as sources + the types, what else do we need?
Plus, we can leverage UTM for a more general classification... I fail to understand what's the whole point of this.

@chaule97 chaule97 force-pushed the 16.0-add_edi_sale_utm_oca branch 6 times, most recently from 93b238a to 8905e1e Compare August 29, 2024 08:54
@chaule97 chaule97 requested a review from simahawk August 30, 2024 02:52
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also this module should be generic and not related to sales only

@chaule97 chaule97 changed the title [16.0][ADD] edi_sale_utm_oca [16.0][ADD] edi_utm_oca Sep 4, 2024
@chaule97 chaule97 force-pushed the 16.0-add_edi_sale_utm_oca branch 2 times, most recently from 48fa939 to b1a2508 Compare September 10, 2024 02:35
Copy link
Contributor

@simahawk simahawk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Minor remarks

edi_utm_oca/models/edi_exchange_consumer_mixin.py Outdated Show resolved Hide resolved
edi_utm_oca/readme/CONTRIBUTORS.rst Show resolved Hide resolved
@simahawk
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

On my way to merge this fine PR!
Prepared branch 16.0-ocabot-merge-pr-102-by-simahawk-bump-nobump, awaiting test results.

@OCA-git-bot
Copy link
Contributor

This PR has the approved label and has been created more than 5 days ago. It should therefore be ready to merge by a maintainer (or a PSC member if the concerned addon has no declared maintainer). 🤖

@OCA-git-bot OCA-git-bot merged commit fa044de into OCA:16.0 Sep 12, 2024
7 checks passed
@OCA-git-bot
Copy link
Contributor

Congratulations, your PR was merged at 99ad214. Thanks a lot for contributing to OCA. ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants