Habitica: free Group Plan subscription from a Party should not be cancelled when leaving a Guild

[Alys]

If you have a free subscription because you are a member of a Party that has a Group Plan, leaving any Guild will cause your free subscription to be terminated. That should not happen. Your free subscription should remain for as long as you are a member of one or more Guilds/Party that still has an active Group Plan.

Whenever someone with a free subscription leaves a Guild or Party, there is code that is executed to check whether you're still in any other groups that have an active Group Plan (and if you are, your free subscription is not removed), but it's clearly not working for the case where the Group Plan is in a Party. It _might_ also not be working when the Group Plan is in a guild - that should be tested as part of the fix for this although I've seen no evidence that it's not working in that case. Below are pointers to parts of that code; the first one is in the api.leaveGroup route (/api/v3/groups/:groupId/leave):

https://github.com/HabitRPG/habitica/blob/ee20b1eea8d761f16ba6ba3aba715f3f89a2467f/website/server/controllers/api-v3/groups.js#L780-L783

https://github.com/HabitRPG/habitica/blob/ee20b1eea8d761f16ba6ba3aba715f3f89a2467f/website/server/models/user/methods.js#L392-L398

https://github.com/HabitRPG/habitica/blob/ee20b1eea8d761f16ba6ba3aba715f3f89a2467f/website/server/models/user/methods.js#L361-L372

https://github.com/HabitRPG/habitica/blob/ee20b1eea8d761f16ba6ba3aba715f3f89a2467f/website/server/models/user/methods.js#L42-L48

I've replicated this bug three times. I'm marking it as important priority for a fast fix.

If any contributors want to work on this, you should definitely post here first to say you're interested and wait for a response before you start. That's a good idea for any issue but it's especially important for this one because the staff might decide to fix it themselves - asking first will save your time if the staff haven't yet updated this issue.
In Using Your Local Install to Modify Habitica%27s Website and API, you can find information about how to add a free subscription from a Group Plan to any user, and how to add a Group Plan to a Guild or Party.

Note that when testing this bug or the code that fixes it, it's important that your test user isn't in any Guilds that still have a Group Plan - if it is, that will likely ensure that your test user keeps its free subscription (that's what's supposed to happen).

_As a note for myself: After the fix for this goes live, the emails that are sent when your free Group Plan subscription is removed for any reason should be tested (either I'll test them myself or make a note somewhere for others to test them). The reason is that at the time you are affected by this bug, you receive an email saying that the leader of the Guild you just left has cancelled the Group Plan in that guild, which is clearly not correct in this situation. There might be other bugs that send you the wrong email when you lose your free subscription for legitimate reasons._

[doublej89]

Hi, I've also managed to replicate this issue, for three test users. Each time I make one of them leave a guild, while being a member of a party with a group plan, his free subscription gets cancelled. Interestingly, their respective "users" document in the mongodb database remains unaltered after the bug takes effect (or so it seems).

So, if the staff isn't already working on it, I'd like to take a shot at fixing this bug.

[Alys]

@doublej89 Thank you for offering to work on this!

There was a related bug that I was talking to the staff about a little while ago and I want to refresh my memory about its details in case any of them are important or helpful for you to know, or in case it means this issue is no longer suitable for being worked on. Sorry - I'd forgotten about the existence of this issue when I was investigating the other bug. I'll post again on this issue when I get home from work in several hours.

In the meantime though, regarding this:
"Interestingly, their respective 'users' document in the mongodb database remains unaltered (or so it seems)."
It definitely should change if the subscription is cancelled so I'm not sure what's happening there. The subscription details (free or otherwise) are stored in the purchased.plan object in the user document. When a subscription is cancelled, the dateTerminated in that object is adjusted. For a non-cancelled subscription, dateTerminated is null; a subscription that's been cancelled has a dateTerminated with a date in the future, and once that date is reached, the user's next cron will delete most of the data from the purchased.plan object.

[Alys]

@doublej89 I've refreshed my memory about the related bug but I'm just checking with the staff about something. Most likely I or one of the staff will give you an update tomorrow or the next day.

[paglias]

Hi @doublej89 as @Alys mentioned it would be useful if you could clarify what you meant / what you saw happening with

"Interestingly, their respective 'users' document in the mongodb database remains unaltered (or so it seems)."

[doublej89]

@Alys Ah sorry, you're right, the 'dateTerminated' property in the 'purchased.plan' object in the user document does get updated after the cancellation. Maybe I was expecting the entire 'purchased.plan' object to become empty - my bad!

Anyway, here's an example of the object for a test user before and after the bug takes effect.

Before:

"plan" : {
                        "owner" : "c6bba6bb-9815-462e-8202-ac3d1c2a0ca7",
                        "dateCreated" : ISODate("2019-06-25T01:00:00Z"),
                        "dateUpdated" : ISODate("2019-06-25T01:00:00Z"),
                        "dateTerminated" : null,
                        "gemsBought" : 0,
                        "extraMonths" : 0,
                        "quantity" : 1,
                        "mysteryItems" : [ ],
                        "consecutive" : {
                                "count" : 1,
                                "offset" : 0,
                                "gemCapExtra" : 0,
                                "trinkets" : 0
                        },
                        "lastBillingDate" : null,
                        "paymentMethod" : "Group Plan",
                        "customerId" : "group-plan",
                        "planId" : "group_plan_auto",
                        "nextPaymentProcessing" : null,
                        "nextBillingDate" : null,
                        "additionalData" : null
                }

After:

"plan" : {
                        "owner" : "c6bba6bb-9815-462e-8202-ac3d1c2a0ca7",
                        "dateCreated" : ISODate("2019-06-25T01:00:00Z"),
                        "dateUpdated" : ISODate("2019-06-25T01:00:00Z"),
                        "dateTerminated" : ISODate("2019-06-13T18:00:00Z"),
                        "gemsBought" : 0,
                        "extraMonths" : 0,
                        "quantity" : 1,
                        "mysteryItems" : [ ],
                        "consecutive" : {
                                "count" : 1,
                                "offset" : 0,
                                "gemCapExtra" : 0,
                                "trinkets" : 0
                        },
                        "lastBillingDate" : null,
                        "paymentMethod" : "Group Plan",
                        "customerId" : "group-plan",
                        "planId" : "group_plan_auto",
                        "nextPaymentProcessing" : null,
                        "nextBillingDate" : null,
                        "additionalData" : null
                }

So it seems something is causing the free subscription to be cancelled after a user (while being in a party with a group plan) leaves a guild. If I get the green light, I'll be happy to look into the problem further and try tackling it. I'll wait for your update

[Alys]

@doublej89 Thanks for waiting! Please go ahead whenever you wish.

As part of the fix for this issue, there should be test(s) to ensure that the bug won't occur again. Using Your Local Install to Modify Habitica%27s Website and API has information about writing tests but ask here or in your PR if you have questions.

Below is what I've observed about code that's likely to be related to this issue. First though, in case you're not aware, a Group Plan is set up using data that's similar in several ways to a user's subscripton data. For example, a Group Plan has a purchased.plan.dateTerminated value that's null if the Group Plan is active, but that has a date value if the Plan has been cancelled. That "Using..." wiki page also has details about Group Plan data.

---

This line has a problem:
https://github.com/HabitRPG/habitica/blob/3a339a4a09de58f37eb7772b023cab9583f97d9b/website/server/libs/payments/groupPayments.js#L220
It's supposed to find all of the user's groups (guilds, party) that have an active Group Plan (i.e., a Group Plan that doesn't have a value for purchased.plan.dateTerminated). However null in a mongodb search matches both documents that have the value null for that field and documents that don't have that field at all. Thus that line will actually find groups that don't have a Group Plan as well as groups that have one.

The search should count only groups where dateTerminated exists and is null. The correct way to express that is with 'purchased.plan.dateTerminated': { $type: 10 } ($type operator).

---

The combination of the index and splice code below is wrong.
https://github.com/HabitRPG/habitica/blob/3a339a4a09de58f37eb7772b023cab9583f97d9b/website/server/libs/payments/groupPayments.js#L213-L214
There are circumstances where group._id is not in the user's list of groups, which means index is -1
That causes splice to incorrectly remove an unrelated id from the userGroups array (splice: "If negative, it will begin that many elements from the end of the array").
Splice should be run only if index >= 0

---

The type section of the mongodb search isn't necessary (this isn't a bug but it's unneeded clutter in the code and might as well be cleaned up).
https://github.com/HabitRPG/habitica/blob/3a339a4a09de58f37eb7772b023cab9583f97d9b/website/server/libs/payments/groupPayments.js#L217

---

So, that's what I observed when I was looking through the code a while back. I'm not sure how much of it is relevant to this issue but fixing those problems might be a good start. You're most welcome to ask questions if anything I've written isn't clear!