Use transaction to update data when joining challenge

[SYN-tactic]

Fixes #8337

Changes

This PR fixes an issue where the member count listed on a challenge's page (which is taken from the challenge's document in the database) can be incorrect.
This issue occurred because of two separate database calls that occur when a user joins a challenge. In particular, the challenge.addToUser(user) method and the challenge.save() methods are what conspired to cause this issue.

Since challenge.addToUser(user) actually modifies the user document as a separate database call, if this were to succeed and subsequently the challenge.save() were to fail, there would be a mismatch in the participants of the challenge (see this comment for reference).

In looking into this, I noticed that this issue was probably mitigated by PR #11502, where the author added a check to make sure the User document had actually been modified before proceeding with saving the challenge. Prior to that, it was likely that challenge.addToUser(user) would sometimes fail but subsequently challenge.save() succeeded, leading to the original issue.

My solution includes all the relevant database calls in a single transaction, so that if any query should fail then no part of the User or Challenge documents will be modified. I've maintained the current functionality as much as possible so as to minimize the impact this change has. The primary change that is happening here is to wrap several database calls into a a single mongoose session/transaction.

---

Habitica user UUID: d419ade0-fb52-4fec-890b-905a75a4fa1c

[SYN-tactic]

I'm a first time contributor so the workflow for tests hasn't run automatically after I merged in the upstream develop branch. Please can an admin click the button to start it?

[SYN-tactic]

Hi, I updated this branch with the latest from develop but it looks like it needs an admin to restart the workflow. Thanks!

[SYN-tactic]

Hi, I updated this branch with the latest from develop but it looks like it needs an admin to restart the workflow. Thanks!

Hello, updated this again!

[SabreCat]

Hi @SYN-tactic, and thanks for this! Gettin back into the swing of moving PRs through QA, here...

This is definitely a change we want to make, but I suspect the reimport of mongoose and starting of a fresh Mongoose session is overkill. Mongoose functions should already be available via the model, imported as Challenge. See for instance https://github.com/HabitRPG/habitica/blob/develop/website/server/controllers/api-v3/groups.js#L144-L147 where we fetch the extant session using the properties from the model db object.

If I'm mistaken or misunderstanding something here, though, volley back!

[SYN-tactic]

Hi @SYN-tactic, and thanks for this! Gettin back into the swing of moving PRs through QA, here...

This is definitely a change we want to make, but I suspect the reimport of mongoose and starting of a fresh Mongoose session is overkill. Mongoose functions should already be available via the model, imported as Challenge. See for instance https://github.com/HabitRPG/habitica/blob/develop/website/server/controllers/api-v3/groups.js#L144-L147 where we fetch the extant session using the properties from the model db object.

If I'm mistaken or misunderstanding something here, though, volley back!

Hey @SabreCat , thank you for your feedback!

That makes sense - what do you think of starting a session this way, instead?

 const session = await Challenge.startSession();
 session.startTransaction();

My approach needs a way to reference the session, so based on your input it sounds like I could just start a session this way. However, it's unclear to me whether a reference to an existing/active session already exists on the model that I could somehow retrieve? Let me know if this approach makes sense - if not happy to dig in to this a little more. Cheers!