[FIX] sale_operating_unit_sequence: resolve operating unit from its d… - #864
Merged
OCA-git-bot merged 1 commit intoJul 27, 2026
Conversation
OCA-git-bot
added
series:16.0
mod:sale_operating_unit_sequence
Member
Author
|
Hey @AaronHForgeFlow , kindly asking for a review :) |
sebinthomasbeo
approved these changes
Jul 24, 2026
sebinthomasst
approved these changes
Jul 24, 2026
parvathyvijayanbeo
approved these changes
Jul 24, 2026
AaronHForgeFlow
left a comment
AaronHForgeFlow
left a comment
Contributor
There was a problem hiding this comment.
LGTM just some suggestions
Comment on lines
+31
to
+35
| operating_unit_id = vals.get("operating_unit_id") | ||
| if not operating_unit_id: | ||
| operating_unit_id = self.default_get(["operating_unit_id"]).get( | ||
| "operating_unit_id" | ||
| ) |
Contributor
There was a problem hiding this comment.
Ok to cover the case when OU is explicitly False?
Suggested change
| operating_unit_id = vals.get("operating_unit_id") | |
| if not operating_unit_id: | |
| operating_unit_id = self.default_get(["operating_unit_id"]).get( | |
| "operating_unit_id" | |
| ) | |
| if "operating_unit_id" in vals: | |
| operating_unit_id = vals["operating_unit_id"] | |
| else: | |
| operating_unit_id = self.default_get(["operating_unit_id"]).get( | |
| "operating_unit_id" | |
| ) |
Member
Author
There was a problem hiding this comment.
Ok to cover the case when OU is explicitly False?
Important point actually. Good catch :)
|
|
||
| @api.model | ||
| def _get_sequence_operating_unit_id(self, vals): | ||
| """Resolve the operating unit to use for sequence generation. |
Contributor
There was a problem hiding this comment.
can you summarize this a bit? it is too much explanation for so little code
| user.has_group("operating_unit.group_multi_operating_unit"), | ||
| "Test user must not have the Multiple Operating Units permission", | ||
| ) | ||
|
|
Contributor
There was a problem hiding this comment.
avoid empty lines within methods
| } | ||
| ) | ||
| ) | ||
|
|
…efault when absent from create vals
operating_unit_id is hidden in the view for users without the Multiple
Operating Units permission, so the client never submits it and it is
absent from vals on create. The same is true for any programmatic/API
create that omits it. _update_sale_order_name only looked at
vals.get("operating_unit_id"), so in both cases it silently did nothing
and the operating unit's sale sequence was never applied, even though the
record still received that operating unit via its field default a moment
later (in base create(), which merges defaults after this hook runs).
Add _get_sequence_operating_unit_id(), which falls back to
self.default_get(["operating_unit_id"]) when vals has no operating unit,
so the sequence is generated consistently for UI and backend/API creates.
baguenth
force-pushed
the
16.0-fix-sale_operating_unit_sequence-ou-default-when-absent
branch
from
July 24, 2026 16:23
c7b8bd1 to
5c519d8
Compare
Member
Author
Thank you for the review :) Updated the code as per your suggestions. |
Member
Author
|
@AaronHForgeFlow Can we integrate? |
AaronHForgeFlow
approved these changes
Jul 27, 2026
Contributor
|
/ocabot merge patch |
Contributor
|
What a great day to merge this nice PR. Let's do it! |
Contributor
|
Congratulations, your PR was merged at 6670b00. Thanks a lot for contributing to OCA. ❤️ |
baguenth
deleted the
16.0-fix-sale_operating_unit_sequence-ou-default-when-absent
branch
July 27, 2026 15:40
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
6 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…efault when absent from create vals
operating_unit_id is hidden in the view for users without the Multiple Operating Units permission, so the client never submits it and it is absent from vals on create. The same is true for any programmatic/API create that omits it. _update_sale_order_name only looked at vals.get("operating_unit_id"), so in both cases it silently did nothing and the operating unit's sale sequence was never applied, even though the record still received that operating unit via its field default a moment later (in base create(), which merges defaults after this hook runs).
Add _get_sequence_operating_unit_id(), which falls back to self.default_get(["operating_unit_id"]) when vals has no operating unit, so the sequence is generated consistently for UI and backend/API creates.