Skip to content

Add a confirmation prompt to lock and unlock path locks

Stan Hu requested to merge sh-add-path-lock-confirmation into master

📖 What does this MR do?

Related to #11826 (closed)

Many users have accidentally clicked on the "Lock" and "Unlock" buttons when looking at a specific file in a repository. This has led to mysterious merge errors due to the backend not surfacing the right error message.

To reduce the chance of accidentally clicking, we now prompt a confirmation modal before toggling the state.

Why use window.confirm?

Currently the implementation of these buttons makes it really hard to use the GitLab UI modal. I went down the rabbit hole of trying to implement a confirmation modal and kept running into issues that were difficult to solve. Here is a branch with that discovery f6037a2d

Ultimately I feel like we need to move these buttons into a Vue application or we are going to create unnecessary technical debit. I did some searching and realized that we have &5531 (closed) for the very purpose of converting these controls to Vue.

Furthermore I discovered #323527 (closed) which will (most likely) require refactoring to Vue to fix.

So long story short, I think the MVC is to use window.confirm

💻 Local testing

  1. Navigate to a project with a repository
  2. Click on a directory
  3. Click the "Lock" button above the file/directory viewer
  4. Click on a file
  5. Click the "Lock" button above the file/directory viewer

📹 Videos

View Before After
Lock directory Screen_Recording_2021-03-05_at_10.24.36_AM Screen_Recording_2021-03-05_at_10.19.10_AM
Lock file Screen_Recording_2021-03-05_at_10.25.13_AM Screen_Recording_2021-03-05_at_10.19.56_AM

🚦 Does this MR meet the acceptance criteria?

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • [-] Label as security and @ mention @gitlab-com/gl-security/appsec
  • [-] The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • [-] Security reports checked/validated by a reviewer from the AppSec team
Edited by Peter Hegman

Merge request reports