[IMP] cooperator: Refactor cooperative.membership preparation code #101
Conversation
This file contains 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
huguesdk
requested changes
Dec 1, 2023
There was a problem hiding this comment.
great refactoring! thanks @carmenbianca! as this is critical code, it’s important to cover all the cases with tests and run them first without the changes, to ensure that the current (valid) behaviors still work as expected.
carmenbianca
force-pushed
the
16.0-refactor-partner-creation
branch
2 times, most recently
from
efd4a26 to
e820f17
Compare
carmenbianca
commented
Dec 14, 2023
| @@ -154,9 +89,6 @@ def set_cooperator_effective(self, effective_date): | |||
|
|
|||
| self._send_certificate_mail(certificate_email_template, sub_reg_line) | |||
There was a problem hiding this comment.
I'm a bit confused by this. Does this assume that the for-loop has one item?
huguesdk
approved these changes
Jan 19, 2024
There were two places where there was broadly the same workflow; set expected values on a partner's cooperative.membership after giving them a membership. That code is now in one place, and does the following as per the docstring: - ``member`` is set to True and ``old_member`` is set to False. - A cooperator register number is assigned if one did not yet exist. - A user is created if one did not yet exist. I also wrote a convenience method get_create_cooperative_membership in res.partner. Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
…dress Using the old method, it was possible (but unlikely) that a different partner's user was affected by changes on the partner. So instead of matching on the login address, match on the partner_id field. As a result of this, the code had to be adjusted to handle multiple user records. Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
Signed-off-by: Carmen Bianca BAKKER <carmen@coopiteasy.be>
carmenbianca
force-pushed
the
16.0-refactor-partner-creation
branch
from
e820f17 to
acd1472
Compare
|
/ocabot merge patch |
|
Hey, thanks for contributing! Proceeding to merge this for you. |
|
Congratulations, your PR was merged at 36cbd02. 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
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.
There were two places where there was broadly the same workflow; set expected values on a partner's cooperative.membership after giving them a membership.
That code is now in one place, and does the following as per the docstring:
memberis set to True andold_memberis set to False.This desperately needs tests.
I also wrote an optional convenience functionality into res.partner's get_cooperative_membership: create one if it doesn't yet exist.
Internal task: https://gestion.coopiteasy.be/web#view_type=form&model=project.task&id=9652&active_id=9652&menu_id=