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

fix(network-pass): handle multiple sync'd membership plans #80

Closed
wants to merge 1 commit into from

Conversation

adekbadek
Copy link
Member

@adekbadek adekbadek commented Mar 1, 2024

All Submissions:

Changes proposed in this Pull Request:

The network pass feature (#40) was written in a way to handle only one synchronised membership plan per site. If more than one membership plan had the same network pass id, only one of these would get the sync treatment. This PR fixed it, so multiple plans can have the same network pass on a site.

How to test the changes in this Pull Request:

  1. Set up synchronised memberships as described in Feat/network pass #40, but with a twist – assign the same Network ID to two membership plans on one of the sites
  2. Test the feature as described in Feat/network pass #40 – create a membership on one site, observe the memberships are set up for linked membership plans
  3. Verify that the site with two synchronised membership plans did grant the test user both memberships

Other information:

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Have you written new tests for your changes, as applicable?
  • Have you successfully ran tests with your changes locally?

@leogermani
Copy link
Contributor

leogermani commented Mar 2, 2024

Does it make sense for more than one plan to share the same network id though? I'm not in the loop of the requirements, but thinking about how this feature was created, I would go the other way and add a check when you save the membership plan to make sure you don't allow two plans to have the same network id.

The "network id" was meant to be the ID of this particular plan in the Network, and not "the ID of the network".

What's the use case for having one membership plan in one site mapping to more than one plan in another site? if a user becomes a member in one plan in one site it would become a member in more than one plan in the other? I think the relationship should always be 1 plan mapping to 1 plan. If you have more than one plan, each plan will have a different Network ID.

Feel free to ignore this comment if it does make sense, just wanted to share what was the original intention there

Copy link
Contributor

@leogermani leogermani 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 we should close this PR in favor of #108

@leogermani
Copy link
Contributor

Closed in favor of #108

@leogermani leogermani closed this Jul 30, 2024
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.

2 participants