exercism icon indicating copy to clipboard operation
exercism copied to clipboard

This mentor request is locked by someone else

Open bobahop opened this issue 4 years ago • 9 comments
trafficstars

After spending just a few minutes commenting on a solution, the message "This mentor request is locked by someone else" appears after clicking the "Send" button.

The solution is no longer in the queue, yet the published solution does not indicate that it was mentored by anyone, leading me to suspect that the student cancelled the request.


See action points at the bottom of the issue

bobahop avatar Oct 21 '21 14:10 bobahop

Out of curiosity what is the expected behavior here? I read somewhere that threads are "tentatively locked" for 30 minutes after we open them (to possibly review). Is that correct? IE, if I comment within 30 minutes it should just work and the danger is supposed to be after the 30m mark?

joshgoebel avatar Oct 22 '21 13:10 joshgoebel

Yes, my understanding is that once you are cleared to comment you have thirty minutes to send the comment before it goes back into the pool. I just got bit by this again today. I don't think 2 people should be cleared to comment at the same time. That would be the fix, I think. I think the lock should commence once the "Start Mentoring" button is clicked", not just when viewing, because you may view and decide not to mentor it based on their blurb.

bobahop avatar May 13 '22 17:05 bobahop

I often check the request and then back out, so this would be good to have as explicit button, indeed!

SleeplessByte avatar May 13 '22 17:05 SleeplessByte

@ErikSchierboom: This is the React: app/javascript/components/mentoring/request/StartMentoringPanel.tsx

Thoughts:

  1. At a glance, it doesn't look like it's doing anything if the lock fails, so that's an issue.
  2. But then I'd not expect Bob to be able to continue, so possibly the API controller (app/controllers/api/mentoring/requests_controller.rb) isn't raising properly if this fails, or the JS isn't properly checking to see if it's passed or failed.
  3. So maybe the command (app/commands/mentor/request/lock.rb) isn't raising, or that raise is somehow being caught somewhere I'm totally missing.

I'd check the system tests and add a new one to see what happens as a starting point.

iHiD avatar May 13 '22 19:05 iHiD

Something else to consider is if the student cancels their mentoring request. If so, it might be helpful to have the error message say that instead of saying that another mentor has locked the request, if that's what's happening now.

bobahop avatar May 14 '22 01:05 bobahop

I've dug into this a bit more, and there are a couple of issues:

  1. The error message is hidden at the bottom of the screen
  2. The error message doesn't differentiate between a lock from another mentor or a cancelled request
  3. There is no instant feedback to the mentor when the student cancels the request. Ideally, the mentor would be notified as soon as the request is cancelled

ErikSchierboom avatar Jun 10 '22 12:06 ErikSchierboom

I've just been through the logs for #6532 where I could catch it early. What happened was that the user removed their request for mentoring just before the feedback was published, so then when feedback was posted, there was no longer a mentor request that the system could fulfil.

I think there are four improvements we should make:

  1. A better error message that tells the mentor what's happened
  2. A websocket notification to the mentor that the student had removed the mentoring request so the mentor can stop typing your response.
  3. Add a banner to a user's solution that a mentor is giving them feedback
  4. If they do cancel, warn the user that a mentor has already started giving feedback, and encourage them not to cancel their request.

Is there anything else we could do to make the UX work better here? Otherwise, I'll add this to the next work-cycle.

iHiD avatar Aug 31 '22 18:08 iHiD

Just go this again today. It brought a little tear to my eye.

bobahop avatar Sep 28 '22 20:09 bobahop

These UX improvements sounds great.

Have a aria-live="polite" region on page render, and render the banner and or notification in there when it starts happening whilst the page is already open.

I especially like 4.

SleeplessByte avatar Sep 29 '22 01:09 SleeplessByte