summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJan Provaznik <jprovaznik@gitlab.com>2018-09-07 13:39:20 +0000
committerJan Provaznik <jprovaznik@gitlab.com>2018-09-07 13:39:20 +0000
commitd95c1f0335f7309114fcbb0d5413b28e1701a640 (patch)
tree6b22580a79dd1f929aecd158c31706ce3870c39b
parent81f4dc059db91577f72134e6008680b72029a29e (diff)
downloadgitlab-ce-d95c1f0335f7309114fcbb0d5413b28e1701a640.tar.gz
Use ResourceLabelEvent for tracking label changes
-rw-r--r--app/assets/javascripts/notes/components/note_actions.vue9
-rw-r--r--app/assets/javascripts/notes/components/note_awards_list.vue2
-rw-r--r--app/assets/javascripts/notes/components/note_form.vue9
-rw-r--r--app/assets/javascripts/notes/components/note_header.vue13
-rw-r--r--app/controllers/concerns/issuable_actions.rb1
-rw-r--r--app/controllers/concerns/notes_actions.rb1
-rw-r--r--app/helpers/notes_helper.rb2
-rw-r--r--app/models/concerns/issuable.rb4
-rw-r--r--app/models/concerns/noteable.rb19
-rw-r--r--app/models/label_note.rb97
-rw-r--r--app/models/note.rb13
-rw-r--r--app/models/resource_label_event.rb101
-rw-r--r--app/serializers/note_entity.rb10
-rw-r--r--app/serializers/project_note_entity.rb4
-rw-r--r--app/services/issuable/common_system_notes_service.rb4
-rw-r--r--app/services/issues/move_service.rb13
-rw-r--r--app/services/labels/promote_service.rb7
-rw-r--r--app/services/resource_events/change_labels_service.rb3
-rw-r--r--app/services/resource_events/merge_into_notes_service.rb55
-rw-r--r--app/services/system_note_service.rb41
-rw-r--r--changelogs/unreleased/label-event.yml6
-rw-r--r--db/migrate/20180901200537_add_resource_label_event_reference_fields.rb11
-rw-r--r--db/schema.rb5
-rw-r--r--doc/api/README.md1
-rw-r--r--doc/api/resource_label_events.md175
-rw-r--r--lib/api/api.rb1
-rw-r--r--lib/api/entities.rb14
-rw-r--r--lib/api/resource_label_events.rb52
-rw-r--r--lib/banzai/pipeline/label_pipeline.rb14
-rw-r--r--lib/gitlab/import_export/import_export.yml10
-rw-r--r--locale/gitlab.pot3
-rw-r--r--spec/controllers/projects/notes_controller_spec.rb2
-rw-r--r--spec/factories/resource_label_events.rb7
-rw-r--r--spec/features/issues/resource_label_events_spec.rb60
-rw-r--r--spec/javascripts/diffs/mock_data/diff_discussions.js10
-rw-r--r--spec/javascripts/notes/components/note_actions_spec.js18
-rw-r--r--spec/javascripts/notes/components/note_awards_list_spec.js4
-rw-r--r--spec/javascripts/notes/components/note_form_spec.js18
-rw-r--r--spec/javascripts/notes/components/note_header_spec.js14
-rw-r--r--spec/javascripts/notes/mock_data.js46
-rw-r--r--spec/javascripts/vue_shared/components/notes/system_note_spec.js2
-rw-r--r--spec/lib/gitlab/import_export/all_models.yml6
-rw-r--r--spec/lib/gitlab/import_export/project.json33
-rw-r--r--spec/lib/gitlab/import_export/project_tree_restorer_spec.rb8
-rw-r--r--spec/lib/gitlab/import_export/project_tree_saver_spec.rb11
-rw-r--r--spec/lib/gitlab/import_export/safe_model_attributes.yml8
-rw-r--r--spec/models/label_note_spec.rb23
-rw-r--r--spec/models/resource_label_event_spec.rb52
-rw-r--r--spec/requests/api/resource_label_events_spec.rb75
-rw-r--r--spec/services/issuable/common_system_notes_service_spec.rb13
-rw-r--r--spec/services/issues/move_service_spec.rb11
-rw-r--r--spec/services/issues/update_service_spec.rb9
-rw-r--r--spec/services/merge_requests/update_service_spec.rb9
-rw-r--r--spec/services/resource_events/change_labels_service_spec.rb8
-rw-r--r--spec/services/resource_events/merge_into_notes_service_spec.rb70
-rw-r--r--spec/services/system_note_service_spec.rb39
-rw-r--r--spec/support/shared_examples/models/label_note_shared_examples.rb109
57 files changed, 1191 insertions, 174 deletions
diff --git a/app/assets/javascripts/notes/components/note_actions.vue b/app/assets/javascripts/notes/components/note_actions.vue
index cdbbb342331..87fc002fcbc 100644
--- a/app/assets/javascripts/notes/components/note_actions.vue
+++ b/app/assets/javascripts/notes/components/note_actions.vue
@@ -24,12 +24,13 @@ export default {
required: true,
},
noteId: {
- type: Number,
+ type: String,
required: true,
},
noteUrl: {
type: String,
- required: true,
+ required: false,
+ default: '',
},
accessLevel: {
type: String,
@@ -225,11 +226,11 @@ export default {
Report as abuse
</a>
</li>
- <li>
+ <li v-if="noteUrl">
<button
:data-clipboard-text="noteUrl"
type="button"
- css-class="btn-default btn-transparent"
+ class="btn-default btn-transparent js-btn-copy-note-link"
>
Copy link
</button>
diff --git a/app/assets/javascripts/notes/components/note_awards_list.vue b/app/assets/javascripts/notes/components/note_awards_list.vue
index e111d3b9ac2..051b17e9aa9 100644
--- a/app/assets/javascripts/notes/components/note_awards_list.vue
+++ b/app/assets/javascripts/notes/components/note_awards_list.vue
@@ -25,7 +25,7 @@ export default {
required: true,
},
noteId: {
- type: Number,
+ type: String,
required: true,
},
canAwardEmoji: {
diff --git a/app/assets/javascripts/notes/components/note_form.vue b/app/assets/javascripts/notes/components/note_form.vue
index abcd4422d7c..c41ed070383 100644
--- a/app/assets/javascripts/notes/components/note_form.vue
+++ b/app/assets/javascripts/notes/components/note_form.vue
@@ -20,9 +20,9 @@ export default {
default: '',
},
noteId: {
- type: Number,
+ type: String,
required: false,
- default: 0,
+ default: '',
},
markdownVersion: {
type: Number,
@@ -67,7 +67,10 @@ export default {
'getUserDataByProp',
]),
noteHash() {
- return `#note_${this.noteId}`;
+ if (this.noteId) {
+ return `#note_${this.noteId}`;
+ }
+ return '#';
},
markdownPreviewPath() {
return this.getNoteableDataByProp('preview_note_path');
diff --git a/app/assets/javascripts/notes/components/note_header.vue b/app/assets/javascripts/notes/components/note_header.vue
index a621418cf72..d669d12a39b 100644
--- a/app/assets/javascripts/notes/components/note_header.vue
+++ b/app/assets/javascripts/notes/components/note_header.vue
@@ -9,7 +9,8 @@ export default {
props: {
author: {
type: Object,
- required: true,
+ required: false,
+ default: () => ({}),
},
createdAt: {
type: String,
@@ -21,7 +22,7 @@ export default {
default: '',
},
noteId: {
- type: Number,
+ type: String,
required: true,
},
includeToggle: {
@@ -72,7 +73,10 @@ export default {
{{ __('Toggle discussion') }}
</button>
</div>
- <a :href="author.path">
+ <a
+ v-if="Object.keys(author).length"
+ :href="author.path"
+ >
<span class="note-header-author-name">{{ author.name }}</span>
<span
v-if="author.status_tooltip_html"
@@ -81,6 +85,9 @@ export default {
@{{ author.username }}
</span>
</a>
+ <span v-else>
+ {{ __('A deleted user') }}
+ </span>
<span class="note-headline-light">
<span class="note-headline-meta">
<template v-if="actionText">
diff --git a/app/controllers/concerns/issuable_actions.rb b/app/controllers/concerns/issuable_actions.rb
index 37e03d70b6f..7b6e5bcb5f1 100644
--- a/app/controllers/concerns/issuable_actions.rb
+++ b/app/controllers/concerns/issuable_actions.rb
@@ -95,6 +95,7 @@ module IssuableActions
.includes(:noteable)
.fresh
+ notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes)
notes = prepare_notes_for_rendering(notes)
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
diff --git a/app/controllers/concerns/notes_actions.rb b/app/controllers/concerns/notes_actions.rb
index 5127db3f5fb..b63f2eb85f0 100644
--- a/app/controllers/concerns/notes_actions.rb
+++ b/app/controllers/concerns/notes_actions.rb
@@ -18,6 +18,7 @@ module NotesActions
notes = notes_finder.execute
.inc_relations_for_view
+ notes = ResourceEvents::MergeIntoNotesService.new(noteable, current_user, last_fetched_at: current_fetched_at).execute(notes)
notes = prepare_notes_for_rendering(notes)
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
diff --git a/app/helpers/notes_helper.rb b/app/helpers/notes_helper.rb
index 5404ead44f3..6285b43f917 100644
--- a/app/helpers/notes_helper.rb
+++ b/app/helpers/notes_helper.rb
@@ -108,7 +108,7 @@ module NotesHelper
end
def noteable_note_url(note)
- Gitlab::UrlBuilder.build(note)
+ Gitlab::UrlBuilder.build(note) if note.id
end
def form_resources
diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb
index 7f14d78e976..5f65fceb7af 100644
--- a/app/models/concerns/issuable.rb
+++ b/app/models/concerns/issuable.rb
@@ -109,10 +109,6 @@ module Issuable
false
end
- def etag_caching_enabled?
- false
- end
-
def has_multiple_assignees?
assignees.count > 1
end
diff --git a/app/models/concerns/noteable.rb b/app/models/concerns/noteable.rb
index ce778eae271..098eed137ba 100644
--- a/app/models/concerns/noteable.rb
+++ b/app/models/concerns/noteable.rb
@@ -82,4 +82,23 @@ module Noteable
def lockable?
[MergeRequest, Issue].include?(self.class)
end
+
+ def etag_caching_enabled?
+ false
+ end
+
+ def expire_note_etag_cache
+ return unless discussions_rendered_on_frontend?
+ return unless etag_caching_enabled?
+
+ Gitlab::EtagCaching::Store.new.touch(note_etag_key)
+ end
+
+ def note_etag_key
+ Gitlab::Routing.url_helpers.project_noteable_notes_path(
+ project,
+ target_type: self.class.name.underscore,
+ target_id: id
+ )
+ end
end
diff --git a/app/models/label_note.rb b/app/models/label_note.rb
new file mode 100644
index 00000000000..680952cf421
--- /dev/null
+++ b/app/models/label_note.rb
@@ -0,0 +1,97 @@
+# frozen_string_literal: true
+
+class LabelNote < Note
+ attr_accessor :resource_parent
+ attr_reader :events
+
+ def self.from_events(events, resource: nil, resource_parent: nil)
+ resource ||= events.first.issuable
+
+ attrs = {
+ system: true,
+ author: events.first.user,
+ created_at: events.first.created_at,
+ discussion_id: events.first.discussion_id,
+ noteable: resource,
+ system_note_metadata: SystemNoteMetadata.new(action: 'label'),
+ events: events,
+ resource_parent: resource_parent
+ }
+
+ if resource_parent.is_a?(Project)
+ attrs[:project_id] = resource_parent.id
+ end
+
+ LabelNote.new(attrs)
+ end
+
+ def events=(events)
+ @events = events
+
+ update_outdated_markdown
+ end
+
+ def cached_html_up_to_date?(markdown_field)
+ true
+ end
+
+ def note
+ @note ||= note_text
+ end
+
+ def note_html
+ @note_html ||= "<p dir=\"auto\">#{note_text(html: true)}</p>"
+ end
+
+ def project
+ resource_parent if resource_parent.is_a?(Project)
+ end
+
+ def group
+ resource_parent if resource_parent.is_a?(Group)
+ end
+
+ private
+
+ def update_outdated_markdown
+ events.each do |event|
+ if event.outdated_markdown?
+ event.refresh_invalid_reference
+ end
+ end
+ end
+
+ def note_text(html: false)
+ added = labels_str('added', label_refs_by_action('add', html))
+ removed = labels_str('removed', label_refs_by_action('remove', html))
+
+ [added, removed].compact.join(' and ')
+ end
+
+ # returns string containing added/removed labels including
+ # count of deleted labels:
+ #
+ # added ~1 ~2 + 1 deleted label
+ # added 3 deleted labels
+ # added ~1 ~2 labels
+ def labels_str(prefix, label_refs)
+ existing_refs = label_refs.select { |ref| ref.present? }.sort
+ refs_str = existing_refs.empty? ? nil : existing_refs.join(' ')
+
+ deleted = label_refs.count - existing_refs.count
+ deleted_str = deleted == 0 ? nil : "#{deleted} deleted"
+
+ return nil unless refs_str || deleted_str
+
+ label_list_str = [refs_str, deleted_str].compact.join(' + ')
+ suffix = 'label'.pluralize(deleted > 0 ? deleted : existing_refs.count)
+
+ "#{prefix} #{label_list_str} #{suffix}"
+ end
+
+ def label_refs_by_action(action, html)
+ field = html ? :reference_html : :reference
+
+ events.select { |e| e.action == action }.map(&field)
+ end
+end
diff --git a/app/models/note.rb b/app/models/note.rb
index 2e343b8f9f8..8f090cc31e6 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -389,18 +389,7 @@ class Note < ActiveRecord::Base
end
def expire_etag_cache
- return unless noteable&.discussions_rendered_on_frontend?
- return unless noteable&.etag_caching_enabled?
-
- Gitlab::EtagCaching::Store.new.touch(etag_key)
- end
-
- def etag_key
- Gitlab::Routing.url_helpers.project_noteable_notes_path(
- project,
- target_type: noteable_type.underscore,
- target_id: noteable_id
- )
+ noteable&.expire_note_etag_cache
end
def touch(*args)
diff --git a/app/models/resource_label_event.rb b/app/models/resource_label_event.rb
index 42c255fcd1e..3fd96b9dc18 100644
--- a/app/models/resource_label_event.rb
+++ b/app/models/resource_label_event.rb
@@ -3,33 +3,122 @@
# This model is not used yet, it will be used for:
# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483
class ResourceLabelEvent < ActiveRecord::Base
+ include Importable
+ include Gitlab::Utils::StrongMemoize
+ include CacheMarkdownField
+
+ cache_markdown_field :reference
+
belongs_to :user
belongs_to :issue
belongs_to :merge_request
belongs_to :label
- validates :user, presence: true, on: :create
- validates :label, presence: true, on: :create
+ scope :created_after, ->(time) { where('created_at > ?', time) }
+
+ validates :user, presence: { unless: :importing? }, on: :create
+ validates :label, presence: { unless: :importing? }, on: :create
validate :exactly_one_issuable
+ after_save :expire_etag_cache
+ after_destroy :expire_etag_cache
+
enum action: {
add: 1,
remove: 2
}
- def self.issuable_columns
- %i(issue_id merge_request_id).freeze
+ def self.issuable_attrs
+ %i(issue merge_request).freeze
end
def issuable
issue || merge_request
end
+ # create same discussion id for all actions with the same user and time
+ def discussion_id(resource = nil)
+ strong_memoize(:discussion_id) do
+ Digest::SHA1.hexdigest([self.class.name, created_at, user_id].join("-"))
+ end
+ end
+
+ def project
+ issuable.project
+ end
+
+ def group
+ issuable.group if issuable.respond_to?(:group)
+ end
+
+ def outdated_markdown?
+ return true if label_id.nil? && reference.present?
+
+ reference.nil? || latest_cached_markdown_version != cached_markdown_version
+ end
+
+ def banzai_render_context(field)
+ super.merge(pipeline: 'label', only_path: true)
+ end
+
+ def refresh_invalid_reference
+ # label_id could be nullified on label delete
+ self.reference = '' if label_id.nil?
+
+ # reference is not set for events which were not rendered yet
+ self.reference ||= label_reference
+
+ if changed?
+ save
+ elsif invalidated_markdown_cache?
+ refresh_markdown_cache!
+ end
+ end
+
private
+ def label_reference
+ if local_label?
+ label.to_reference(format: :id)
+ elsif label.is_a?(GroupLabel)
+ label.to_reference(label.group, target_project: resource_parent, format: :id)
+ else
+ label.to_reference(resource_parent, format: :id)
+ end
+ end
+
def exactly_one_issuable
- if self.class.issuable_columns.count { |attr| self[attr] } != 1
- errors.add(:base, "Exactly one of #{self.class.issuable_columns.join(', ')} is required")
+ issuable_count = self.class.issuable_attrs.count { |attr| self["#{attr}_id"] }
+
+ return true if issuable_count == 1
+
+ # if none of issuable IDs is set, check explicitly if nested issuable
+ # object is set, this is used during project import
+ if issuable_count == 0 && importing?
+ issuable_count = self.class.issuable_attrs.count { |attr| self.public_send(attr) } # rubocop:disable GitlabSecurity/PublicSend
+
+ return true if issuable_count == 1
end
+
+ errors.add(:base, "Exactly one of #{self.class.issuable_attrs.join(', ')} is required")
+ end
+
+ def expire_etag_cache
+ issuable.expire_note_etag_cache
+ end
+
+ def local_label?
+ params = { include_ancestor_groups: true }
+ if resource_parent.is_a?(Project)
+ params[:project_id] = resource_parent.id
+ else
+ params[:group_id] = resource_parent.id
+ end
+
+ LabelsFinder.new(nil, params).execute(skip_authorization: true).where(id: label.id).any?
+ end
+
+ def resource_parent
+ issuable.project || issuable.group
end
end
diff --git a/app/serializers/note_entity.rb b/app/serializers/note_entity.rb
index daa5c24d0f5..c6d27817411 100644
--- a/app/serializers/note_entity.rb
+++ b/app/serializers/note_entity.rb
@@ -4,6 +4,12 @@ class NoteEntity < API::Entities::Note
include RequestAwareEntity
include NotesHelper
+ expose :id do |note|
+ # resource events are represented as notes too, but don't
+ # have ID, discussion ID is used for them instead
+ note.id ? note.id.to_s : note.discussion_id
+ end
+
expose :type
expose :author, using: NoteUserEntity
@@ -46,8 +52,8 @@ class NoteEntity < API::Entities::Note
expose :emoji_awardable?, as: :emoji_awardable
expose :award_emoji, if: -> (note, _) { note.emoji_awardable? }, using: AwardEmojiEntity
- expose :report_abuse_path do |note|
- new_abuse_report_path(user_id: note.author.id, ref_url: Gitlab::UrlBuilder.build(note))
+ expose :report_abuse_path, if: -> (note, _) { note.author_id } do |note|
+ new_abuse_report_path(user_id: note.author_id, ref_url: Gitlab::UrlBuilder.build(note))
end
expose :noteable_note_url do |note|
diff --git a/app/serializers/project_note_entity.rb b/app/serializers/project_note_entity.rb
index d7c4d0aacc6..f6cdea1d8b5 100644
--- a/app/serializers/project_note_entity.rb
+++ b/app/serializers/project_note_entity.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true
class ProjectNoteEntity < NoteEntity
- expose :human_access do |note|
+ expose :human_access, if: -> (note, _) { note.project.present? } do |note|
note.project.team.human_max_access(note.author_id)
end
@@ -9,7 +9,7 @@ class ProjectNoteEntity < NoteEntity
toggle_award_emoji_project_note_path(note.project, note.id)
end
- expose :path do |note|
+ expose :path, if: -> (note, _) { note.id } do |note|
project_note_path(note.project, note)
end
diff --git a/app/services/issuable/common_system_notes_service.rb b/app/services/issuable/common_system_notes_service.rb
index 028b350ca07..ab53c38aa3a 100644
--- a/app/services/issuable/common_system_notes_service.rb
+++ b/app/services/issuable/common_system_notes_service.rb
@@ -55,7 +55,9 @@ module Issuable
added_labels = issuable.labels - old_labels
removed_labels = old_labels - issuable.labels
- SystemNoteService.change_label(issuable, issuable.project, current_user, added_labels, removed_labels)
+ ResourceEvents::ChangeLabelsService
+ .new(issuable, current_user)
+ .execute(added_labels: added_labels, removed_labels: removed_labels)
end
def create_title_change_note(old_title)
diff --git a/app/services/issues/move_service.rb b/app/services/issues/move_service.rb
index 841bce9949e..c52aa577dd8 100644
--- a/app/services/issues/move_service.rb
+++ b/app/services/issues/move_service.rb
@@ -36,6 +36,7 @@ module Issues
def update_new_issue
rewrite_notes
+ copy_resource_label_events
rewrite_issue_award_emoji
add_note_moved_from
end
@@ -96,6 +97,18 @@ module Issues
end
end
+ def copy_resource_label_events
+ @old_issue.resource_label_events.find_in_batches do |batch|
+ events = batch.map do |event|
+ event.attributes
+ .except('id', 'reference', 'reference_html')
+ .merge('issue_id' => @new_issue.id, 'created_at' => event.created_at)
+ end
+
+ Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, events)
+ end
+ end
+
def rewrite_issue_award_emoji
rewrite_award_emoji(@old_issue, @new_issue)
end
diff --git a/app/services/labels/promote_service.rb b/app/services/labels/promote_service.rb
index 623a5f0950e..fcdcea2d0ea 100644
--- a/app/services/labels/promote_service.rb
+++ b/app/services/labels/promote_service.rb
@@ -13,6 +13,7 @@ module Labels
label_ids_for_merge(new_label).find_in_batches(batch_size: BATCH_SIZE) do |batched_ids|
update_issuables(new_label, batched_ids)
+ update_resource_label_events(new_label, batched_ids)
update_issue_board_lists(new_label, batched_ids)
update_priorities(new_label, batched_ids)
subscribe_users(new_label, batched_ids)
@@ -52,6 +53,12 @@ module Labels
.update_all(label_id: new_label)
end
+ def update_resource_label_events(new_label, label_ids)
+ ResourceLabelEvent
+ .where(label: label_ids)
+ .update_all(label_id: new_label)
+ end
+
def update_issue_board_lists(new_label, label_ids)
List
.where(label: label_ids)
diff --git a/app/services/resource_events/change_labels_service.rb b/app/services/resource_events/change_labels_service.rb
index 8edb0ddb3ed..039d6e2ebad 100644
--- a/app/services/resource_events/change_labels_service.rb
+++ b/app/services/resource_events/change_labels_service.rb
@@ -1,7 +1,5 @@
# frozen_string_literal: true
-# This service is not used yet, it will be used for:
-# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483
module ResourceEvents
class ChangeLabelsService
attr_reader :resource, :user
@@ -25,6 +23,7 @@ module ResourceEvents
end
Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, labels)
+ resource.expire_note_etag_cache
end
private
diff --git a/app/services/resource_events/merge_into_notes_service.rb b/app/services/resource_events/merge_into_notes_service.rb
new file mode 100644
index 00000000000..1b02a1602e2
--- /dev/null
+++ b/app/services/resource_events/merge_into_notes_service.rb
@@ -0,0 +1,55 @@
+# frozen_string_literal: true
+
+# We store events about issuable label changes in a separate table (not as
+# other system notes), but we still want to display notes about label changes
+# as classic system notes in UI. This service generates "synthetic" notes for
+# label event changes and merges them with classic notes and sorts them by
+# creation time.
+
+module ResourceEvents
+ class MergeIntoNotesService
+ include Gitlab::Utils::StrongMemoize
+
+ attr_reader :resource, :current_user, :params
+
+ def initialize(resource, current_user, params = {})
+ @resource = resource
+ @current_user = current_user
+ @params = params
+ end
+
+ def execute(notes = [])
+ (notes + label_notes).sort_by { |n| n.created_at }
+ end
+
+ private
+
+ def label_notes
+ label_events_by_discussion_id.map do |discussion_id, events|
+ LabelNote.from_events(events, resource: resource, resource_parent: resource_parent)
+ end
+ end
+
+ def label_events_by_discussion_id
+ return [] unless resource.respond_to?(:resource_label_events)
+
+ events = resource.resource_label_events.includes(:label, :user)
+ events = since_fetch_at(events)
+
+ events.group_by { |event| event.discussion_id }
+ end
+
+ def since_fetch_at(events)
+ return events unless params[:last_fetched_at].present?
+
+ last_fetched_at = Time.at(params.fetch(:last_fetched_at).to_i)
+ events.created_after(last_fetched_at - NotesFinder::FETCH_OVERLAP)
+ end
+
+ def resource_parent
+ strong_memoize(:resource_parent) do
+ resource.project || resource.group
+ end
+ end
+ end
+end
diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb
index dda89830179..3ea81445798 100644
--- a/app/services/system_note_service.rb
+++ b/app/services/system_note_service.rb
@@ -98,47 +98,6 @@ module SystemNoteService
create_note(NoteSummary.new(issue, project, author, body, action: 'assignee'))
end
- # Called when one or more labels on a Noteable are added and/or removed
- #
- # noteable - Noteable object
- # project - Project owning noteable
- # author - User performing the change
- # added_labels - Array of Labels added
- # removed_labels - Array of Labels removed
- #
- # Example Note text:
- #
- # "added ~1 and removed ~2 ~3 labels"
- #
- # "added ~4 label"
- #
- # "removed ~5 label"
- #
- # Returns the created Note object
- def change_label(noteable, project, author, added_labels, removed_labels)
- labels_count = added_labels.count + removed_labels.count
-
- references = ->(label) { label.to_reference(format: :id) }
- added_labels = added_labels.map(&references).join(' ')
- removed_labels = removed_labels.map(&references).join(' ')
-
- text_parts = []
-
- if added_labels.present?
- text_parts << "added #{added_labels}"
- text_parts << 'and' if removed_labels.present?
- end
-
- if removed_labels.present?
- text_parts << "removed #{removed_labels}"
- end
-
- text_parts << 'label'.pluralize(labels_count)
- body = text_parts.join(' ')
-
- create_note(NoteSummary.new(noteable, project, author, body, action: 'label'))
- end
-
# Called when the milestone of a Noteable is changed
#
# noteable - Noteable object
diff --git a/changelogs/unreleased/label-event.yml b/changelogs/unreleased/label-event.yml
new file mode 100644
index 00000000000..e543abe5649
--- /dev/null
+++ b/changelogs/unreleased/label-event.yml
@@ -0,0 +1,6 @@
+---
+title: Use separate model for tracking resource label changes and render label system
+ notes based on data from this model.
+merge_request:
+author:
+type: added
diff --git a/db/migrate/20180901200537_add_resource_label_event_reference_fields.rb b/db/migrate/20180901200537_add_resource_label_event_reference_fields.rb
new file mode 100644
index 00000000000..264970ceed8
--- /dev/null
+++ b/db/migrate/20180901200537_add_resource_label_event_reference_fields.rb
@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+class AddResourceLabelEventReferenceFields < ActiveRecord::Migration
+ DOWNTIME = false
+
+ def change
+ add_column :resource_label_events, :cached_markdown_version, :integer
+ add_column :resource_label_events, :reference, :text
+ add_column :resource_label_events, :reference_html, :text
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 9ffaae207b8..e0b704f82d4 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -11,7 +11,7 @@
#
# It's strongly recommended that you check this file into your version control system.
-ActiveRecord::Schema.define(version: 20180901171833) do
+ActiveRecord::Schema.define(version: 20180901200537) do
# These are extensions that must be enabled in order to support this database
enable_extension "plpgsql"
@@ -1841,6 +1841,9 @@ ActiveRecord::Schema.define(version: 20180901171833) do
t.integer "label_id"
t.integer "user_id"
t.datetime_with_timezone "created_at", null: false
+ t.integer "cached_markdown_version"
+ t.text "reference"
+ t.text "reference_html"
end
add_index "resource_label_events", ["issue_id"], name: "index_resource_label_events_on_issue_id", using: :btree
diff --git a/doc/api/README.md b/doc/api/README.md
index e2a6e87a2c3..1738d4fae5c 100644
--- a/doc/api/README.md
+++ b/doc/api/README.md
@@ -40,6 +40,7 @@ following locations:
- [Namespaces](namespaces.md)
- [Notes](notes.md) (comments)
- [Discussions](discussions.md) (threaded comments)
+- [Resource Label Events](resource_label_events.md)
- [Notification settings](notification_settings.md)
- [Open source license templates](templates/licenses.md)
- [Pages Domains](pages_domains.md)
diff --git a/doc/api/resource_label_events.md b/doc/api/resource_label_events.md
new file mode 100644
index 00000000000..33e4821ccf4
--- /dev/null
+++ b/doc/api/resource_label_events.md
@@ -0,0 +1,175 @@
+# Resource label events API
+
+Resource label events keep track about who, when, and which label was added or removed to an issuable.
+
+## Issues
+
+### List project issue label events
+
+Gets a list of all label events for a single issue.
+
+```
+GET /projects/:id/issues/:issue_iid/resource_label_events
+```
+
+| Attribute | Type | Required | Description |
+| ------------------- | ---------------- | ---------- | ------------ |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `issue_iid` | integer | yes | The IID of an issue |
+
+```json
+[
+ {
+ "id": 142,
+ "user": {
+ "id": 1,
+ "name": "Administrator",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
+ "web_url": "http://gitlab.example.com/root"
+ },
+ "created_at": "2018-08-20T13:38:20.077Z",
+ "resource_type": "Issue",
+ "resource_id": 253,
+ "label": {
+ "id": 73,
+ "name": "a1",
+ "color": "#34495E",
+ "description": ""
+ },
+ "action": "add"
+ },
+ {
+ "id": 143,
+ "user": {
+ "id": 1,
+ "name": "Administrator",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
+ "web_url": "http://gitlab.example.com/root"
+ },
+ "created_at": "2018-08-20T13:38:20.077Z",
+ "resource_type": "Issue",
+ "resource_id": 253,
+ "label": {
+ "id": 74,
+ "name": "p1",
+ "color": "#0033CC",
+ "description": ""
+ },
+ "action": "remove"
+ }
+]
+```
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/resource_label_events
+```
+
+### Get single issue label event
+
+Returns a single label event for a specific project issue
+
+```
+GET /projects/:id/issues/:issue_iid/resource_label_events/:resource_label_event_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| --------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `issue_iid` | integer | yes | The IID of an issue |
+| `resource_label_event_id` | integer | yes | The ID of a label event |
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/issues/11/resource_label_events/1
+```
+
+## Merge requests
+
+### List project merge request label events
+
+Gets a list of all label events for a single merge request.
+
+```
+GET /projects/:id/merge_requests/:merge_request_iid/resource_label_events
+```
+
+| Attribute | Type | Required | Description |
+| ------------------- | ---------------- | ---------- | ------------ |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+
+```json
+[
+ {
+ "id": 119,
+ "user": {
+ "id": 1,
+ "name": "Administrator",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
+ "web_url": "http://gitlab.example.com/root"
+ },
+ "created_at": "2018-08-20T06:17:28.394Z",
+ "resource_type": "MergeRequest",
+ "resource_id": 28,
+ "label": {
+ "id": 74,
+ "name": "p1",
+ "color": "#0033CC",
+ "description": ""
+ },
+ "action": "add"
+ },
+ {
+ "id": 120,
+ "user": {
+ "id": 1,
+ "name": "Administrator",
+ "username": "root",
+ "state": "active",
+ "avatar_url": "https://www.gravatar.com/avatar/e64c7d89f26bd1972efa854d13d7dd61?s=80&d=identicon",
+ "web_url": "http://gitlab.example.com/root"
+ },
+ "created_at": "2018-08-20T06:17:28.394Z",
+ "resource_type": "MergeRequest",
+ "resource_id": 28,
+ "label": {
+ "id": 41,
+ "name": "project",
+ "color": "#D1D100",
+ "description": ""
+ },
+ "action": "add"
+ }
+]
+```
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/resource_label_events
+```
+
+### Get single merge request label event
+
+Returns a single label event for a specific project merge request
+
+```
+GET /projects/:id/merge_requests/:merge_request_iid/resource_label_events/:resource_label_event_id
+```
+
+Parameters:
+
+| Attribute | Type | Required | Description |
+| ------------------- | -------------- | -------- | ----------- |
+| `id` | integer/string | yes | The ID or [URL-encoded path of the project](README.md#namespaced-path-encoding) |
+| `merge_request_iid` | integer | yes | The IID of a merge request |
+| `resource_label_event_id` | integer | yes | The ID of a label event |
+
+```bash
+curl --request GET --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/projects/5/merge_requests/11/resource_label_events/120
+```
diff --git a/lib/api/api.rb b/lib/api/api.rb
index 843f75d3096..e89d9337853 100644
--- a/lib/api/api.rb
+++ b/lib/api/api.rb
@@ -118,6 +118,7 @@ module API
mount ::API::Namespaces
mount ::API::Notes
mount ::API::Discussions
+ mount ::API::ResourceLabelEvents
mount ::API::NotificationSettings
mount ::API::PagesDomains
mount ::API::Pipelines
diff --git a/lib/api/entities.rb b/lib/api/entities.rb
index 90abee94f6a..f0eafbaeb94 100644
--- a/lib/api/entities.rb
+++ b/lib/api/entities.rb
@@ -1437,5 +1437,19 @@ module API
badge.type == 'ProjectBadge' ? 'project' : 'group'
end
end
+
+ class ResourceLabelEvent < Grape::Entity
+ expose :id
+ expose :user, using: Entities::UserBasic
+ expose :created_at
+ expose :resource_type do |event, options|
+ event.issuable.class.name
+ end
+ expose :resource_id do |event, options|
+ event.issuable.id
+ end
+ expose :label, using: Entities::LabelBasic
+ expose :action
+ end
end
end
diff --git a/lib/api/resource_label_events.rb b/lib/api/resource_label_events.rb
new file mode 100644
index 00000000000..5ac3adeb990
--- /dev/null
+++ b/lib/api/resource_label_events.rb
@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+
+module API
+ class ResourceLabelEvents < Grape::API
+ include PaginationParams
+ helpers ::API::Helpers::NotesHelpers
+
+ before { authenticate! }
+
+ EVENTABLE_TYPES = [Issue, MergeRequest].freeze
+
+ EVENTABLE_TYPES.each do |eventable_type|
+ parent_type = eventable_type.parent_class.to_s.underscore
+ eventables_str = eventable_type.to_s.underscore.pluralize
+
+ params do
+ requires :id, type: String, desc: "The ID of a #{parent_type}"
+ end
+ resource parent_type.pluralize.to_sym, requirements: API::PROJECT_ENDPOINT_REQUIREMENTS do
+ desc "Get a list of #{eventable_type.to_s.downcase} resource label events" do
+ success Entities::ResourceLabelEvent
+ detail 'This feature was introduced in 11.3'
+ end
+ params do
+ requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable'
+ use :pagination
+ end
+ get ":id/#{eventables_str}/:eventable_id/resource_label_events" do
+ eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
+ events = eventable.resource_label_events.includes(:label, :user)
+
+ present paginate(events), with: Entities::ResourceLabelEvent
+ end
+
+ desc "Get a single #{eventable_type.to_s.downcase} resource label event" do
+ success Entities::ResourceLabelEvent
+ detail 'This feature was introduced in 11.3'
+ end
+ params do
+ requires :event_id, type: String, desc: 'The ID of a resource label event'
+ requires :eventable_id, types: [Integer, String], desc: 'The ID of the eventable'
+ end
+ get ":id/#{eventables_str}/:eventable_id/resource_label_events/:event_id" do
+ eventable = find_noteable(parent_type, eventables_str, params[:eventable_id])
+ event = eventable.resource_label_events.find(params[:event_id])
+
+ present event, with: Entities::ResourceLabelEvent
+ end
+ end
+ end
+ end
+end
diff --git a/lib/banzai/pipeline/label_pipeline.rb b/lib/banzai/pipeline/label_pipeline.rb
new file mode 100644
index 00000000000..725cccc4b2b
--- /dev/null
+++ b/lib/banzai/pipeline/label_pipeline.rb
@@ -0,0 +1,14 @@
+# frozen_string_literal: true
+
+module Banzai
+ module Pipeline
+ class LabelPipeline < BasePipeline
+ def self.filters
+ @filters ||= FilterArray[
+ Filter::SanitizationFilter,
+ Filter::LabelReferenceFilter
+ ]
+ end
+ end
+ end
+end
diff --git a/lib/gitlab/import_export/import_export.yml b/lib/gitlab/import_export/import_export.yml
index f030edc7d22..a19b3c88627 100644
--- a/lib/gitlab/import_export/import_export.yml
+++ b/lib/gitlab/import_export/import_export.yml
@@ -19,6 +19,9 @@ project_tree:
- milestone:
- events:
- :push_event_payload
+ - resource_label_events:
+ - label:
+ :priorities
- :issue_assignees
- snippets:
- :award_emoji
@@ -45,6 +48,9 @@ project_tree:
- milestone:
- events:
- :push_event_payload
+ - resource_label_events:
+ - label:
+ :priorities
- pipelines:
- notes:
- :author
@@ -137,6 +143,10 @@ excluded_attributes:
- :event_id
project_badges:
- :group_id
+ resource_label_events:
+ - :reference
+ - :reference_html
+ - :epic_id
methods:
labels:
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index a241e1bffef..491da5d9631 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -259,6 +259,9 @@ msgstr ""
msgid "A default branch cannot be chosen for an empty project."
msgstr ""
+msgid "A deleted user"
+msgstr ""
+
msgid "A new branch will be created in your fork and a new merge request will be started."
msgstr ""
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