Skip to content

[16.0][IMP] membership_delegated_partner: allow multiple delegated members per invoice#207

Open
flaenen wants to merge 1 commit intoOCA:16.0from
flaenen:16.0-membership_delegated_partner_multi
Open

[16.0][IMP] membership_delegated_partner: allow multiple delegated members per invoice#207
flaenen wants to merge 1 commit intoOCA:16.0from
flaenen:16.0-membership_delegated_partner_multi

Conversation

@flaenen
Copy link
Copy Markdown

@flaenen flaenen commented Apr 28, 2025

It is now possible to define a delegate member per invoice line (in addition to the one defined on the invoice). A customer can therefore be invoiced for the memberships of several partners.

A delegated member defined on an invoice line has priority over the delegated member defined globally on the invoice.

Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Did you see #151 ? There were some issues spotted there with this approach. (fyi @gdgellatly )

In any case, I'm missing test coverage here

@flaenen
Copy link
Copy Markdown
Author

flaenen commented Apr 30, 2025

Did you see #151 ? There were some issues spotted there with this approach. (fyi @gdgellatly )

In any case, I'm missing test coverage here

I'll take a look at this PR, thank you.

@gdgellatly
Copy link
Copy Markdown

So in #151 I have passed the torch on that and I have not been involved in a long time, can't quite remember who to, but they are doing the work. 151 has actually been in production for years now for the OCA and IMO should eventually supersede membership_delegated_partner as the preferred module.

The reason for a separate module was technical at the time, something to do with computed fields in the upstream module, I don't really remember. That issue may no longer exist in newer versions.

Given the OCA's reliance on the feature, I imagine this is something that the relevant working group will want to be involved in.

Copy link
Copy Markdown
Member

@huguesdk huguesdk left a comment

Choose a reason for hiding this comment

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

i think this approach is much simpler than #151 which introduces a separate and incompatible module.

lgtm.

@gdgellatly
Copy link
Copy Markdown

gdgellatly commented Jul 12, 2025

i think this approach is much simpler than #151 which introduces a separate and incompatible module.

lgtm.

Sure, recent tendencies for people to just do what they want and not worry about anyone who depends on the existing behaviour, but at the time you would never dream of disrupting an existing module and its existing dependencies. That is why it was seperate after discussion with maintainer

@github-actions
Copy link
Copy Markdown

There hasn't been any activity on this pull request in the past 4 months, so it has been marked as stale and it will be closed automatically if no further activity occurs in the next 30 days.
If you want this PR to never become stale, please ask a PSC member to apply the "no stale" label.

@github-actions github-actions bot added the stale PR/Issue without recent activity, it'll be soon closed automatically. label Dec 21, 2025
@chienandalu chienandalu added no stale Use this label to prevent the automated stale action from closing this PR/Issue. needs review and removed stale PR/Issue without recent activity, it'll be soon closed automatically. question labels Jan 12, 2026
Copy link
Copy Markdown
Member

@chienandalu chienandalu left a comment

Choose a reason for hiding this comment

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

Reviewing you approach I agree with @gdgellatly that it would be compatible with both use cases. And this would not introduce incompatibility with the existing base, I think.

In any case, the extra test cases for membership from lines are very much needed, IMO. Maybe you could borrow them from #151

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

Labels

needs review no stale Use this label to prevent the automated stale action from closing this PR/Issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants