Challenge member count can be wrong

[Alys]

The member count listed on a challenge's page (which is taken from the challenge's document in the database) can be incorrect.

For example, for challenge 4d596684-ad74-416e-9abb-8195638247ea created on 2016-11-01, the memberCount in the document is 58, but a search for all players participating in the challenge shows only 53

This has also been noticed in other challenges (e.g., ccba7258-bac9-41e2-b9b5-e23ff0a50bf9, b2f1dff2-d5e0-4589-8100-5fcd0049d035, 4eb1f160-29bf-4036-bc0f-9d77bcb791e3).

I'm marking this as for suggestion-discussion so that we can discuss whether we should change the code to make updating the memberCount more reliable or do away with the memberCount attribute and take the count from the members retrieved.

---

With PR #12335 the Habitica codebase has been updated to support MongoDB 4.2 and transactions making it possible to fix this issue.

Transactions allow operations on multiple documents to be executed ensuring that either all of them are executed correctly or none, making it possible to fix this issue.

An example on using transactions can be found at https://mongoosejs.com/docs/transactions.html while more info at https://docs.mongodb.com/manual/core/transactions/. If you want to work on this issue and have any question please leave a comment!

---

EDIT 2020-11-20: I've removed some old comments that are no longer relevant, and added strike-through to some others that are also not as relevant for the new desired fix.
The desired fix is described in this comment: #8337 (comment)

[Alys]

There's two separate database update commands when a user joins a Challenge and if one fails the count will be different than the actual number of participants.

[SabreCat]

After a bit of staff discussion, I think the preferred solution here would be to obviate the need for a stored computed value by having the API return a count of Challenge participants at time of requesting Challenge data. We'd need to give careful consideration to performance especially on pages with lists of Challenges, but that's the way that will definitively solve this.

[Alys]

I know I was the person to log this issue, and I know there's already been staff discussion about it so feel free to completely ignore this disagreement if you wish 😄 but I'm wondering...
If you're concerned about performance (which certainly seems like a good concern), is it actually worth fixing this now? I don't see reports of the bug often so I don't think it's too disruptive. Once we're able to use database transactions, the current member count process could then be modified to use a transaction and so it would become reliable.

But it's just an idea. :) If any contributor wants to work on this, feel free to take it up since it's marked as help wanted!

[SabreCat]

It's a good point that it's not a high priority. If we were to tackle one of these stored-computed issues pre transactions, Guild/Party member counts would probably be more important as people run into that a lot more often!

EDIT: Based on that, I'm going to knock the priority tag down to minor, if that makes sense!

[veeeeeee]

May be related to #12286 and potentially solved in a similar fashion. This ticket will remain on hold until we can upgrade to a version that allows transactions, which will allow us to make a complete fix.

[paglias]

With PR #12335 the Habitica codebase has been updated to support MongoDB 4.2 and transactions making it possible to fix this issue.

Transactions allow operations on multiple documents to be executed ensuring that either all of them are executed correctly or none, making it possible to fix this issue.

An example on using transactions can be found at https://mongoosejs.com/docs/transactions.html while more info at https://docs.mongodb.com/manual/core/transactions/. If you want to work on this issue and have any question please leave a comment!

[SYN-tactic]

Hello 👋 - I'd love to help out with this issue if it's still needed. Am I correct in surmising that this line has the two database update commands that should be included in the same transaction?

Line 350 in website/server/controllers/api-v3/challenges.js

const results = await Promise.all([challenge.syncTasksToUser(user), challenge.save()]);

[CuriousMagpie]

@SYN-tactic If you're willing to tackle this, you have our full permission to do so! Thank you!

[SYN-tactic]

Awesome, thanks @CuriousMagpie - I'm on it!