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

[17.0][MIG] sale_partner_address_restrict #3574

Merged

Conversation

JordiMForgeFlow
Copy link
Contributor

@JordiMForgeFlow JordiMForgeFlow commented Feb 7, 2025

Standard migration moving the module from partner-contact to sale-workflow

Includes improvements from #3570

Improvements:

  • Allow to configure the restriction activation (per company)
  • Add a domain computed field

@JordiMForgeFlow JordiMForgeFlow force-pushed the 17.0-mig-sale_partner_address_restrict branch from 7c21efb to bab1184 Compare February 7, 2025 11:58
@JordiMForgeFlow JordiMForgeFlow force-pushed the 17.0-mig-sale_partner_address_restrict branch from bab1184 to d204021 Compare February 7, 2025 12:10
@JordiMForgeFlow
Copy link
Contributor Author

@rousseldenis ready to review :)

Copy link

@AlexPForgeFlow AlexPForgeFlow left a comment

Choose a reason for hiding this comment

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

Functional and code review 👍

@AlexPForgeFlow
Copy link

@JordiMForgeFlow as an improvement, could we consider to restrict also not confirmed sales orders if setting it's enabled, or maybe is it too much restrictive?

@JordiMForgeFlow
Copy link
Contributor Author

@AlexPForgeFlow the restriction is already covering non confirmed orders with the constrain

@AlexPForgeFlow
Copy link

@JordiMForgeFlow The constrain does not cover that if we have a draft sale order created on the system, and then we enable the setting, not being able to confirm it. I have been able to confirm sale order in that scenario.

@JordiMForgeFlow
Copy link
Contributor Author

@AlexPForgeFlow ah I see. I don't think we should bother about previously created SOs. If they are updated then they will get the error.

Copy link
Contributor

@AaronHForgeFlow AaronHForgeFlow left a comment

Choose a reason for hiding this comment

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

Code review LGTM!

@rousseldenis
Copy link
Contributor

/ocabot migration sale_partner_address_restrict

@rousseldenis
Copy link
Contributor

/ocabot merge nobump

@OCA-git-bot
Copy link
Contributor

Hey, thanks for contributing! Proceeding to merge this for you.
Prepared branch 17.0-ocabot-merge-pr-3574-by-rousseldenis-bump-nobump, awaiting test results.

@OCA-git-bot OCA-git-bot merged commit b51f086 into OCA:17.0 Feb 10, 2025
7 checks passed
@OCA-git-bot
Copy link
Contributor

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

@MiquelRForgeFlow MiquelRForgeFlow deleted the 17.0-mig-sale_partner_address_restrict branch February 10, 2025 09:49
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.

7 participants