Mark files as viewed
What does this MR do?
For #17531 (closed)
This feature is behind a feature flag -
:local_file_reviews- which is disabled by default
Rails Console Enable it with Feature.enable(:local_file_reviews)Disable it again with Feature.disable(:local_file_reviews)
This MR adds a UI to review files in an MR.
| Merge Request | Description |
|---|---|
|
!49173 (merged) !49174 (merged) !49190 (merged) !49208 (merged) |
Various cleanup merge requests in preparation for this feature. |
| !49506 (merged) | Identify diff files using the content_sha
|
| !50022 (merged) | Switch identification to use blob.id
|
| !50033 (merged) | Add tooling to add/remove reviews from LocalStorage to accommodate the file review feature |
| !50132 (merged) | Add the current merge request IID to the file ID |
| We're here |
Add UI to review files |
| !48976 (merged) | Add documentation for file reviews |
Use the feature
Click the Viewed checkbox on a diff file to mark that file as reviewed.
Click the checkbox again to unmark the file.
Reviewed files will be collapsed when they are reviewed (nothing happens if they're already collapsed), and they will start collapsed when the UI loads.
Files that are unreviewed will be expanded (nothing happens if they're already expanded), and they will start with whatever collapse state would be default otherwise (automatically collapsed, in some cases, not collapsed at all, or manually toggled which "pins" the collapsed state to whatever the user selected).
Notable caveats
- Reviews will persist for a file until it changes.
- This means that a review will persist forward through many MR versions if a file doesn't change in those versions
- It also means that a review will propagate backwards through versions if the file is identical in those versions to the reviewed version
- Reviews are local only.
- They are not stored to any centralized server, so
- They will not be available to the same user on multiple devices, and
- They will not be visible in any way to other users
Screenshots (strongly suggested)
| Scenario | Media |
|---|---|
| Default (unreviewed) or after unreviewing a reviewed file |  |
| Reviewed |  |
| Interactivity |  |
Does this MR meet the acceptance criteria?
Conformity
-
Changelog entry -
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