Call QuickActionsService on Note update
Add support for notes which just contain
commands after editing
Return http status gone (410) if note was deleted
Temporary frontend addition so it is not
failing when a note is deleted
Move specs to shared examples
Fix rubocop style issue
Deleting note on frontend when status is 410
Use guard clause for note which got deleted
Simplified condition for nil note
This method should no longer be called
with nil note
Refactoring of execute method to reduce
complexity
Move errors update to delete_note method
Note is now deleted visually when it only
contains commands after update
Add expectation
Fix style issues
Changing action to fix tests
Add tests for removeNote and update
deleteNote expectations
Remove project from NotesFinder constructor
Add project parameter to specs
Also look for methods in private scope
Fix specs to match new NotesFinder constructor
Enables frozen string for some vestigial files as
well as the following:
* app/controllers/projects/**/*.rb
* app/controllers/sherlock/**/*.rb
* app/controllers/snippets/**/*.rb
* app/controllers/users/**/*.rb
Partially addresses #47424.
The status is shown for
- The author of a commit when viewing a commit
- Notes on a commit (regular/diff)
- The user that triggered a pipeline when viewing a pipeline
- The author of a merge request when viewing a merge request
- The author of notes on a merge request (regular/diff)
- The author of an issue when viewing an issue
- The author of notes on an issue
- The author of a snippet when viewing a snippet
- The author of notes on a snippet
- A user's profile page
- The list of members of a group/user
This refactors the Markdown pipeline so it supports the rendering of
multiple documents that may belong to different projects. An example of
where this happens is when displaying the event feed of a group. In this
case we retrieve events for all projects in the group. Previously we
would group events per project and render these chunks separately, but
this would result in many SQL queries being executed. By extending the
Markdown pipeline to support this out of the box we can drastically
reduce the number of SQL queries.
To achieve this we introduce a new object to the pipeline:
Banzai::RenderContext. This object simply wraps two other objects: an
optional Project instance, and an optional User instance. On its own
this wouldn't be very helpful, but a RenderContext can also be used to
associate HTML documents with specific Project instances. This work is
done in Banzai::ObjectRenderer and allows us to reuse as many queries
(and results) as possible.
This ensures that we have more visibility in the number of SQL queries
that are executed in web requests. The current threshold is hardcoded to
100 as we will rarely (maybe once or twice) change it.
In production and development we use Sentry if enabled, in the test
environment we raise an error. This feature is also only enabled in
production/staging when running on GitLab.com as it's not very useful to
other users.
* Changed name of delete_user_service and worker to destroy
* Move and change delete_group_service to Groups::DestroyService
* Rename Notes::DeleteService to Notes::DestroyService
Fix missing Note access checks in by moving Note#search to updated NoteFinder
Split from !2024 to partially solve https://gitlab.com/gitlab-org/gitlab-ce/issues/23867
## Which fixes are in this MR?
⚠️ - Potentially untested
💣 - No test coverage
🚥 - Test coverage of some sort exists (a test failed when error raised)
🚦 - Test coverage of return value (a test failed when nil used)
✅ - Permissions check tested
### Note lookup without access check
- [x] ✅ app/finders/notes_finder.rb:13 :download_code check
- [x] ✅ app/finders/notes_finder.rb:19 `SnippetsFinder`
- [x] ✅ app/models/note.rb:121 [`Issue#visible_to_user`]
- [x] ✅ lib/gitlab/project_search_results.rb:113
- This is the only use of `app/models/note.rb:121` above, but importantly has no access checks at all. This means it leaks MR comments and snippets when those features are `team-only` in addition to the issue comments which would be fixed by `app/models/note.rb:121`.
- It is only called from SearchController where `can?(current_user, :download_code, @project)` is checked, so commit comments are not leaked.
### Previous discussions
- [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b915c5267a63628b0bafd23d37792ae73ceae272_13_13 `: download_code` check on commit
- [x] https://dev.gitlab.org/gitlab/gitlabhq/merge_requests/2024/diffs#b915c5267a63628b0bafd23d37792ae73ceae272_19_19 `SnippetsFinder` should be used
- `SnippetsFinder` should check if the snippets feature is enabled -> https://gitlab.com/gitlab-org/gitlab-ce/issues/25223
### Acceptance criteria met?
- [x] Tests added for new code
- [x] TODO comments removed
- [x] Squashed and removed skipped tests
- [x] Changelog entry
- [ ] State Gitlab versions affected and issue severity in description
- [ ] Create technical debt issue for NotesFinder.
- Either split into `NotesFinder::ForTarget` and `NotesFinder::Search` or consider object per notable type such as `NotesFinder::OnIssue`. For the first option could create `NotesFinder::Base` which is either inherited from or which can be included in the other two.
- Avoid case statement anti-pattern in this finder with use of `NotesFinder::OnCommit` etc. Consider something on the finder for this? `Model.finder(user, project)`
- Move `inc_author` to the controller, and implement `related_notes` to replace `non_diff_notes`/`mr_and_commit_notes`
See merge request !2035
GitLab Bot
Patrick Derichs
Oswaldo Ferreira
gfyoung
Bob Van Landuyt
blackst0ne
Jan Provaznik
Yorick Peterse
Fatih Acet
Rémy Coutable
Jacopo
Jarka Kadlecova
Douwe Maan
Adam Niedzielski
mhasbini
dixpac
Jarka Kadlecova
Douwe Maan