diff options
author | Jan Provaznik <jprovaznik@gitlab.com> | 2018-09-07 13:39:20 +0000 |
---|---|---|
committer | Jan Provaznik <jprovaznik@gitlab.com> | 2018-09-07 13:39:20 +0000 |
commit | d95c1f0335f7309114fcbb0d5413b28e1701a640 (patch) | |
tree | 6b22580a79dd1f929aecd158c31706ce3870c39b /spec | |
parent | 81f4dc059db91577f72134e6008680b72029a29e (diff) | |
download | gitlab-ce-d95c1f0335f7309114fcbb0d5413b28e1701a640.tar.gz |
Use ResourceLabelEvent for tracking label changes
Diffstat (limited to 'spec')
26 files changed, 574 insertions, 91 deletions
diff --git a/spec/controllers/projects/notes_controller_spec.rb b/spec/controllers/projects/notes_controller_spec.rb index 1458113b90c..81badaac76b 100644 --- a/spec/controllers/projects/notes_controller_spec.rb +++ b/spec/controllers/projects/notes_controller_spec.rb @@ -154,7 +154,7 @@ describe Projects::NotesController do get :index, request_params expect(parsed_response[:notes].count).to eq(1) - expect(note_json[:id]).to eq(note.id) + expect(note_json[:id]).to eq(note.id.to_s) end it 'does not result in N+1 queries' do diff --git a/spec/factories/resource_label_events.rb b/spec/factories/resource_label_events.rb index a67ad78c098..739ba901052 100644 --- a/spec/factories/resource_label_events.rb +++ b/spec/factories/resource_label_events.rb @@ -2,9 +2,12 @@ FactoryBot.define do factory :resource_label_event do - user { issue.project.creator } action :add label - issue + user { issuable&.author || create(:user) } + + after(:build) do |event, evaluator| + event.issue = create(:issue) unless event.issuable + end end end diff --git a/spec/features/issues/resource_label_events_spec.rb b/spec/features/issues/resource_label_events_spec.rb new file mode 100644 index 00000000000..40c452c991a --- /dev/null +++ b/spec/features/issues/resource_label_events_spec.rb @@ -0,0 +1,60 @@ +# frozen_string_literal: true + +require 'rails_helper' + +describe 'List issue resource label events', :js do + let(:user) { create(:user) } + let(:project) { create(:project, :public) } + let(:issue) { create(:issue, project: project, author: user) } + let!(:label) { create(:label, project: project, title: 'foo') } + + context 'when user displays the issue' do + let!(:note) { create(:note_on_issue, author: user, project: project, noteable: issue, note: 'some note') } + let!(:event) { create(:resource_label_event, user: user, issue: issue, label: label) } + + before do + visit project_issue_path(project, issue) + wait_for_requests + end + + it 'shows both notes and resource label events' do + page.within('#notes') do + expect(find("#note_#{note.id}")).to have_content 'some note' + expect(find("#note_#{event.discussion_id}")).to have_content 'added foo label' + end + end + end + + context 'when user adds label to the issue' do + def toggle_labels(labels) + page.within '.labels' do + click_link 'Edit' + wait_for_requests + + labels.each { |label| click_link label } + + click_link 'Edit' + wait_for_requests + end + end + + before do + create(:label, project: project, title: 'bar') + project.add_developer(user) + + sign_in(user) + visit project_issue_path(project, issue) + wait_for_requests + end + + it 'shows add note for newly added labels' do + toggle_labels(%w(foo bar)) + visit project_issue_path(project, issue) + wait_for_requests + + page.within('#notes') do + expect(page).to have_content 'added bar foo labels' + end + end + end +end diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js index 41d0dfd8939..b29a22da7c2 100644 --- a/spec/javascripts/diffs/mock_data/diff_discussions.js +++ b/spec/javascripts/diffs/mock_data/diff_discussions.js @@ -16,7 +16,7 @@ export default { expanded: true, notes: [ { - id: 1749, + id: '1749', type: 'DiffNote', attachment: null, author: { @@ -68,7 +68,7 @@ export default { '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', }, { - id: 1753, + id: '1753', type: 'DiffNote', attachment: null, author: { @@ -120,7 +120,7 @@ export default { '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', }, { - id: 1754, + id: '1754', type: 'DiffNote', attachment: null, author: { @@ -162,7 +162,7 @@ export default { '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', }, { - id: 1755, + id: '1755', type: 'DiffNote', attachment: null, author: { @@ -204,7 +204,7 @@ export default { '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', }, { - id: 1756, + id: '1756', type: 'DiffNote', attachment: null, author: { diff --git a/spec/javascripts/notes/components/note_actions_spec.js b/spec/javascripts/notes/components/note_actions_spec.js index 52cc42cb53d..d7298cb3483 100644 --- a/spec/javascripts/notes/components/note_actions_spec.js +++ b/spec/javascripts/notes/components/note_actions_spec.js @@ -28,7 +28,7 @@ describe('issue_note_actions component', () => { canEdit: true, canAwardEmoji: true, canReportAsAbuse: true, - noteId: 539, + noteId: '539', noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', @@ -59,6 +59,20 @@ describe('issue_note_actions component', () => { expect(vm.$el.querySelector(`a[href="${props.reportAbusePath}"]`)).toBeDefined(); }); + it('should be possible to copy link to a note', () => { + expect(vm.$el.querySelector('.js-btn-copy-note-link')).not.toBeNull(); + }); + + it('should not show copy link action when `noteUrl` prop is empty', done => { + vm.noteUrl = ''; + Vue.nextTick() + .then(() => { + expect(vm.$el.querySelector('.js-btn-copy-note-link')).toBeNull(); + }) + .then(done) + .catch(done.fail); + }); + it('should be possible to delete comment', () => { expect(vm.$el.querySelector('.js-note-delete')).toBeDefined(); }); @@ -77,7 +91,7 @@ describe('issue_note_actions component', () => { canEdit: false, canAwardEmoji: false, canReportAsAbuse: false, - noteId: 539, + noteId: '539', noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', reportAbusePath: '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', diff --git a/spec/javascripts/notes/components/note_awards_list_spec.js b/spec/javascripts/notes/components/note_awards_list_spec.js index 9d98ba219da..6a6a810acff 100644 --- a/spec/javascripts/notes/components/note_awards_list_spec.js +++ b/spec/javascripts/notes/components/note_awards_list_spec.js @@ -30,7 +30,7 @@ describe('note_awards_list component', () => { propsData: { awards: awardsMock, noteAuthorId: 2, - noteId: 545, + noteId: '545', canAwardEmoji: true, toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', }, @@ -70,7 +70,7 @@ describe('note_awards_list component', () => { propsData: { awards: awardsMock, noteAuthorId: 2, - noteId: 545, + noteId: '545', canAwardEmoji: false, toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', }, diff --git a/spec/javascripts/notes/components/note_form_spec.js b/spec/javascripts/notes/components/note_form_spec.js index 95d400ab3df..147ffcf1b81 100644 --- a/spec/javascripts/notes/components/note_form_spec.js +++ b/spec/javascripts/notes/components/note_form_spec.js @@ -19,7 +19,7 @@ describe('issue_note_form component', () => { props = { isEditing: false, noteBody: 'Magni suscipit eius consectetur enim et ex et commodi.', - noteId: 545, + noteId: '545', }; vm = new Component({ @@ -32,6 +32,22 @@ describe('issue_note_form component', () => { vm.$destroy(); }); + describe('noteHash', () => { + it('returns note hash string based on `noteId`', () => { + expect(vm.noteHash).toBe(`#note_${props.noteId}`); + }); + + it('return note hash as `#` when `noteId` is empty', done => { + vm.noteId = ''; + Vue.nextTick() + .then(() => { + expect(vm.noteHash).toBe('#'); + }) + .then(done) + .catch(done.fail); + }); + }); + describe('conflicts editing', () => { it('should show conflict message if note changes outside the component', done => { vm.isEditing = true; diff --git a/spec/javascripts/notes/components/note_header_spec.js b/spec/javascripts/notes/components/note_header_spec.js index a3c6bf78988..379780f43a0 100644 --- a/spec/javascripts/notes/components/note_header_spec.js +++ b/spec/javascripts/notes/components/note_header_spec.js @@ -33,7 +33,7 @@ describe('note_header component', () => { }, createdAt: '2017-08-02T10:51:58.559Z', includeToggle: false, - noteId: 1394, + noteId: '1394', expanded: true, }, }).$mount(); @@ -47,6 +47,16 @@ describe('note_header component', () => { it('should render timestamp link', () => { expect(vm.$el.querySelector('a[href="#note_1394"]')).toBeDefined(); }); + + it('should not render user information when prop `author` is empty object', done => { + vm.author = {}; + Vue.nextTick() + .then(() => { + expect(vm.$el.querySelector('.note-header-author-name')).toBeNull(); + }) + .then(done) + .catch(done.fail); + }); }); describe('discussion', () => { @@ -66,7 +76,7 @@ describe('note_header component', () => { }, createdAt: '2017-08-02T10:51:58.559Z', includeToggle: true, - noteId: 1395, + noteId: '1395', expanded: true, }, }).$mount(); diff --git a/spec/javascripts/notes/mock_data.js b/spec/javascripts/notes/mock_data.js index 0423fcb6ec4..1f030e5af28 100644 --- a/spec/javascripts/notes/mock_data.js +++ b/spec/javascripts/notes/mock_data.js @@ -66,7 +66,7 @@ export const individualNote = { individual_note: true, notes: [ { - id: 1390, + id: '1390', attachment: { url: null, filename: null, @@ -111,7 +111,7 @@ export const individualNote = { }; export const note = { - id: 546, + id: '546', attachment: { url: null, filename: null, @@ -174,7 +174,7 @@ export const discussionMock = { expanded: true, notes: [ { - id: 1395, + id: '1395', attachment: { url: null, filename: null, @@ -211,7 +211,7 @@ export const discussionMock = { path: '/gitlab-org/gitlab-ce/notes/1395', }, { - id: 1396, + id: '1396', attachment: { url: null, filename: null, @@ -257,7 +257,7 @@ export const discussionMock = { path: '/gitlab-org/gitlab-ce/notes/1396', }, { - id: 1437, + id: '1437', attachment: { url: null, filename: null, @@ -308,7 +308,7 @@ export const discussionMock = { }; export const loggedOutnoteableData = { - id: 98, + id: '98', iid: 26, author_id: 1, description: '', @@ -358,7 +358,7 @@ export const collapseNotesMock = [ individual_note: true, notes: [ { - id: 1390, + id: '1390', attachment: null, author: { id: 1, @@ -393,7 +393,7 @@ export const collapseNotesMock = [ individual_note: true, notes: [ { - id: 1391, + id: '1391', attachment: null, author: { id: 1, @@ -433,7 +433,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { expanded: true, notes: [ { - id: 1390, + id: '1390', attachment: { url: null, filename: null, @@ -495,7 +495,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { expanded: true, notes: [ { - id: 1391, + id: '1391', attachment: { url: null, filename: null, @@ -544,7 +544,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { '/gitlab-org/gitlab-ce/notes/1471': { commands_changes: null, valid: true, - id: 1471, + id: '1471', attachment: null, author: { id: 1, @@ -600,7 +600,7 @@ export const DISCUSSION_NOTE_RESPONSE_MAP = { expanded: true, notes: [ { - id: 1471, + id: '1471', attachment: { url: null, filename: null, @@ -671,7 +671,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 901, + id: '901', type: null, attachment: null, author: { @@ -718,7 +718,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 902, + id: '902', type: null, attachment: null, author: { @@ -765,7 +765,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 903, + id: '903', type: null, attachment: null, author: { @@ -809,7 +809,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 904, + id: '904', type: null, attachment: null, author: { @@ -854,7 +854,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 905, + id: '905', type: null, attachment: null, author: { @@ -898,7 +898,7 @@ export const notesWithDescriptionChanges = [ expanded: true, notes: [ { - id: 906, + id: '906', type: null, attachment: null, author: { @@ -945,7 +945,7 @@ export const collapsedSystemNotes = [ expanded: true, notes: [ { - id: 901, + id: '901', type: null, attachment: null, author: { @@ -992,7 +992,7 @@ export const collapsedSystemNotes = [ expanded: true, notes: [ { - id: 902, + id: '902', type: null, attachment: null, author: { @@ -1039,7 +1039,7 @@ export const collapsedSystemNotes = [ expanded: true, notes: [ { - id: 904, + id: '904', type: null, attachment: null, author: { @@ -1084,7 +1084,7 @@ export const collapsedSystemNotes = [ expanded: true, notes: [ { - id: 905, + id: '905', type: null, attachment: null, author: { @@ -1129,7 +1129,7 @@ export const collapsedSystemNotes = [ expanded: true, notes: [ { - id: 906, + id: '906', type: null, attachment: null, author: { diff --git a/spec/javascripts/vue_shared/components/notes/system_note_spec.js b/spec/javascripts/vue_shared/components/notes/system_note_spec.js index 2a6015fe35f..adcb1c858aa 100644 --- a/spec/javascripts/vue_shared/components/notes/system_note_spec.js +++ b/spec/javascripts/vue_shared/components/notes/system_note_spec.js @@ -9,7 +9,7 @@ describe('system note component', () => { beforeEach(() => { props = { note: { - id: 1424, + id: '1424', author: { id: 1, name: 'Root', diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index 2412cc3010a..ec2bdbe22e1 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -324,3 +324,9 @@ metrics: - latest_closed_by - merged_by - pipeline +resource_label_events: +- user +- issue +- merge_request +- epic +- label diff --git a/spec/lib/gitlab/import_export/project.json b/spec/lib/gitlab/import_export/project.json index 1b7fa11cb3c..eefd00e7383 100644 --- a/spec/lib/gitlab/import_export/project.json +++ b/spec/lib/gitlab/import_export/project.json @@ -331,6 +331,28 @@ }, "events": [] } + ], + "resource_label_events": [ + { + "id":244, + "action":"remove", + "issue_id":40, + "merge_request_id":null, + "label_id":2, + "user_id":1, + "created_at":"2018-08-28T08:24:00.494Z", + "label": { + "id": 2, + "title": "test2", + "color": "#428bca", + "project_id": 8, + "created_at": "2016-07-22T08:55:44.161Z", + "updated_at": "2016-07-22T08:55:44.161Z", + "template": false, + "description": "", + "type": "ProjectLabel" + } + } ] }, { @@ -2515,6 +2537,17 @@ "events": [] } ], + "resource_label_events": [ + { + "id":243, + "action":"add", + "issue_id":null, + "merge_request_id":27, + "label_id":null, + "user_id":1, + "created_at":"2018-08-28T08:24:00.494Z" + } + ], "merge_request_diff": { "id": 27, "state": "collected", diff --git a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb index a88ac0a091e..3ff6be595a8 100644 --- a/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_restorer_spec.rb @@ -89,6 +89,14 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do expect(ProtectedTag.first.create_access_levels).not_to be_empty end + it 'restores issue resource label events' do + expect(Issue.find_by(title: 'Voluptatem').resource_label_events).not_to be_empty + end + + it 'restores merge requests resource label events' do + expect(MergeRequest.find_by(title: 'MR1').resource_label_events).not_to be_empty + end + context 'event at forth level of the tree' do let(:event) { Event.where(action: 6).first } diff --git a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb index fec8a2af9ab..5dc372263ad 100644 --- a/spec/lib/gitlab/import_export/project_tree_saver_spec.rb +++ b/spec/lib/gitlab/import_export/project_tree_saver_spec.rb @@ -169,6 +169,14 @@ describe Gitlab::ImportExport::ProjectTreeSaver do expect(priorities.flatten).not_to be_empty end + it 'has issue resource label events' do + expect(saved_project_json['issues'].first['resource_label_events']).not_to be_empty + end + + it 'has merge request resource label events' do + expect(saved_project_json['merge_requests'].first['resource_label_events']).not_to be_empty + end + it 'saves the correct service type' do expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService') end @@ -291,6 +299,9 @@ describe Gitlab::ImportExport::ProjectTreeSaver do project: project, commit_id: ci_build.pipeline.sha) + create(:resource_label_event, label: project_label, issue: issue) + create(:resource_label_event, label: group_label, merge_request: merge_request) + create(:event, :created, target: milestone, project: project, author: user) create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' }) diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 1be448b0486..e9f1be172b0 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -579,3 +579,11 @@ Badge: - type ProjectCiCdSetting: - group_runners_enabled +ResourceLabelEvent: +- id +- action +- issue_id +- merge_request_id +- label_id +- user_id +- created_at diff --git a/spec/models/label_note_spec.rb b/spec/models/label_note_spec.rb new file mode 100644 index 00000000000..f69874d94aa --- /dev/null +++ b/spec/models/label_note_spec.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe LabelNote do + set(:project) { create(:project, :repository) } + set(:user) { create(:user) } + set(:label) { create(:label, project: project) } + set(:label2) { create(:label, project: project) } + let(:resource_parent) { project } + + context 'when resource is issue' do + set(:resource) { create(:issue, project: project) } + + it_behaves_like 'label note created from events' + end + + context 'when resource is merge request' do + set(:resource) { create(:merge_request, source_project: project, target_project: project) } + + it_behaves_like 'label note created from events' + end +end diff --git a/spec/models/resource_label_event_spec.rb b/spec/models/resource_label_event_spec.rb index 4756caa1b97..da6e1b5610d 100644 --- a/spec/models/resource_label_event_spec.rb +++ b/spec/models/resource_label_event_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe ResourceLabelEvent, type: :model do - subject { build(:resource_label_event) } + subject { build(:resource_label_event, issue: issue) } let(:issue) { create(:issue) } let(:merge_request) { create(:merge_request) } @@ -16,8 +16,6 @@ RSpec.describe ResourceLabelEvent, type: :model do describe 'validations' do it { is_expected.to be_valid } - it { is_expected.to validate_presence_of(:label) } - it { is_expected.to validate_presence_of(:user) } describe 'Issuable validation' do it 'is invalid if issue_id and merge_request_id are missing' do @@ -45,4 +43,52 @@ RSpec.describe ResourceLabelEvent, type: :model do end end end + + describe '#expire_etag_cache' do + def expect_expiration(issue) + expect_any_instance_of(Gitlab::EtagCaching::Store) + .to receive(:touch) + .with("/#{issue.project.namespace.to_param}/#{issue.project.to_param}/noteable/issue/#{issue.id}/notes") + end + + it 'expires resource note etag cache on event save' do + expect_expiration(subject.issuable) + + subject.save! + end + + it 'expires resource note etag cache on event destroy' do + subject.save! + + expect_expiration(subject.issuable) + + subject.destroy! + end + end + + describe '#outdated_markdown?' do + it 'returns true if label is missing and reference is not empty' do + subject.attributes = { reference: 'ref', label_id: nil } + + expect(subject.outdated_markdown?).to be true + end + + it 'returns true if reference is not set yet' do + subject.attributes = { reference: nil } + + expect(subject.outdated_markdown?).to be true + end + + it 'returns true markdown is outdated' do + subject.attributes = { cached_markdown_version: 0 } + + expect(subject.outdated_markdown?).to be true + end + + it 'returns false if label and reference are set' do + subject.attributes = { reference: 'whatever', cached_markdown_version: CacheMarkdownField::CACHE_COMMONMARK_VERSION } + + expect(subject.outdated_markdown?).to be false + end + end end diff --git a/spec/requests/api/resource_label_events_spec.rb b/spec/requests/api/resource_label_events_spec.rb new file mode 100644 index 00000000000..b7d4a5152cc --- /dev/null +++ b/spec/requests/api/resource_label_events_spec.rb @@ -0,0 +1,75 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe API::ResourceLabelEvents do + set(:user) { create(:user) } + set(:project) { create(:project, :public, :repository, namespace: user.namespace) } + set(:private_user) { create(:user) } + + before do + project.add_developer(user) + end + + shared_examples 'resource_label_events API' do |parent_type, eventable_type, id_name| + describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events" do + it "returns an array of resource label events" do + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", user) + + expect(response).to have_gitlab_http_status(200) + expect(response).to include_pagination_headers + expect(json_response).to be_an Array + expect(json_response.first['id']).to eq(event.id) + end + + it "returns a 404 error when eventable id not found" do + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/12345/resource_label_events", user) + + expect(response).to have_gitlab_http_status(404) + end + + it "returns 404 when not authorized" do + parent.update!(visibility_level: Gitlab::VisibilityLevel::PRIVATE) + + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events", private_user) + + expect(response).to have_gitlab_http_status(404) + end + end + + describe "GET /#{parent_type}/:id/#{eventable_type}/:noteable_id/resource_label_events/:event_id" do + it "returns a resource label event by id" do + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/#{event.id}", user) + + expect(response).to have_gitlab_http_status(200) + expect(json_response['id']).to eq(event.id) + end + + it "returns a 404 error if resource label event not found" do + get api("/#{parent_type}/#{parent.id}/#{eventable_type}/#{eventable[id_name]}/resource_label_events/12345", user) + + expect(response).to have_gitlab_http_status(404) + end + end + end + + context 'when eventable is an Issue' do + let(:issue) { create(:issue, project: project, author: user) } + + it_behaves_like 'resource_label_events API', 'projects', 'issues', 'iid' do + let(:parent) { project } + let(:eventable) { issue } + let!(:event) { create(:resource_label_event, issue: issue) } + end + end + + context 'when eventable is a Merge Request' do + let(:merge_request) { create(:merge_request, source_project: project, target_project: project, author: user) } + + it_behaves_like 'resource_label_events API', 'projects', 'merge_requests', 'iid' do + let(:parent) { project } + let(:eventable) { merge_request } + let!(:event) { create(:resource_label_event, merge_request: merge_request) } + end + end +end diff --git a/spec/services/issuable/common_system_notes_service_spec.rb b/spec/services/issuable/common_system_notes_service_spec.rb index dcf4503ef9c..fa1a421d528 100644 --- a/spec/services/issuable/common_system_notes_service_spec.rb +++ b/spec/services/issuable/common_system_notes_service_spec.rb @@ -12,12 +12,21 @@ describe Issuable::CommonSystemNotesService do it_behaves_like 'system note creation', { time_estimate: 5 }, 'changed time estimate' context 'when new label is added' do + let(:label) { create(:label, project: project) } + before do - label = create(:label, project: project) issuable.labels << label + issuable.save end - it_behaves_like 'system note creation', {}, /added ~\w+ label/ + it 'creates a resource label event' do + described_class.new(project, user).execute(issuable, []) + event = issuable.reload.resource_label_events.last + + expect(event).not_to be_nil + expect(event.label_id).to eq label.id + expect(event.user_id).to eq user.id + end end context 'when new milestone is assigned' do diff --git a/spec/services/issues/move_service_spec.rb b/spec/services/issues/move_service_spec.rb index 609eef76d2c..b5767583952 100644 --- a/spec/services/issues/move_service_spec.rb +++ b/spec/services/issues/move_service_spec.rb @@ -122,6 +122,17 @@ describe Issues::MoveService do end end + context 'issue with resource label events' do + it 'assigns resource label events to new issue' do + old_issue.resource_label_events = create_list(:resource_label_event, 2, issue: old_issue) + + new_issue = move_service.execute(old_issue, new_project) + + expected = old_issue.resource_label_events.map(&:label_id) + expect(new_issue.resource_label_events.map(&:label_id)).to match_array(expected) + end + end + context 'generic issue' do include_context 'issue move executed' diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 5bcfef46b75..07aa8449a66 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -189,11 +189,12 @@ describe Issues::UpdateService, :mailer do expect(note.note).to include "assigned to #{user2.to_reference}" end - it 'creates system note about issue label edit' do - note = find_note('added ~') + it 'creates a resource label event' do + event = issue.resource_label_events.last - expect(note).not_to be_nil - expect(note.note).to include "added #{label.to_reference} label" + expect(event).not_to be_nil + expect(event.label_id).to eq label.id + expect(event.user_id).to eq user.id end it 'creates system note about title change' do diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index f0029af83cc..55dfab81c26 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -109,11 +109,12 @@ describe MergeRequests::UpdateService, :mailer do expect(note.note).to include "assigned to #{user2.to_reference}" end - it 'creates system note about merge_request label edit' do - note = find_note('added ~') + it 'creates a resource label event' do + event = merge_request.resource_label_events.last - expect(note).not_to be_nil - expect(note.note).to include "added #{label.to_reference} label" + expect(event).not_to be_nil + expect(event.label_id).to eq label.id + expect(event.user_id).to eq user.id end it 'creates system note about title change' do diff --git a/spec/services/resource_events/change_labels_service_spec.rb b/spec/services/resource_events/change_labels_service_spec.rb index 41b0fb3eea3..4c9138fb1ef 100644 --- a/spec/services/resource_events/change_labels_service_spec.rb +++ b/spec/services/resource_events/change_labels_service_spec.rb @@ -18,6 +18,14 @@ describe ResourceEvents::ChangeLabelsService do expect(event.action).to eq(action) end + it 'expires resource note etag cache' do + expect_any_instance_of(Gitlab::EtagCaching::Store) + .to receive(:touch) + .with("/#{resource.project.namespace.to_param}/#{resource.project.to_param}/noteable/issue/#{resource.id}/notes") + + described_class.new(resource, author).execute(added_labels: [labels[0]]) + end + context 'when adding a label' do let(:added) { [labels[0]] } let(:removed) { [] } diff --git a/spec/services/resource_events/merge_into_notes_service_spec.rb b/spec/services/resource_events/merge_into_notes_service_spec.rb new file mode 100644 index 00000000000..0d333d541c9 --- /dev/null +++ b/spec/services/resource_events/merge_into_notes_service_spec.rb @@ -0,0 +1,70 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe ResourceEvents::MergeIntoNotesService do + def create_event(params) + event_params = { action: :add, label: label, issue: resource, + user: user } + + create(:resource_label_event, event_params.merge(params)) + end + + def create_note(params) + opts = { noteable: resource, project: project } + + create(:note_on_issue, opts.merge(params)) + end + + set(:project) { create(:project) } + set(:user) { create(:user) } + set(:resource) { create(:issue, project: project) } + set(:label) { create(:label, project: project) } + set(:label2) { create(:label, project: project) } + let(:time) { Time.now } + + describe '#execute' do + it 'merges label events into notes in order of created_at' do + note1 = create_note(created_at: 4.days.ago) + note2 = create_note(created_at: 2.days.ago) + event1 = create_event(created_at: 3.days.ago) + event2 = create_event(created_at: 1.day.ago) + + notes = described_class.new(resource, user).execute([note1, note2]) + + expected = [note1, event1, note2, event2].map(&:discussion_id) + expect(notes.map(&:discussion_id)).to eq expected + end + + it 'squashes events with same time and author into single note' do + user2 = create(:user) + + create_event(created_at: time) + create_event(created_at: time, label: label2, action: :remove) + create_event(created_at: time, user: user2) + create_event(created_at: 1.day.ago, label: label2) + + notes = described_class.new(resource, user).execute() + + expected = [ + "added #{label.to_reference} label and removed #{label2.to_reference} label", + "added #{label.to_reference} label", + "added #{label2.to_reference} label" + ] + + expect(notes.count).to eq 3 + expect(notes.map(&:note)).to match_array expected + end + + it 'fetches only notes created after last_fetched_at' do + create_event(created_at: 4.days.ago) + event = create_event(created_at: 1.day.ago) + + notes = described_class.new(resource, user, + last_fetched_at: 2.days.ago.to_i).execute() + + expect(notes.count).to eq 1 + expect(notes.first.discussion_id).to eq event.discussion_id + end + end +end diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 48aad8ebdbe..d5d750e182b 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -197,45 +197,6 @@ describe SystemNoteService do end end - describe '.change_label' do - subject { described_class.change_label(noteable, project, author, added, removed) } - - let(:labels) { create_list(:label, 2, project: project) } - let(:added) { [] } - let(:removed) { [] } - - it_behaves_like 'a system note' do - let(:action) { 'label' } - end - - context 'with added labels' do - let(:added) { labels } - let(:removed) { [] } - - it 'sets the note text' do - expect(subject.note).to eq "added ~#{labels[0].id} ~#{labels[1].id} labels" - end - end - - context 'with removed labels' do - let(:added) { [] } - let(:removed) { labels } - - it 'sets the note text' do - expect(subject.note).to eq "removed ~#{labels[0].id} ~#{labels[1].id} labels" - end - end - - context 'with added and removed labels' do - let(:added) { [labels[0]] } - let(:removed) { [labels[1]] } - - it 'sets the note text' do - expect(subject.note).to eq "added ~#{labels[0].id} and removed ~#{labels[1].id} labels" - end - end - end - describe '.change_milestone' do context 'for a project milestone' do subject { described_class.change_milestone(noteable, project, author, milestone) } diff --git a/spec/support/shared_examples/models/label_note_shared_examples.rb b/spec/support/shared_examples/models/label_note_shared_examples.rb new file mode 100644 index 00000000000..5803b3af74b --- /dev/null +++ b/spec/support/shared_examples/models/label_note_shared_examples.rb @@ -0,0 +1,109 @@ +# frozen_string_literal: true + +shared_examples 'label note created from events' do + def create_event(params = {}) + event_params = { action: :add, label: label, user: user } + resource_key = resource.class.name.underscore.to_s + event_params[resource_key] = resource + + build(:resource_label_event, event_params.merge(params)) + end + + def label_refs(events) + sorted_labels = events.map(&:label).compact.sort_by(&:title) + + sorted_labels.map { |l| l.to_reference}.join(' ') + end + + let(:time) { Time.now } + let(:local_label_ids) { [label.id, label2.id] } + + describe '.from_events' do + it 'returns system note with expected attributes' do + event = create_event + + note = described_class.from_events([event, create_event]) + + expect(note.system).to be true + expect(note.author_id).to eq event.user_id + expect(note.discussion_id).to eq event.discussion_id + expect(note.noteable).to eq event.issuable + expect(note.note).to be_present + expect(note.note_html).to be_present + end + + it 'updates markdown cache if reference is not set yet' do + event = create_event(reference: nil) + + described_class.from_events([event]) + + expect(event.reference).not_to be_nil + end + + it 'updates markdown cache if label was deleted' do + event = create_event(reference: 'some_ref', label: nil) + + described_class.from_events([event]) + + expect(event.reference).to eq '' + end + + it 'returns html note' do + events = [create_event(created_at: time)] + + note = described_class.from_events(events) + + expect(note.note_html).to include label.title + end + + it 'returns text note for added labels' do + events = [create_event(created_at: time), + create_event(created_at: time, label: label2), + create_event(created_at: time, label: nil)] + + note = described_class.from_events(events) + + expect(note.note).to eq "added #{label_refs(events)} + 1 deleted label" + end + + it 'returns text note for removed labels' do + events = [create_event(action: :remove, created_at: time), + create_event(action: :remove, created_at: time, label: label2), + create_event(action: :remove, created_at: time, label: nil)] + + note = described_class.from_events(events) + + expect(note.note).to eq "removed #{label_refs(events)} + 1 deleted label" + end + + it 'returns text note for added and removed labels' do + add_events = [create_event(created_at: time), + create_event(created_at: time, label: nil)] + + remove_events = [create_event(action: :remove, created_at: time), + create_event(action: :remove, created_at: time, label: nil)] + + note = described_class.from_events(add_events + remove_events) + + expect(note.note).to eq "added #{label_refs(add_events)} + 1 deleted label and removed #{label_refs(remove_events)} + 1 deleted label" + end + + it 'returns text note for cross-project label' do + other_label = create(:label) + event = create_event(label: other_label) + + note = described_class.from_events([event]) + + expect(note.note).to eq "added #{other_label.to_reference(resource_parent)} label" + end + + it 'returns text note for cross-group label' do + other_label = create(:group_label) + event = create_event(label: other_label) + + note = described_class.from_events([event]) + + expect(note.note).to eq "added #{other_label.to_reference(other_label.group, target_project: project, full: true)} label" + end + end +end |