Skip to content

[18.0][MIG] partner_archive_propagate#2361

Open
ntsirintanis wants to merge 9 commits into
OCA:18.0from
Therp:18.0-mig-partner_archive_propagate
Open

[18.0][MIG] partner_archive_propagate#2361
ntsirintanis wants to merge 9 commits into
OCA:18.0from
Therp:18.0-mig-partner_archive_propagate

Conversation

@ntsirintanis
Copy link
Copy Markdown
Contributor

Migration from 16.0

@OCA-git-bot OCA-git-bot added series:18.0 mod:partner_archive_propagate Module partner_archive_propagate labels May 22, 2026
Copy link
Copy Markdown

@vanesanetkia vanesanetkia left a comment

Choose a reason for hiding this comment

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

LGTM: Functional review

@ntsirintanis ntsirintanis force-pushed the 18.0-mig-partner_archive_propagate branch from 41037fc to ac105fc Compare May 26, 2026 12:50
@NL66278
Copy link
Copy Markdown
Contributor

NL66278 commented May 27, 2026

@ntsirintanis The extremely minor decrease in coverage for the project is due to these lines:

+         if "active" in vals:
+             if vals["active"] is False:
+                 archiving_partners = self.filtered(lambda self: self.active)
+             elif vals["active"] is True:
+                 unarchiving_partners = self.filtered(lambda self: not self.active)

So basically the elif never has a value that is not True at this point, because if there is a value for active it can only be True or False. So elif vals["active"] is True: can be replaced by just else:.

Copy link
Copy Markdown
Member

@Kiplangatdan Kiplangatdan left a comment

Choose a reason for hiding this comment

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

@ntsirintanis Tested locally, works as expected.

Other than the code-coverage tests mentioned by Ronald.
It looks good.

@NL66278
Copy link
Copy Markdown
Contributor

NL66278 commented May 28, 2026

Please add a note to the USAGE section of the README that using the standard Archive action is still available as before and that the user can select either the standard action without propagation, or the button with propagation. OK, make that part of the currently planned improvement to 16.0, and it will come along with that.

Copy link
Copy Markdown
Contributor

@BhaveshHeliconia BhaveshHeliconia left a comment

Choose a reason for hiding this comment

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

Code and functional review LGTM!

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

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants