Free group plan subscription from a party/guild no longer gets cancelled after leaving a guild - fixes #11055

[doublej89]

Fixes #11055

Changes

If a user attempts to leave a guild, the code now checks whether that user is a member of at least one other group (party/guild) with an active group plan. If the check succeeds, the user's free subscription will not get cancelled.

Previously, the buggy code would check if all of the user's groups had a group plan. So if just one of those groups did not have a group plan, the check would fail and the user's free subscription would get cancelled.

That doesn't happen anymore.

The fix was made by simply replacing the .every() with the .some() array method in the group membership checking function.

Tests have been added to ensure that when leaving a group a user doesn't have to worry about losing their free subscription as long as they are a member of at least one group plan.

An additional bug fix is in the code that handles cancelling group subscription for users. The code used to incorrectly check user membership in a group with a group plan using the query condition 'purchased.plan.dateTerminated': null. This would return not only groups that have a 'purchased.plan.dateTerminated' field with null value, but also groups that don't have a 'purchased.plan.dateTerminated' field.

To make sure the query only returns groups that have a 'purchased.plan.dateTerminated' field with null value, the query condition should be 'purchased.plan.dateTerminated': { $type: 'null' }. That was the fix.

---

UUID:
395c7c76-da35-4145-85dd-783bf753e3ea

[paglias]

Hi @doublej89 and thanks for the PR! Sorry for the delay here but during this week I'll make sure to review it

[paglias]

@doublej89 could you add some tests to make sure the functionality is working as intended? let me know if you need any help

[doublej89]

@paglias Sorry for the delay as well! I've added some tests. Hope it helps. Let me know if more needs to be done.

[Alys]

@doublej89 I've done some tests on my local install to compare the behaviour between develop and this PR and this is looking great! From everything I can see, you've fixed the bug described in #11055. I appreciate that you've also made the code improvements I suggested in that issue - thank you!
Below is a quick-and-dirty description of the tests that I did and the results. If anyone wants me to provide more details about the tests, just ask.

I have not done a code review; the staff will do that.

Party has G.Plan
User is also a member of two guilds with no G.Plan
Leave one guild - develop & PR 11228: keep subscription (correct)
Leave your last guild - develop: lose subscription (bug); PR 11228: keep subscription (correct - bug fixed)

Party has G.Plan
Guild has G.Plan
Leave party - develop & PR 11228: keep subscription (both correct)

Party has G.Plan
Guild has G.Plan
Leave guild - develop: lose subscription (bug); PR 11228: keep subscription (correct - bug fixed)

Guild has G.Plan
User is also a member of two guilds with no G.Plan
Leave a non-G.Plan guild - develop & PR 11228: keep subscription (both correct)
Leave your last guild without G.Plan - develop & PR 11228: keep subscription (both correct)
Leave party - develop & PR 11228: keep subscription (both correct)

Guild has G.Plan
User is also a member of two guilds with no G.Plan
Leave a non-G.Plan guild - develop & PR 11228: keep subscription (both correct)
Leave party - develop & PR 11228: keep subscription (both correct)
Leave your last guild without G.Plan - develop: lose subscription (bug); PR 11228: keep subscription (correct - bug fixed)

Guild has G.Plan
Second guild has G.Plan
Leave one guild - develop: lose subscription (bug); PR 11228: keep subscription (correct - bug fixed)

[paglias]

thanks @doublej89 the tests look good and thanks @Alys ! Just one last thing before we can merge this, could you undo the changes to package-lock.json? thanks!

[doublej89]

@paglias done. package-lock.json should now be the same as in the main repo

[doublej89]

@paglias hmm, coverage seems to have gone down a bit. Let me know if more tests need to be written

[paglias]

Hi @doublej89 ! Don't worry about the coverage, sometimes it's buggy. I'm currently on vacation so I'll merge this when I'm back on the 9th or 10th.

[SabreCat]

Thanks for this, @doublej89, and welcome to the ranks of the Blacksmiths at tier 1! This will go into a staging environment for a few days, and you should see it live on the production site around this time next week.

Note that subsequent contributor tiers take increasing amounts of effort from one to the next, so Blacksmith 2 will require multiple PRs or a larger PR to attain. But keep helping out and we'll express our gratitude accordingly!