<feed xmlns='http://www.w3.org/2005/Atom'>
<title>delta/gitlab/gitlab-ce.git/app/models/note.rb, branch fix-include-description-commit-message</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>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>Feature: delegate all open discussions to Issue</title>
<updated>2016-12-05T19:55:45+00:00</updated>
<author>
<name>Bob Van Landuyt</name>
<email>bob@vanlanduyt.co</email>
</author>
<published>2016-10-26T21:21:50+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=1123057ab792ac73b1611f4d3a9faf79369dd6da'/>
<id>1123057ab792ac73b1611f4d3a9faf79369dd6da</id>
<content type='text'>
When a merge request can only be merged when all discussions are
resolved. This feature allows to easily delegate those discussions to a
new issue, while marking them as resolved in the merge request.

The user is presented with a new issue, prepared with mentions of all
unresolved discussions, including the first unresolved note of the
discussion, time and link to the note.

When the issue is created, the discussions in the merge request will get
a system note directing the user to the newly created issue.
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
When a merge request can only be merged when all discussions are
resolved. This feature allows to easily delegate those discussions to a
new issue, while marking them as resolved in the merge request.

The user is presented with a new issue, prepared with mentions of all
unresolved discussions, including the first unresolved note of the
discussion, time and link to the note.

When the issue is created, the discussions in the merge request will get
a system note directing the user to the newly created issue.
</pre>
</div>
</content>
</entry>
<entry>
<title>Merge branch 'events-cache-invalidation' into 'master'</title>
<updated>2016-11-28T03:34:12+00:00</updated>
<author>
<name>Douwe Maan</name>
<email>douwe@gitlab.com</email>
</author>
<published>2016-11-28T03:34:12+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=d8f75233686fe20bff26599704fbcb235f7bb43b'/>
<id>d8f75233686fe20bff26599704fbcb235f7bb43b</id>
<content type='text'>

Remove caching of events data

This MR removes the caching of events data as this was deemed unnecessary while increasing load on the database. See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6578#note_18864037 and 5371da341e9d7768ebab8e159b3e2cc8fad1d827 for more information.

See merge request !6578</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>

Remove caching of events data

This MR removes the caching of events data as this was deemed unnecessary while increasing load on the database. See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6578#note_18864037 and 5371da341e9d7768ebab8e159b3e2cc8fad1d827 for more information.

See merge request !6578</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>Remove event caching code</title>
<updated>2016-11-23T13:17:07+00:00</updated>
<author>
<name>Yorick Peterse</name>
<email>yorickpeterse@gmail.com</email>
</author>
<published>2016-11-23T13:14:04+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=5371da341e9d7768ebab8e159b3e2cc8fad1d827'/>
<id>5371da341e9d7768ebab8e159b3e2cc8fad1d827</id>
<content type='text'>
Flushing the events cache worked by updating a recent number of rows in
the "events" table. This has the result that on PostgreSQL a lot of dead
tuples are produced on a regular basis. This in turn means that
PostgreSQL will spend considerable amounts of time vacuuming this table.
This in turn can lead to an increase of database load.

For GitLab.com we measured the impact of not using events caching and
found no measurable increase in response timings. Meanwhile not flushing
the events cache lead to the "events" table having no more dead tuples
as now rows are only inserted into this table.

As a result of this we are hereby removing events caching as it does not
appear to help and only increases database load.

For more information see the following comment:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6578#note_18864037
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
Flushing the events cache worked by updating a recent number of rows in
the "events" table. This has the result that on PostgreSQL a lot of dead
tuples are produced on a regular basis. This in turn means that
PostgreSQL will spend considerable amounts of time vacuuming this table.
This in turn can lead to an increase of database load.

For GitLab.com we measured the impact of not using events caching and
found no measurable increase in response timings. Meanwhile not flushing
the events cache lead to the "events" table having no more dead tuples
as now rows are only inserted into this table.

As a result of this we are hereby removing events caching as it does not
appear to help and only increases database load.

For more information see the following comment:
https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/6578#note_18864037
</pre>
</div>
</content>
</entry>
<entry>
<title>Does not raise error when Note not found when processing NewNoteWorker</title>
<updated>2016-11-12T00:54:11+00:00</updated>
<author>
<name>Oswaldo Ferreira</name>
<email>oswluizf@gmail.com</email>
</author>
<published>2016-10-13T15:26:44+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=d48d879ef5e0b1517c43bef27f584655535259c8'/>
<id>d48d879ef5e0b1517c43bef27f584655535259c8</id>
<content type='text'>
- Also remove unnecessary param
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
- Also remove unnecessary param
</pre>
</div>
</content>
</entry>
<entry>
<title>Use CacheMarkdownField for notes</title>
<updated>2016-10-07T01:54:26+00:00</updated>
<author>
<name>Nick Thomas</name>
<email>nick@gitlab.com</email>
</author>
<published>2016-10-06T21:52:44+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=109816c42fbe44fca108b52308a5fa4366876216'/>
<id>109816c42fbe44fca108b52308a5fa4366876216</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>Start Frontend work, fix routing problem</title>
<updated>2016-09-19T16:50:40+00:00</updated>
<author>
<name>Z.J. van de Weg</name>
<email>zegerjan@gitlab.com</email>
</author>
<published>2016-09-04T08:51:12+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=412ff80b7b1381485a306df240aab020e9b90801'/>
<id>412ff80b7b1381485a306df240aab020e9b90801</id>
<content type='text'>
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
</pre>
</div>
</content>
</entry>
<entry>
<title>prevent authored awardable thumbs votes</title>
<updated>2016-08-28T17:15:55+00:00</updated>
<author>
<name>barthc</name>
<email>mackintosh02@gmail.com</email>
</author>
<published>2016-08-17T11:21:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=76872372376e57cd7d55ba9b9c63b25fe53c82df'/>
<id>76872372376e57cd7d55ba9b9c63b25fe53c82df</id>
<content type='text'>
prevent authored awardable thumbs votes

prevent authored awardable thumbs votes
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
prevent authored awardable thumbs votes

prevent authored awardable thumbs votes
</pre>
</div>
</content>
</entry>
<entry>
<title>Fix Error 500 resulting when loading network graph</title>
<updated>2016-08-20T16:57:20+00:00</updated>
<author>
<name>Stan Hu</name>
<email>stanhu@gmail.com</email>
</author>
<published>2016-08-20T16:18:06+00:00</published>
<link rel='alternate' type='text/html' href='http://git.baserock.org/cgit/delta/gitlab/gitlab-ce.git/commit/?id=5cb488e8a1a10432c1c5a322b2d4748cb754277f'/>
<id>5cb488e8a1a10432c1c5a322b2d4748cb754277f</id>
<content type='text'>
`discussion_id` may not be present when the SELECT call for notes
does not include this attribute. Don't attempt to set the discussion ID
unless the model contains the attribute.

Closes #21119, #21128
</content>
<content type='xhtml'>
<div xmlns='http://www.w3.org/1999/xhtml'>
<pre>
`discussion_id` may not be present when the SELECT call for notes
does not include this attribute. Don't attempt to set the discussion ID
unless the model contains the attribute.

Closes #21119, #21128
</pre>
</div>
</content>
</entry>
</feed>
