Users cannot view plan until merge review has been submitted

[parkerabercrombie]

See the comments below. This was reported as a bug, but we converted it to a feature request. The request is to allow for viewing a plan in the UI that is currently undergoing review (and is thus locked).

Version

1.7.0

Describe the feature

If a user initiates a merge review but does not complete the review (e.g. closes the browser) then when another user attempts to open the plan they see the merge review UI instead of the plan contents.

Reproduction

Create a merge request, and open the merge review UI.
Reload the Aerie browser tab and navigate to the plan. Merge review UI appears instead of the plan content.

[camargo]

@parkerabercrombie This was by design. When a merge review is started, the plan is intentionally locked in the database to prevent edits to the plan until the merge review is complete. This is to prevent the plan from getting out of sync with the review.

That said, instead of locking the plan we could potentially make it read-only in the UI since we now have better permission infrastructure. That would require a little more design tho. I can change this to a feature request if that is more what you are going for.

[parkerabercrombie]

Thanks @camargo. Ideally users would be able to view the plan before the merge request was processed (e.g. an instrument team submitted a merge request but the main plain owner is on a long lunch break, and now no one can see the plan until they get back). Please convert to a feature request, but let me gather some more info from the thread test conductors on how much this impacted operations, to inform priority.

[joswig]

Related to ECGDSITD-532

Preliminary design is that users with merge permissions would still initially see the merge view.
Users without merge permissions would initially see the plan in read only view.
All users would be able to switch between views

[joswig]

An option is to move the merge request view to a new endpoint, and then leverage the read only views created for plan snapshot when a merge request is open

[lklyne]

Based on @cohansen and I's discussion, here's a few proposed improvements:

Update the nav in the merge request to "Merge review: name of plan from name of branch"

In a locked plan with an active merge request, show a callout to open the merge review

In the merge requests modal, add a link to open the merge review

[cohansen]

[lklyne]

Looks good @cohansen!

• Is the 1 in the first screenshot a link to the target plan? Its tough to tell the link styling apart from the text. Not sure if that's our default link style or maybe an issue with the screenshot and github compression. Could consider adding an underline or underline on hover.
• Probably ok to ignore, but a nice polish item would be to add 0.5-1rem (8 / 16px) of additional padding between the "open merge request" button and the branch name.

[lklyne]

Also now that i'm looking at this I wonder if "view merge request" would make more sense? Could "open" be confused with "initiate merge request" to someone less familiar with the process?