Fix/con/898 ensure exclusive acceptance of mentors

[ericbolikowski]

What Github issue does this PR relate to? Insert link.

898

This PR fixes bug #898 which allowed mentees in special cases to have two simultaneously active/accepted Mentorship Matches:

The case could happen when for example:

1. Eric sends mentorship requests to two mentors, Anil and Kate
2. Two Mentorship Matches are created, both with state Applied
3. Anil and Kate access the CON Applications page at the same time
4. Anil accepts the mentorship
5. Nestjs updates the two Mentorship Mathches:
   • The Eric+Anil Mentorship Match becomes Accepted
   • The Eric+Kate Mentorship Match becomes Invalidated/Cancelled
6. Kate accepts the mentorship
7. The Eric+Kate Mentorship Match becomes Accepted

The fix was obvious: in the code that handles the "accept mentorship" mutation, only change the Mentorship Match state into Accepted if the current state is Applied.

What should the reviewer know?

The backend issue is solved, and we likely won't get into a state of one mentee + two active mentorships. But an unpleasant UX issue came up:

If the above steps happened again, but with the fix in this PR, Kate would in step 6 be prevented from accepting the mentorship request. Nestjs would return an error handled by the CON frontend in the catch (error) { clause you'll see. I've implemented a crude alert() shown to the user and then the page refreshes.

That scenario is illustrated in this Loom: https://www.loom.com/share/47fb8764f8d14c619092a3c8a6775b6d?sid=1dd21bcc-d13f-4020-a133-7a91d5bbb265

This leads to the loss of the acceptance message that Kate wrote - which is annoying, but the message isn't relevant any longer since Eric has a mentorship with Anil and isn't supposed to have one with Kate.

The question is: are there any easy ways to improve this UX?

Summary by CodeRabbit

• New Features

  • Introduced periodic data refresh for mentorship matches every 60 seconds in applications.

• Bug Fixes

  • Added validation to ensure mentorship matches are in "APPLIED" state before acceptance.

• Improvements

  • Enhanced error handling in the ConfirmMentorship component with new alert messages and page reloads.
  • Removed unnecessary console log statements in the Company Profile Editables component.

• Refactor

  • Added new logic to track changes in mentorship match statuses between renders.

[coderabbitai[bot]]

Walkthrough

This update introduces a series of changes intended to enhance the functionality and user experience of the mentorship matching and application pages within the Redi-Connect platform. Key highlights include the addition of status checks within mentorship match acceptance, improved error handling in the confirmation component, and a periodic data refresh interval for fetching mentorship matches, along with a new logic to track status changes across renders.

Changes

File Path	Change Summary
apps/nestjs-api/src/con-mentorship-matches/...	Added check to ensure mentorship match is in "APPLIED" state before accepting it.
apps/redi-connect/src/components/organisms/...	Added new alert message and page reload in the error handling section of the ConfirmMentorship component.
apps/redi-connect/src/pages/app/applications/...	Enhanced Applications with periodic data refetch and status change tracking logic.
apps/redi-connect/src/pages/app/applications/...	Modified ApplicationsFilterContext for periodic data refetch every 60 seconds.
apps/redi-talent-pool/src/components/organisms/...	Removed console.log statement related to myData in EditableNamePhotoLocation.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant ConfirmMentorship
    participant Applications
    participant API

    User->>ConfirmMentorship: Accept Mentorship
    ConfirmMentorship->>API: POST /acceptMentorship
    API-->>ConfirmMentorship: Success/Error Response
    ConfirmMentorship-->>User: Display Alert

    User->>Applications: Open Applications Page
    Applications->>API: GET /mentorshipMatches { refetchInterval: 60s }
    API-->>Applications: Mentorship Matches Data
    Applications->>User: Display Matches
    loop Every 60s
        Applications->>API: Refetch /mentorshipMatches
        API-->>Applications: Updated Matches Data
        Applications->>User: Update Display if data changed
    end

Poem

Amid the code and data flow,
The mentorship grows and starts to glow.
With each commit, a small delight,
Refreshed and ready, shining bright.
Through errors handled, alerts in place,
Our connections strengthen, face to face. 🌟

---

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share

- [X](https://twitter.com/intent/tweet?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A&url=https%3A//coderabbit.ai) - [Mastodon](https://mastodon.social/share?text=I%20just%20used%20%40coderabbitai%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20the%20proprietary%20code.%20Check%20it%20out%3A%20https%3A%2F%2Fcoderabbit.ai) - [Reddit](https://www.reddit.com/submit?title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&text=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code.%20Check%20it%20out%3A%20https%3A//coderabbit.ai) - [LinkedIn](https://www.linkedin.com/sharing/share-offsite/?url=https%3A%2F%2Fcoderabbit.ai&mini=true&title=Great%20tool%20for%20code%20review%20-%20CodeRabbit&summary=I%20just%20used%20CodeRabbit%20for%20my%20code%20review%2C%20and%20it%27s%20fantastic%21%20It%27s%20free%20for%20OSS%20and%20offers%20a%20free%20trial%20for%20proprietary%20code)

Tips

### Chat There are 3 ways to chat with [CodeRabbit](https://coderabbit.ai): - Review comments: Directly reply to a review comment made by CodeRabbit. Example: - `I pushed a fix in commit .` - `Generate unit testing code for this file.` - `Open a follow-up GitHub issue for this discussion.` - Files and specific lines of code (under the "Files changed" tab): Tag `@coderabbitai` in a new review comment at the desired location with your query. Examples: - `@coderabbitai generate unit testing code for this file.` - `@coderabbitai modularize this function.` - PR comments: Tag `@coderabbitai` in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples: - `@coderabbitai generate interesting stats about this repository and render them as a table.` - `@coderabbitai show all the console.log statements in this repository.` - `@coderabbitai read src/utils.ts and generate unit testing code.` - `@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.` - `@coderabbitai help me debug CodeRabbit configuration file.` Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. ### CodeRabbit Commands (invoked as PR comments) - `@coderabbitai pause` to pause the reviews on a PR. - `@coderabbitai resume` to resume the paused reviews. - `@coderabbitai review` to trigger an incremental review. This is useful when automatic reviews are disabled for the repository. - `@coderabbitai full review` to do a full review from scratch and review all the files again. - `@coderabbitai summary` to regenerate the summary of the PR. - `@coderabbitai resolve` resolve all the CodeRabbit review comments. - `@coderabbitai configuration` to show the current CodeRabbit configuration for the repository. - `@coderabbitai help` to get help. Additionally, you can add `@coderabbitai ignore` anywhere in the PR description to prevent this PR from being reviewed. ### CodeRabbit Configration File (`.coderabbit.yaml`) - You can programmatically configure CodeRabbit by adding a `.coderabbit.yaml` file to the root of your repository. - Please see the [configuration documentation](https://docs.coderabbit.ai/guides/configure-coderabbit) for more information. - If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: `# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json` ### Documentation and Community - Visit our [Documentation](https://coderabbit.ai/docs) for detailed information on how to use CodeRabbit. - Join our [Discord Community](https://discord.com/invite/GsXnASn26c) to get help, request features, and share feedback. - Follow us on [X/Twitter](https://twitter.com/coderabbitai) for updates and announcements.

[katamatata]

@ericbolikowski, I think the better way could be to inform them about application invalidation on the platform as well and not let them accept it. But I don't understand why the mentor sees the Accept & Decline buttons on the Invalidated application. They shouldn't:

[astkhikatredi]

Hey @ericbolikowski, thanks for the PR and questions. Dear @katamatata, thanks for the comment.

Here are my thoughts, what do you think?

1. Move Application to Cancelled: Automatically move Kate's application to the Cancelled state once Anil accepts the mentorship. 2. Hide Accept and Decline Buttons: Ensure that Kate no longer sees the Accept andDeclinebuttons, preventing her from accepting the application. 3. Insert Notification: Provide a clear and concise notification to inform Kate about the status change and the reason behind it, for example: "Sorry, this mentorship request cannot be accepted anymore because the mentee has already been accepted by another mentor. As a result, we have moved this application to "Cancelled". We hope you will receive another application very soon."

Thanks!

[ericbolikowski]

Ah, we're getting into the nasty territory of concurrency and simultaneity here.

First, a brainfuck: we're already doing what @astkhikatredi suggests in her points (1) and (2). And @katamatata, yes, we don't show Accept/Decline buttons when application is Invalidated.

Here's why - let me illustrate with the example again:

• Eric sends applications to Anil and Kate. Both applications are in state Applied
• Within seconds of one another (this is important), both Anil and Kate visit the Applications page
• Anil now sees the application he's received. Kate sees the application she's received
• Anil accepts the application. That application becomes Accepted. The application to Kate becomes Invalidated
• ... buuuuuut: Kate's browser still thinks the application is Applied. That's why it still shows Kate the Accept / Decline buttons. If Kate had refreshed the browser, then they would have disappeared

[ericbolikowski]

Writing this made me think about a simple fix: the refresh. We can very easily update our code so that Kate's browser asks the server every five or ten seconds to give it an up-to-date application, so that it can become aware of a change from Applied to Invalidated. Then we will show what we already show to hundreds of mentors that have invalidated applications: the status message "Cancelled".

We don't give a lot of context to tell the mentor in the browser what "Cancelled" means (that's done in a lot of detail in the email they receive), but we can quite easily also add some info bubble/card that says something like "This application was cancelled since another mentor accepted an application from this mentee".

[katamatata]

Writing this made me think about a simple fix: the refresh. We can very easily update our code so that Kate's browser asks the server every five or ten seconds to give it an up-to-date application, so that it can become aware of a change from Applied to Invalidated.

I thought about the refresh, too, and thought we were already doing it. Probably not that often.

[ericbolikowski]

We do all our data loading using react-query. React-query does its queries using a query client what we configure and give it. Here's how we do it in both CON and TP:

Note cacheTime here. The difference between cacheTime and its cousins staleTime and refetchInterval are subtle, but the gist of it here is that the way we've set up the query client, react-query will caches data for up to 5 minutes before it's marked "out of date", which, in combination with some other parameters leads to a refetch under certain conditions.

We can set these options on a per-query level as well. At the moment we've got apps/redi-connect/src/pages/app/applications/Applications.tsx with this:

function Applications() {
  const mentorshipMatchesQuery = useGetMentorshipMatchesQuery()

that could be changed to:

function Applications() {
  const mentorshipMatchesQuery = useGetMentorshipMatchesQuery(
    {},
    { refetchInterval: 5 * 1000 }
  )

... which will probably have this effect: while user is on the applications page, aggressively refetch the data every five seconds. We'd need to recheck the react-query docs (I just did so very briefly right now), but this is easy to put in place.

@katamatata, maybe a nice learning opportunity? Wanna dig in and then we talk more in next mentoring session?

[astkhikatredi]

Dear @ericbolikowski, any timeline for the implementation? THANKS!

[ericbolikowski]

Hi @astkhikatredi, @katamatata and I aligned this morning:

• we're likely going for the approach I outlined in a comment here
• @katamatata is checking the validity of the approach (by looking at react-query docs), or any final tweaks we need to do before implementing it

Most work is already done, so I don't think we're far from completing this ticket.

[katamatata]

@katamatata, maybe a nice learning opportunity? Wanna dig in and then we talk more in next mentoring session?

@ericbolikowski, I apologize for the delayed response and thank you for providing more context on the query implementation!

Upon reviewing the react-query docs, I found that since useQuery considers cached data as stale immediately after fetching, we don't need to use the staleTime: 0 option as the default staleTime is already 0. If we were to pass values greater than 0 to staleTime, it would allow the data to be slightly outdated before triggering a re-fetch, but it's unnecessary in this case.

We indeed can use only the refetchInterval option to customize the mentorshipMatchesQuery in order to automatically re-fetch data in the background and keep it updated at regular intervals without user interaction. Though, I suggest setting refetchInterval to a more reasonable interval - 1 minute:

function Applications() {
  const mentorshipMatchesQuery = useGetMentorshipMatchesQuery(
    {},
    { refetchInterval: 60 * 1000 }
  )

Do you agree?

[ericbolikowski]

@katamatata sounds good - thanks for digging into it. I agree with the higher retfetchInterval of a minute.

Can you test this locally? If it works as intended, you should see a new request every minute (or whatever you set, for testing) in the Network tab in DevTools.

[katamatata]

@ericbolikowski, this solution works fine 👍 I tested it & pushed the commit. As a mentor who started writing an acceptance message in a modal and wasn't fast enough, I see the page reloading and the application with invalidated status (without Accept/Decline buttons). I think besides the email, we should also display a message on the platform to this mentor about the application being invalidated. What do you think?

[ericbolikowski]

@katamatata thank you! And - agreed. Let's talk more about this on Monday morning.

[astkhikatredi]

Hey @katamatata and @ericbolikowski , could you maybe review point 3 in my message above? What do you think about showing it to a mentor with an invalid mentorship application?

3. Insert Notification: "Sorry, this mentorship request cannot be accepted anymore because the mentee has already been accepted by another mentor. As a result, we have moved this application to "Cancelled". We hope you will receive another application very soon."

[ericbolikowski]

@astkhikatredi I'd like that too - I suggest we start off by Kate and I looking at how much effort is required to implement that, code-wise. I'll make a note to make sure we look at this tomorrow (this Monday morning) and get back to you.