Changed the review screen to only default for someone that has perms,…

[cohansen]

… otherwise they see a readonly plan

Closes #829.

Going forward you only get directed to the /merge route for a plan if you have the proper plan merge permissions. If you don't you see a readonly version of the plan.

[AaronPlave]

A few notes:

• Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?
• I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

[cohansen]

A few notes:

* Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?

* I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

Sorry I didn't see a notification for this!

1. We could enable this, we'd just need a way to go back and forth between the plan merge and the plan itself other than manually editing the URL. This PR could encompass that if we come up with a clear direction for this feature.
2. I don't think we could ever get into that state in production? Someone's role will never change on the fly while they're viewing a page right?

[AaronPlave]

A few notes:

* Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?

* I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

Sorry I didn't see a notification for this!

1. We could enable this, we'd just need a way to go back and forth between the plan merge and the plan itself other than manually editing the URL. This PR could encompass that if we come up with a clear direction for this feature.
2. I don't think we could ever get into that state in production? Someone's role will never change on the fly while they're viewing a page right?

1. 👍
2. Seeing this if you switch from user -> viewer using the role dropdown

[cohansen]

A few notes:

* Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?

* I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

Sorry I didn't see a notification for this!

1. We could enable this, we'd just need a way to go back and forth between the plan merge and the plan itself other than manually editing the URL. This PR could encompass that if we come up with a clear direction for this feature.
2. I don't think we could ever get into that state in production? Someone's role will never change on the fly while they're viewing a page right?

1. 👍

2. Seeing this if you switch from user -> viewer using the role dropdown

That dropdown doesn't exist when the app is deployed right?

[AaronPlave]

A few notes:

* Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?

* I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

Sorry I didn't see a notification for this!

1. We could enable this, we'd just need a way to go back and forth between the plan merge and the plan itself other than manually editing the URL. This PR could encompass that if we come up with a clear direction for this feature.
2. I don't think we could ever get into that state in production? Someone's role will never change on the fly while they're viewing a page right?

1. 👍

2. Seeing this if you switch from user -> viewer using the role dropdown

That dropdown doesn't exist when the app is deployed right?

Believe it does, at least for Clipper? Could be wrong, maybe @Mythicaeda or @duranb knows?

[Mythicaeda]

A few notes:

* Would it be possible for us to have a way for anyone to view the plan and optionally the merge if they have merge review permissions?

* I'm seeing that if you switch to "viewer" on the merge review page you aren't redirected and activities disappear. Likely not from this PR though?

Sorry I didn't see a notification for this!

1. We could enable this, we'd just need a way to go back and forth between the plan merge and the plan itself other than manually editing the URL. This PR could encompass that if we come up with a clear direction for this feature.
2. I don't think we could ever get into that state in production? Someone's role will never change on the fly while they're viewing a page right?

1. 👍

2. Seeing this if you switch from user -> viewer using the role dropdown

That dropdown doesn't exist when the app is deployed right?

Believe it does, at least for Clipper? Could be wrong, maybe @Mythicaeda or @duranb knows?

This dropdown in the top-right? This one always exists unless the current user only has one allowed role. (Though maybe we should still display it but in a non-interactable form so the user knows their role?) A user not being able to swap roles on the fly is considered the rare case.

[cohansen]

@AaronPlave and @duranb this should be ready to review again. We're not longer redirecting users automatically but we added buttons to navigate between the plan and the merge request, see https://github.com/NASA-AMMOS/aerie-ui/issues/829 for more details.

[AaronPlave]

A few comments:

1. If the user had enabled drag activities mode before plan was locked, the user can still drag activities around during plan lock though they fail to update. Could decide to take no action on this since DB is unaffected.
2. Consider hiding the View Merge Request button in the Merge Request modal if the request has not entered review since otherwise clicking on the button results in no action?
3. If i enter merge review, click on the child branch to navigate back to that plan, and then click on "view merge request" button, I am not taken back to merge request.
4. Issue if you open a merge request, go to review, click on either parent or child plan names to go back to the plan, click on view merge request again, and then cancel the merge request. Result is that merge request is canceled but plan is still locked in the UI, have to refresh the page to get out of the locked state.

[cohansen]

A few comments:

1. If the user had enabled drag activities mode before plan was locked, the user can still drag activities around during plan lock though they fail to update. Could decide to take no action on this since DB is unaffected.

2. Consider hiding the `View Merge Request` button in the Merge Request modal if the request has not entered review since otherwise clicking on the button results in no action?

3. If i enter merge review, click on the child branch to navigate back to that plan, and then click on "view merge request" button, I am not taken back to merge request.

4. Issue if you open a merge request, go to review, click on either parent or child plan names to go back to the plan, click on view merge request again, and then cancel the merge request. Result is that merge request is canceled but plan is still locked in the UI, have to refresh the page to get out of the locked state.

The branch should be ready for re-testing. Hopefully all the edge cases where the buttons aren't working are corrected and note the changes that I message you and @lklyne about!

[AaronPlave]

Still seeing: 4. Issue if you open a merge request, go to review, click on either parent or child plan names to go back to the plan, click on view merge request again, and then cancel the merge request. Result is that merge request is canceled but plan is still locked in the UI, have to refresh the page to get out of the locked state.

One last tweak I might suggest would be to add some button at the bottom here that takes you back to the plan? Navigating from the top of the navbar is nice but maybe less discoverable and my gut reaction was to use Cancel button to take me back even though this cancels the review. Could also rename cancel -> cancel review?

[cohansen]

Still seeing: 4. Issue if you open a merge request, go to review, click on either parent or child plan names to go back to the plan, click on view merge request again, and then cancel the merge request. Result is that merge request is canceled but plan is still locked in the UI, have to refresh the page to get out of the locked state.

One last tweak I might suggest would be to add some button at the bottom here that takes you back to the plan? Navigating from the top of the navbar is nice but maybe less discoverable and my gut reaction was to use Cancel button to take me back even though this cancels the review. Could also rename cancel -> cancel review?

Okay, fix incoming for that problem as well. I found the spot I missed. I think renaming it to Cancel Review would make it more clear for sure.