Add a confirmation prompt to lock and unlock path locks
📖 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.
window.confirm
?
Why use 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
- Navigate to a project with a repository
- Click on a directory
- Click the "Lock" button above the file/directory viewer
- Click on a file
- 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?
-
📋 Does this MR need a changelog?-
I have included a changelog entry. -
I have not included a changelog entry because _______.
-
- [-] Documentation (if required)
-
Code review guidelines -
Merge request performance guidelines -
Style guides - [-] Database guides
-
Separation of EE specific content
Availability and Testing
-
Review and add/update tests for this feature/bug. Consider all test levels. See the Test Planning Process. -
Tested in all supported browsers - [-] Informed Infrastructure department of a default or new setting change, if applicable per definition of done
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