<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/gitlab/gitlab-ce.git/app/controllers/projects/notes_controller.rb, branch diff-notes-refactor</title>
<subtitle>gitlab.com: gitlab-org/gitlab-ce.git
</subtitle>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/'/>
<entry>
<title>support `/merge` slash comand for MRs</title>
<updated>2017-01-11T13:48:07+00:00</updated>
<author>
<name>Jarka Kadlecova</name>
<email>kadlecovaj@gmail.com</email>
</author>
<published>2016-11-24T14:05:15+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=7ab3dd4b302a85c1b005e9ef290ebac631cda673'/>
<id>7ab3dd4b302a85c1b005e9ef290ebac631cda673</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'jej-note-search-uses-finder' into 'security'</title>
<updated>2016-12-15T14:40:12+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@gitlab.com</email>
</author>
<published>2016-12-09T01:56:31+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=12db4cc0e70d3e249f3bf9fde85e336839422319'/>
<id>12db4cc0e70d3e249f3bf9fde85e336839422319</id>
<content type='text'>

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?

:warning: - Potentially untested  
:bomb: - No test coverage  
:traffic_light: - Test coverage of some sort exists (a test failed when error raised)  
:vertical_traffic_light: - Test coverage of return value (a test failed when nil used)  
:white_check_mark: - Permissions check tested

### Note lookup without access check

- [x] :white_check_mark: app/finders/notes_finder.rb:13 :download_code check
- [x] :white_check_mark: app/finders/notes_finder.rb:19 `SnippetsFinder`
- [x] :white_check_mark: app/models/note.rb:121 [`Issue#visible_to_user`]
- [x] :white_check_mark: 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 -&gt; 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</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>

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?

:warning: - Potentially untested  
:bomb: - No test coverage  
:traffic_light: - Test coverage of some sort exists (a test failed when error raised)  
:vertical_traffic_light: - Test coverage of return value (a test failed when nil used)  
:white_check_mark: - Permissions check tested

### Note lookup without access check

- [x] :white_check_mark: app/finders/notes_finder.rb:13 :download_code check
- [x] :white_check_mark: app/finders/notes_finder.rb:19 `SnippetsFinder`
- [x] :white_check_mark: app/models/note.rb:121 [`Issue#visible_to_user`]
- [x] :white_check_mark: 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 -&gt; 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</pre>
</div>
</content>
</entry>
<entry>
<title>Backport Note#commands_changes from EE</title>
<updated>2016-11-24T06:32:32+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@selenight.nl</email>
</author>
<published>2016-11-24T06:32:32+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=ba5e98bb701672d0cf1d98a80272c16a754ec83c'/>
<id>ba5e98bb701672d0cf1d98a80272c16a754ec83c</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Backport some changes done from Time Tracking feature in EE.</title>
<updated>2016-11-19T02:19:04+00:00</updated>
<author>
<name>Ruben Davila</name>
<email>rdavila84@gmail.com</email>
</author>
<published>2016-11-19T02:19:04+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=452d1d0824f44664484c35a2acc7c008c8167196'/>
<id>452d1d0824f44664484c35a2acc7c008c8167196</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'diff-line-comment-vuejs' into 'master'</title>
<updated>2016-08-19T02:24:34+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@gitlab.com</email>
</author>
<published>2016-08-19T02:24:34+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=579951720afcfa7d2fbef6f3dc2d2ca0d2816025'/>
<id>579951720afcfa7d2fbef6f3dc2d2ca0d2816025</id>
<content type='text'>

Diff line comments resolve

## What does this MR do?

Diff line comments can be resolved.

Part of #10325 

To do:

- [x] Backend (@DouweM)
  - [x] Fix https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13319326. Will be made easier by https://gitlab.com/gitlab-org/gitlab-ce/issues/17237#note_13370331
  - [x] System note when all discussions are resolved
  - [x] Notification when all discussions are resolved
  - [x] Write unit tests
  - [x] Look at resolve time https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13912743 - Fixed by 4a13aa9
- [x] Frontend (@iamphill)
  - [x] Fix bugs
  - [x] Write more feature tests 
- [x] Frontend (@connorshea)
  - [x] Address frontend feedback
  - [x] Feature specs for Jump feature
  - [x] Documentation
  - [x] Add Vue.js in a standard way

