Indicate review in progress on reviewer sidebar

added to epic &9577

@phikai, Please add a group or category label to identify issue ownership.

You can refer to the Features by Group handbook page for guidance.

If you are unsure about the correct group, please do not leave the issue without a group label, and refer to GitLab's shared responsibility functionality guidelines for more information on how to triage this kind of issue.

This message was generated automatically. You're welcome to improve it.

This issue was automatically tagged with the label groupcode review by TanukiStan, a machine learning classification model, with a probability of 0.93.

If this label is incorrect, please tag this issue with the correct group label as well as automation:ml wrong to help TanukiStan learn from its mistakes.

If you are unsure about the correct group, please do not leave the issue without a group label. Please refer to GitLab's shared responsibility functionality guidelines for more information on how to triage this kind of issues.

Authors who do not have permission to update labels can use the @gitlab-bot label ~"group::<correct group name>" command, or leave the issue to be triaged by group leaders initially assigned by TanukiStan.

This message was generated automatically. You're welcome to improve it.

changed the description

added automation:ml groupcode review labels

added devopscreate sectiondev labels

changed the description

added featureenhancement typefeature labels

@mle @afracazo As we're working through states with the new merge request homepage, I wonder if this is a state we should consider and how we would reflect this. This would be slightly more generic than something like &1409 which might track progress or each individual file, but could be something to signal that a person is at least looking.

WDYT?

@phikai and @mle Yes! If we have an "in progress" state, it would help not only the merge request homepage but also to align the status of the MR review, current it's inconsistent.

List

MR page

@phikai I agree that there could be an intermediate state. We can use distinct states based on buttons pressed to have:

Review requested
Started
Approved / changes requested

I can also see that "Started" could be replaced with "In progress":

Review requested
Review in progress
Approved / changes requested

I am not too precious about that. In my previous life working in online food deliveries, I ran into scenarios where mixing the states caused confusion but I don't see that happening here.

It's possible (likely?) that the interim state between started review and submitted review is so short that there's not much value here

For small merge requests yes but some reviews can take days or someone forgot to submit their review. Having feedback about when the review was started would be helpful - "Hey it looks like you started this review 2 days ago, is it going or do you need help with anything?"

By combining with &1409, this info could be displayed to let you know more details of the review in progress "Michael started review 2 days ago. 10/15 viewed."

@mle or @afracazo Could you decide what this might look like and then let's try and get it into %17.3 since it sounds like we:

Think this state makes sense
Agree it would impact the merge request home page

I assume all we're really talking about here is a new icon for the side bar that indicates "in progress" and tying that to the trigger of someone clicking Start a review?

cc: @andr3 @iamphill for planning purposes

@phikai I will leave this to @mle to decide on what approach to take. Once we have a direction I can bring it to the merge request home page.

@mle @phikai adding workflowdesign

@mle Are you able to determine what we want to do here prior to %17.3 or should we wait until after your review flow work?

@mle do you think that the paradigm of "User is typing" would apply here?

Kapture_2024-07-03_at_13.18.49

@phikai @andr3 I have updated the issue description with an icon and detials for the state of REVIEW_STARTED

I like the idea of the paradigm of "user typing" but the time between when a review starts to when it submits can range from a few minutes to a few days. Therefore at this time I would suggest a static icon as it would cover that variable time . I considered too since it is related closely to REVIEWED state but going with status-running icon to convey that the review is in progress.

I am open to feedback around what triggers a start of a review: To keep it simple, I would recommend that when someone clicks on "Start a review" that would move the review to a state of REVIEW_STARTED.

One thought I have is whether when a reviewer marks a file as Viewed or comments on a file diff but didn't click "Start a review" does that mean a review has been started? I don't have a solid answer yet at this time but will address it as I progress further on the interactions around starting a review for the next few days.

@mle FWIW, I agree for keeping this with Start a Review. Any other actions would be implicitly related and users might not expect a check in Viewed to trigger a change in how their Reviewer status shows to others.

status-running seems ok to me, even if it might invoke the CI status to me, since it's displayed in the context of Reviewers, I think it's quite self-explanatory.

Question: If I requested comments but end up starting another review before anyone Re-requests another review, does that mean it would go from REQUESTED_CHANGES to REVIEW_STARTED? /Cc @iamphill

Question: If I requested comments but end up starting another review before anyone Re-requests another review, does that mean it would go from REQUESTED_CHANGES to REVIEW_STARTED?

We can do an extra check here and not update to REVIEW_STARTED if it is anything else other than UNREVIEWED