See merge request !5022</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>

Diff line comments resolve

## What does this MR do?

Diff line comments can be resolved.

Part of #10325 

To do:

- [x] Backend (@DouweM)
  - [x] Fix https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13319326. Will be made easier by https://gitlab.com/gitlab-org/gitlab-ce/issues/17237#note_13370331
  - [x] System note when all discussions are resolved
  - [x] Notification when all discussions are resolved
  - [x] Write unit tests
  - [x] Look at resolve time https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/5022#note_13912743 - Fixed by 4a13aa9
- [x] Frontend (@iamphill)
  - [x] Fix bugs
  - [x] Write more feature tests 
- [x] Frontend (@connorshea)
  - [x] Address frontend feedback
  - [x] Feature specs for Jump feature
  - [x] Documentation
  - [x] Add Vue.js in a standard way

See merge request !5022</pre>
</div>
</content>
</entry>
<entry>
<title>Address review feedback</title>
<updated>2016-08-16T16:04:26+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@selenight.nl</email>
</author>
<published>2016-08-15T23:45:23+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=41007f6d3c30a294bbf361ff900b3b19bb463291'/>
<id>41007f6d3c30a294bbf361ff900b3b19bb463291</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Support slash commands in noteable description and notes</title>
<updated>2016-08-12T22:05:57+00:00</updated>
<author>
<name>Rémy Coutable</name>
<email>remy@rymai.me</email>
</author>
<published>2016-06-30T15:34:19+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=0eea8c885743575b0e93a98846b3663e9903aa66'/>
<id>0eea8c885743575b0e93a98846b3663e9903aa66</id>
<content type='text'>
Some important things to note:

- commands are removed from noteable.description / note.note
- commands are translated to params so that they are treated as normal
  params in noteable Creation services
- the logic is not in the models but in the Creation services, which is
  the right place for advanced logic that has nothing to do with what
  models should be responsible of!
- UI/JS needs to be updated to handle notes which consist of commands
  only
- the `/merge` command is not handled yet

Other improvements:

- Don't process commands in commit notes and display a flash is note is only commands
- Add autocomplete for slash commands
- Add description and params to slash command DSL methods
- Ensure replying by email with a commands-only note works
- Use :subscription_event instead of calling noteable.subscribe
- Support :todo_event in IssuableBaseService

Signed-off-by: Rémy Coutable &lt;remy@rymai.me&gt;
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Some important things to note:

- commands are removed from noteable.description / note.note
- commands are translated to params so that they are treated as normal
  params in noteable Creation services
- the logic is not in the models but in the Creation services, which is
  the right place for advanced logic that has nothing to do with what
  models should be responsible of!
- UI/JS needs to be updated to handle notes which consist of commands
  only
- the `/merge` command is not handled yet

Other improvements:

- Don't process commands in commit notes and display a flash is note is only commands
- Add autocomplete for slash commands
- Add description and params to slash command DSL methods
- Ensure replying by email with a commands-only note works
- Use :subscription_event instead of calling noteable.subscribe
- Support :todo_event in IssuableBaseService

Signed-off-by: Rémy Coutable &lt;remy@rymai.me&gt;
</pre>
</div>
</content>
</entry>
<entry>
<title>Add specs for NotesController and DiscussionsController</title>
<updated>2016-08-12T21:24:09+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@selenight.nl</email>
</author>
<published>2016-08-12T21:24:09+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=e25720045988e59f7ddfecebbc02e640dc2f8360'/>
<id>e25720045988e59f7ddfecebbc02e640dc2f8360</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Actually don't send resolved notifications when deleting a note</title>
<updated>2016-08-04T20:44:28+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@selenight.nl</email>
</author>
<published>2016-08-04T00:16:37+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=9b115ce4d3ccdd03043f070935d3a25e96f537f4'/>
<id>9b115ce4d3ccdd03043f070935d3a25e96f537f4</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Don't send resolved notifications when deleting a note</title>
<updated>2016-07-29T03:14:41+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@selenight.nl</email>
</author>
<published>2016-07-29T03:14:41+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=1bee660b96d7a84cfcc4a274c68cbe0fba4b5fa5'/>
<id>1bee660b96d7a84cfcc4a274c68cbe0fba4b5fa5</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
</feed>
