Commit f4d3f81b authored by Jan Provaznik's avatar Jan Provaznik

Merge branch 'label-event' into 'master'

Use ResourceLabelEvent for tracking label changes

Closes #48483

See merge request gitlab-org/gitlab-ce!21281
parents 81f4dc05 d95c1f03
...@@ -24,12 +24,13 @@ export default { ...@@ -24,12 +24,13 @@ export default {
required: true, required: true,
}, },
noteId: { noteId: {
type: Number, type: String,
required: true, required: true,
}, },
noteUrl: { noteUrl: {
type: String, type: String,
required: true, required: false,
default: '',
}, },
accessLevel: { accessLevel: {
type: String, type: String,
...@@ -225,11 +226,11 @@ export default { ...@@ -225,11 +226,11 @@ export default {
Report as abuse Report as abuse
</a> </a>
</li> </li>
<li> <li v-if="noteUrl">
<button <button
:data-clipboard-text="noteUrl" :data-clipboard-text="noteUrl"
type="button" type="button"
css-class="btn-default btn-transparent" class="btn-default btn-transparent js-btn-copy-note-link"
> >
Copy link Copy link
</button> </button>
......
...@@ -25,7 +25,7 @@ export default { ...@@ -25,7 +25,7 @@ export default {
required: true, required: true,
}, },
noteId: { noteId: {
type: Number, type: String,
required: true, required: true,
}, },
canAwardEmoji: { canAwardEmoji: {
......
...@@ -20,9 +20,9 @@ export default { ...@@ -20,9 +20,9 @@ export default {
default: '', default: '',
}, },
noteId: { noteId: {
type: Number, type: String,
required: false, required: false,
default: 0, default: '',
}, },
markdownVersion: { markdownVersion: {
type: Number, type: Number,
...@@ -67,7 +67,10 @@ export default { ...@@ -67,7 +67,10 @@ export default {
'getUserDataByProp', 'getUserDataByProp',
]), ]),
noteHash() { noteHash() {
return `#note_${this.noteId}`; if (this.noteId) {
return `#note_${this.noteId}`;
}
return '#';
}, },
markdownPreviewPath() { markdownPreviewPath() {
return this.getNoteableDataByProp('preview_note_path'); return this.getNoteableDataByProp('preview_note_path');
......
...@@ -9,7 +9,8 @@ export default { ...@@ -9,7 +9,8 @@ export default {
props: { props: {
author: { author: {
type: Object, type: Object,
required: true, required: false,
default: () => ({}),
}, },
createdAt: { createdAt: {
type: String, type: String,
...@@ -21,7 +22,7 @@ export default { ...@@ -21,7 +22,7 @@ export default {
default: '', default: '',
}, },
noteId: { noteId: {
type: Number, type: String,
required: true, required: true,
}, },
includeToggle: { includeToggle: {
...@@ -72,7 +73,10 @@ export default { ...@@ -72,7 +73,10 @@ export default {
{{ __('Toggle discussion') }} {{ __('Toggle discussion') }}
</button> </button>
</div> </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 class="note-header-author-name">{{ author.name }}</span>
<span <span
v-if="author.status_tooltip_html" v-if="author.status_tooltip_html"
...@@ -81,6 +85,9 @@ export default { ...@@ -81,6 +85,9 @@ export default {
@{{ author.username }} @{{ author.username }}
</span> </span>
</a> </a>
<span v-else>
{{ __('A deleted user') }}
</span>
<span class="note-headline-light"> <span class="note-headline-light">
<span class="note-headline-meta"> <span class="note-headline-meta">
<template v-if="actionText"> <template v-if="actionText">
......
...@@ -95,6 +95,7 @@ module IssuableActions ...@@ -95,6 +95,7 @@ module IssuableActions
.includes(:noteable) .includes(:noteable)
.fresh .fresh
notes = ResourceEvents::MergeIntoNotesService.new(issuable, current_user).execute(notes)
notes = prepare_notes_for_rendering(notes) notes = prepare_notes_for_rendering(notes)
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
......
...@@ -18,6 +18,7 @@ module NotesActions ...@@ -18,6 +18,7 @@ module NotesActions
notes = notes_finder.execute notes = notes_finder.execute
.inc_relations_for_view .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 = prepare_notes_for_rendering(notes)
notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) } notes = notes.reject { |n| n.cross_reference_not_visible_for?(current_user) }
......
...@@ -108,7 +108,7 @@ module NotesHelper ...@@ -108,7 +108,7 @@ module NotesHelper
end end
def noteable_note_url(note) def noteable_note_url(note)
Gitlab::UrlBuilder.build(note) Gitlab::UrlBuilder.build(note) if note.id
end end
def form_resources def form_resources
......
...@@ -109,10 +109,6 @@ module Issuable ...@@ -109,10 +109,6 @@ module Issuable
false false
end end
def etag_caching_enabled?
false
end
def has_multiple_assignees? def has_multiple_assignees?
assignees.count > 1 assignees.count > 1
end end
......
...@@ -82,4 +82,23 @@ module Noteable ...@@ -82,4 +82,23 @@ module Noteable
def lockable? def lockable?
[MergeRequest, Issue].include?(self.class) [MergeRequest, Issue].include?(self.class)
end 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 end
# 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
...@@ -389,18 +389,7 @@ class Note < ActiveRecord::Base ...@@ -389,18 +389,7 @@ class Note < ActiveRecord::Base
end end
def expire_etag_cache def expire_etag_cache
return unless noteable&.discussions_rendered_on_frontend? noteable&.expire_note_etag_cache
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
)
end end
def touch(*args) def touch(*args)
......
...@@ -3,33 +3,122 @@ ...@@ -3,33 +3,122 @@
# This model is not used yet, it will be used for: # This model is not used yet, it will be used for:
# https://gitlab.com/gitlab-org/gitlab-ce/issues/48483 # https://gitlab.com/gitlab-org/gitlab-ce/issues/48483
class ResourceLabelEvent < ActiveRecord::Base class ResourceLabelEvent < ActiveRecord::Base
include Importable
include Gitlab::Utils::StrongMemoize
include CacheMarkdownField
cache_markdown_field :reference
belongs_to :user belongs_to :user
belongs_to :issue belongs_to :issue
belongs_to :merge_request belongs_to :merge_request
belongs_to :label belongs_to :label
validates :user, presence: true, on: :create scope :created_after, ->(time) { where('created_at > ?', time) }
validates :label, presence: true, on: :create
validates :user, presence: { unless: :importing? }, on: :create
validates :label, presence: { unless: :importing? }, on: :create
validate :exactly_one_issuable validate :exactly_one_issuable
after_save :expire_etag_cache
after_destroy :expire_etag_cache
enum action: { enum action: {
add: 1, add: 1,
remove: 2 remove: 2
} }
def self.issuable_columns def self.issuable_attrs
%i(issue_id merge_request_id).freeze %i(issue merge_request).freeze
end end
def issuable def issuable
issue || merge_request issue || merge_request
end 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 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 def exactly_one_issuable
if self.class.issuable_columns.count { |attr| self[attr] } != 1 issuable_count = self.class.issuable_attrs.count { |attr| self["#{attr}_id"] }
errors.add(:base, "Exactly one of #{self.class.issuable_columns.join(', ')} is required")
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 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
end end
...@@ -4,6 +4,12 @@ class NoteEntity < API::Entities::Note ...@@ -4,6 +4,12 @@ class NoteEntity < API::Entities::Note
include RequestAwareEntity include RequestAwareEntity
include NotesHelper 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 :type
expose :author, using: NoteUserEntity expose :author, using: NoteUserEntity
...@@ -46,8 +52,8 @@ class NoteEntity < API::Entities::Note ...@@ -46,8 +52,8 @@ class NoteEntity < API::Entities::Note
expose :emoji_awardable?, as: :emoji_awardable expose :emoji_awardable?, as: :emoji_awardable
expose :award_emoji, if: -> (note, _) { note.emoji_awardable? }, using: AwardEmojiEntity expose :award_emoji, if: -> (note, _) { note.emoji_awardable? }, using: AwardEmojiEntity
expose :report_abuse_path do |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)) new_abuse_report_path(user_id: note.author_id, ref_url: Gitlab::UrlBuilder.build(note))
end end
expose :noteable_note_url do |note| expose :noteable_note_url do |note|
......
# frozen_string_literal: true # frozen_string_literal: true
class ProjectNoteEntity < NoteEntity 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) note.project.team.human_max_access(note.author_id)
end end
...@@ -9,7 +9,7 @@ class ProjectNoteEntity < NoteEntity ...@@ -9,7 +9,7 @@ class ProjectNoteEntity < NoteEntity
toggle_award_emoji_project_note_path(note.project, note.id) toggle_award_emoji_project_note_path(note.project, note.id)
end end
expose :path do |note| expose :path, if: -> (note, _) { note.id } do |note|
project_note_path(note.project, note) project_note_path(note.project, note)
end end
......
...@@ -55,7 +55,9 @@ module Issuable ...@@ -55,7 +55,9 @@ module Issuable
added_labels = issuable.labels - old_labels added_labels = issuable.labels - old_labels
removed_labels = old_labels - issuable.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 end
def create_title_change_note(old_title) def create_title_change_note(old_title)
......
...@@ -36,6 +36,7 @@ module Issues ...@@ -36,6 +36,7 @@ module Issues
def update_new_issue def update_new_issue
rewrite_notes rewrite_notes
copy_resource_label_events
rewrite_issue_award_emoji rewrite_issue_award_emoji
add_note_moved_from add_note_moved_from
end end
...@@ -96,6 +97,18 @@ module Issues ...@@ -96,6 +97,18 @@ module Issues
end end
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 def rewrite_issue_award_emoji
rewrite_award_emoji(@old_issue, @new_issue) rewrite_award_emoji(@old_issue, @new_issue)
end end
......
...@@ -13,6 +13,7 @@ module Labels ...@@ -13,6 +13,7 @@ module Labels
label_ids_for_merge(new_label).find_in_batches(batch_size: BATCH_SIZE) do |batched_ids| label_ids_for_merge(new_label).find_in_batches(batch_size: BATCH_SIZE) do |batched_ids|
update_issuables(new_label, 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_issue_board_lists(new_label, batched_ids)
update_priorities(new_label, batched_ids) update_priorities(new_label, batched_ids)
subscribe_users(new_label, batched_ids) subscribe_users(new_label, batched_ids)
...@@ -52,6 +53,12 @@ module Labels ...@@ -52,6 +53,12 @@ module Labels
.update_all(label_id: new_label) .update_all(label_id: new_label)
end 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) def update_issue_board_lists(new_label, label_ids)
List List
.where(label: label_ids) .where(label: label_ids)
......
# frozen_string_literal: true # 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 module ResourceEvents
class ChangeLabelsService class ChangeLabelsService
attr_reader :resource, :user attr_reader :resource, :user
...@@ -25,6 +23,7 @@ module ResourceEvents ...@@ -25,6 +23,7 @@ module ResourceEvents
end end
Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, labels) Gitlab::Database.bulk_insert(ResourceLabelEvent.table_name, labels)
resource.expire_note_etag_cache
end end
private private
......
# 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
...@@ -98,47 +98,6 @@ module SystemNoteService ...@@ -98,47 +98,6 @@ module SystemNoteService
create_note(NoteSummary.new(issue, project, author, body, action: 'assignee')) create_note(NoteSummary.new(issue, project, author, body, action: 'assignee'))
end 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 # Called when the milestone of a Noteable is changed
# #
# noteable - Noteable object # noteable - Noteable object
......
---
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
# 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
...@@ -11,7 +11,7 @@ ...@@ -11,7 +11,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # 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 # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
...@@ -1841,6 +1841,9 @@ ActiveRecord::Schema.define(version: 20180901171833) do ...@@ -1841,6 +1841,9 @@ ActiveRecord::Schema.define(version: 20180901171833) do
t.integer "label_id" t.integer "label_id"
t.integer "user_id" t.integer "user_id"
t.datetime_with_timezone "created_at", null: false t.datetime_with_timezone "created_at", null: false
t.integer "cached_markdown_version"
t.text "reference"
t.text "reference_html"
end end
add_index "resource_label_events", ["issue_id"], name: "index_resource_label_events_on_issue_id", using: :btree add_index "resource_label_events", ["issue_id"], name: "index_resource_label_events_on_issue_id", using: :btree
......
...@@ -40,6 +40,7 @@ following locations: ...@@ -40,6 +40,7 @@ following locations:
- [Namespaces](namespaces.md) - [Namespaces](namespaces.md)
- [Notes](notes.md) (comments) - [Notes](notes.md) (comments)
- [Discussions](discussions.md) (threaded comments) - [Discussions](discussions.md) (threaded comments)
- [Resource Label Events](resource_label_events.md)
- [Notification settings](notification_settings.md) - [Notification settings](notification_settings.md)
- [Open source license templates](templates/licenses.md) - [Open source license templates](templates/licenses.md)
- [Pages Domains](pages_domains.md) - [Pages Domains](pages_domains.md)
......
# 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
```
...@@ -118,6 +118,7 @@ module API ...@@ -118,6 +118,7 @@ module API
mount ::API::Namespaces mount ::API::Namespaces
mount ::API::Notes mount ::API::Notes
mount ::API::Discussions mount ::API::Discussions
mount ::API::ResourceLabelEvents
mount ::API::NotificationSettings mount ::API::NotificationSettings
mount ::API::PagesDomains mount ::API::PagesDomains
mount ::API::Pipelines mount ::API::Pipelines
......
...@@ -1437,5 +1437,19 @@ module API ...@@ -1437,5 +1437,19 @@ module API
badge.type == 'ProjectBadge' ? 'project' : 'group' badge.type == 'ProjectBadge' ? 'project' : 'group'
end end
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
end end
# 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
# frozen_string_literal: true
module Banzai
module Pipeline
class LabelPipeline < BasePipeline
def self.filters
@filters ||= FilterArray[
Filter::SanitizationFilter,
Filter::LabelReferenceFilter
]
end
end
end
end
...@@ -19,6 +19,9 @@ project_tree: ...@@ -19,6 +19,9 @@ project_tree:
- milestone: - milestone:
- events: - events:
- :push_event_payload - :push_event_payload
- resource_label_events:
- label:
:priorities
- :issue_assignees - :issue_assignees
- snippets: - snippets:
- :award_emoji - :award_emoji
...@@ -45,6 +48,9 @@ project_tree: ...@@ -45,6 +48,9 @@ project_tree:
- milestone: - milestone:
- events: - events:
- :push_event_payload - :push_event_payload
- resource_label_events:
- label:
:priorities
- pipelines: - pipelines:
- notes: - notes:
- :author - :author
...@@ -137,6 +143,10 @@ excluded_attributes: ...@@ -137,6 +143,10 @@ excluded_attributes:
- :event_id - :event_id
project_badges: project_badges:
- :group_id - :group_id
resource_label_events:
- :reference
- :reference_html
- :epic_id
methods: methods:
labels: labels:
......
...@@ -259,6 +259,9 @@ msgstr "" ...@@ -259,6 +259,9 @@ msgstr ""
msgid "A default branch cannot be chosen for an empty project." msgid "A default branch cannot be chosen for an empty project."
msgstr "" msgstr ""
msgid "A deleted user"
msgstr ""
msgid "A new branch will be created in your fork and a new merge request will be started." msgid "A new branch will be created in your fork and a new merge request will be started."
msgstr "" msgstr ""
......
...@@ -154,7 +154,7 @@ describe Projects::NotesController do ...@@ -154,7 +154,7 @@ describe Projects::NotesController do
get :index, request_params get :index, request_params
expect(parsed_response[:notes].count).to eq(1) 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 end
it 'does not result in N+1 queries' do it 'does not result in N+1 queries' do
......
...@@ -2,9 +2,12 @@ ...@@ -2,9 +2,12 @@
FactoryBot.define do FactoryBot.define do
factory :resource_label_event do factory :resource_label_event do
user { issue.project.creator }
action :add action :add
label label
issue user { issuable&.author || create(:user) }
after(:build) do |event, evaluator|
event.issue = create(:issue) unless event.issuable
end
end end
end end
# 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
...@@ -16,7 +16,7 @@ export default { ...@@ -16,7 +16,7 @@ export default {
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 1749, id: '1749',
type: 'DiffNote', type: 'DiffNote',
attachment: null, attachment: null,
author: { author: {
...@@ -68,7 +68,7 @@ export default { ...@@ -68,7 +68,7 @@ export default {
'/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20',
}, },
{ {
id: 1753, id: '1753',
type: 'DiffNote', type: 'DiffNote',
attachment: null, attachment: null,
author: { author: {
...@@ -120,7 +120,7 @@ export default { ...@@ -120,7 +120,7 @@ export default {
'/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20',
}, },
{ {
id: 1754, id: '1754',
type: 'DiffNote', type: 'DiffNote',
attachment: null, attachment: null,
author: { author: {
...@@ -162,7 +162,7 @@ export default { ...@@ -162,7 +162,7 @@ export default {
'/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20',
}, },
{ {
id: 1755, id: '1755',
type: 'DiffNote', type: 'DiffNote',
attachment: null, attachment: null,
author: { author: {
...@@ -204,7 +204,7 @@ export default { ...@@ -204,7 +204,7 @@ export default {
'/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20', '/gitlab-org/gitlab-test/issues/new?discussion_to_resolve=6b232e05bea388c6b043ccc243ba505faac04ea8&merge_request_to_resolve_discussions_of=20',
}, },
{ {
id: 1756, id: '1756',
type: 'DiffNote', type: 'DiffNote',
attachment: null, attachment: null,
author: { author: {
......
...@@ -28,7 +28,7 @@ describe('issue_note_actions component', () => { ...@@ -28,7 +28,7 @@ describe('issue_note_actions component', () => {
canEdit: true, canEdit: true,
canAwardEmoji: true, canAwardEmoji: true,
canReportAsAbuse: true, canReportAsAbuse: true,
noteId: 539, noteId: '539',
noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1',
reportAbusePath: reportAbusePath:
'/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', '/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', () => { ...@@ -59,6 +59,20 @@ describe('issue_note_actions component', () => {
expect(vm.$el.querySelector(`a[href="${props.reportAbusePath}"]`)).toBeDefined(); 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', () => { it('should be possible to delete comment', () => {
expect(vm.$el.querySelector('.js-note-delete')).toBeDefined(); expect(vm.$el.querySelector('.js-note-delete')).toBeDefined();
}); });
...@@ -77,7 +91,7 @@ describe('issue_note_actions component', () => { ...@@ -77,7 +91,7 @@ describe('issue_note_actions component', () => {
canEdit: false, canEdit: false,
canAwardEmoji: false, canAwardEmoji: false,
canReportAsAbuse: false, canReportAsAbuse: false,
noteId: 539, noteId: '539',
noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1', noteUrl: 'https://localhost:3000/group/project/merge_requests/1#note_1',
reportAbusePath: reportAbusePath:
'/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26', '/abuse_reports/new?ref_url=http%3A%2F%2Flocalhost%3A3000%2Fgitlab-org%2Fgitlab-ce%2Fissues%2F7%23note_539&user_id=26',
......
...@@ -30,7 +30,7 @@ describe('note_awards_list component', () => { ...@@ -30,7 +30,7 @@ describe('note_awards_list component', () => {
propsData: { propsData: {
awards: awardsMock, awards: awardsMock,
noteAuthorId: 2, noteAuthorId: 2,
noteId: 545, noteId: '545',
canAwardEmoji: true, canAwardEmoji: true,
toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
}, },
...@@ -70,7 +70,7 @@ describe('note_awards_list component', () => { ...@@ -70,7 +70,7 @@ describe('note_awards_list component', () => {
propsData: { propsData: {
awards: awardsMock, awards: awardsMock,
noteAuthorId: 2, noteAuthorId: 2,
noteId: 545, noteId: '545',
canAwardEmoji: false, canAwardEmoji: false,
toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji', toggleAwardPath: '/gitlab-org/gitlab-ce/notes/545/toggle_award_emoji',
}, },
......
...@@ -19,7 +19,7 @@ describe('issue_note_form component', () => { ...@@ -19,7 +19,7 @@ describe('issue_note_form component', () => {
props = { props = {
isEditing: false, isEditing: false,
noteBody: 'Magni suscipit eius consectetur enim et ex et commodi.', noteBody: 'Magni suscipit eius consectetur enim et ex et commodi.',
noteId: 545, noteId: '545',
}; };
vm = new Component({ vm = new Component({
...@@ -32,6 +32,22 @@ describe('issue_note_form component', () => { ...@@ -32,6 +32,22 @@ describe('issue_note_form component', () => {
vm.$destroy(); 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', () => { describe('conflicts editing', () => {
it('should show conflict message if note changes outside the component', done => { it('should show conflict message if note changes outside the component', done => {
vm.isEditing = true; vm.isEditing = true;
......
...@@ -33,7 +33,7 @@ describe('note_header component', () => { ...@@ -33,7 +33,7 @@ describe('note_header component', () => {
}, },
createdAt: '2017-08-02T10:51:58.559Z', createdAt: '2017-08-02T10:51:58.559Z',
includeToggle: false, includeToggle: false,
noteId: 1394, noteId: '1394',
expanded: true, expanded: true,
}, },
}).$mount(); }).$mount();
...@@ -47,6 +47,16 @@ describe('note_header component', () => { ...@@ -47,6 +47,16 @@ describe('note_header component', () => {
it('should render timestamp link', () => { it('should render timestamp link', () => {
expect(vm.$el.querySelector('a[href="#note_1394"]')).toBeDefined(); 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', () => { describe('discussion', () => {
...@@ -66,7 +76,7 @@ describe('note_header component', () => { ...@@ -66,7 +76,7 @@ describe('note_header component', () => {
}, },
createdAt: '2017-08-02T10:51:58.559Z', createdAt: '2017-08-02T10:51:58.559Z',
includeToggle: true, includeToggle: true,
noteId: 1395, noteId: '1395',
expanded: true, expanded: true,
}, },
}).$mount(); }).$mount();
......
...@@ -66,7 +66,7 @@ export const individualNote = { ...@@ -66,7 +66,7 @@ export const individualNote = {
individual_note: true, individual_note: true,
notes: [ notes: [
{ {
id: 1390, id: '1390',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -111,7 +111,7 @@ export const individualNote = { ...@@ -111,7 +111,7 @@ export const individualNote = {
}; };
export const note = { export const note = {
id: 546, id: '546',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -174,7 +174,7 @@ export const discussionMock = { ...@@ -174,7 +174,7 @@ export const discussionMock = {
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 1395, id: '1395',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -211,7 +211,7 @@ export const discussionMock = { ...@@ -211,7 +211,7 @@ export const discussionMock = {
path: '/gitlab-org/gitlab-ce/notes/1395', path: '/gitlab-org/gitlab-ce/notes/1395',
}, },
{ {
id: 1396, id: '1396',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -257,7 +257,7 @@ export const discussionMock = { ...@@ -257,7 +257,7 @@ export const discussionMock = {
path: '/gitlab-org/gitlab-ce/notes/1396', path: '/gitlab-org/gitlab-ce/notes/1396',
}, },
{ {
id: 1437, id: '1437',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -308,7 +308,7 @@ export const discussionMock = { ...@@ -308,7 +308,7 @@ export const discussionMock = {
}; };
export const loggedOutnoteableData = { export const loggedOutnoteableData = {
id: 98, id: '98',
iid: 26, iid: 26,
author_id: 1, author_id: 1,
description: '', description: '',
...@@ -358,7 +358,7 @@ export const collapseNotesMock = [ ...@@ -358,7 +358,7 @@ export const collapseNotesMock = [
individual_note: true, individual_note: true,
notes: [ notes: [
{ {
id: 1390, id: '1390',
attachment: null, attachment: null,
author: { author: {
id: 1, id: 1,
...@@ -393,7 +393,7 @@ export const collapseNotesMock = [ ...@@ -393,7 +393,7 @@ export const collapseNotesMock = [
individual_note: true, individual_note: true,
notes: [ notes: [
{ {
id: 1391, id: '1391',
attachment: null, attachment: null,
author: { author: {
id: 1, id: 1,
...@@ -433,7 +433,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { ...@@ -433,7 +433,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 1390, id: '1390',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -495,7 +495,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { ...@@ -495,7 +495,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 1391, id: '1391',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -544,7 +544,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = { ...@@ -544,7 +544,7 @@ export const INDIVIDUAL_NOTE_RESPONSE_MAP = {
'/gitlab-org/gitlab-ce/notes/1471': { '/gitlab-org/gitlab-ce/notes/1471': {
commands_changes: null, commands_changes: null,
valid: true, valid: true,
id: 1471, id: '1471',
attachment: null, attachment: null,
author: { author: {
id: 1, id: 1,
...@@ -600,7 +600,7 @@ export const DISCUSSION_NOTE_RESPONSE_MAP = { ...@@ -600,7 +600,7 @@ export const DISCUSSION_NOTE_RESPONSE_MAP = {
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 1471, id: '1471',
attachment: { attachment: {
url: null, url: null,
filename: null, filename: null,
...@@ -671,7 +671,7 @@ export const notesWithDescriptionChanges = [ ...@@ -671,7 +671,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 901, id: '901',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -718,7 +718,7 @@ export const notesWithDescriptionChanges = [ ...@@ -718,7 +718,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 902, id: '902',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -765,7 +765,7 @@ export const notesWithDescriptionChanges = [ ...@@ -765,7 +765,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 903, id: '903',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -809,7 +809,7 @@ export const notesWithDescriptionChanges = [ ...@@ -809,7 +809,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 904, id: '904',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -854,7 +854,7 @@ export const notesWithDescriptionChanges = [ ...@@ -854,7 +854,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 905, id: '905',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -898,7 +898,7 @@ export const notesWithDescriptionChanges = [ ...@@ -898,7 +898,7 @@ export const notesWithDescriptionChanges = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 906, id: '906',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -945,7 +945,7 @@ export const collapsedSystemNotes = [ ...@@ -945,7 +945,7 @@ export const collapsedSystemNotes = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 901, id: '901',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -992,7 +992,7 @@ export const collapsedSystemNotes = [ ...@@ -992,7 +992,7 @@ export const collapsedSystemNotes = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 902, id: '902',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -1039,7 +1039,7 @@ export const collapsedSystemNotes = [ ...@@ -1039,7 +1039,7 @@ export const collapsedSystemNotes = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 904, id: '904',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -1084,7 +1084,7 @@ export const collapsedSystemNotes = [ ...@@ -1084,7 +1084,7 @@ export const collapsedSystemNotes = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 905, id: '905',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
...@@ -1129,7 +1129,7 @@ export const collapsedSystemNotes = [ ...@@ -1129,7 +1129,7 @@ export const collapsedSystemNotes = [
expanded: true, expanded: true,
notes: [ notes: [
{ {
id: 906, id: '906',
type: null, type: null,
attachment: null, attachment: null,
author: { author: {
......
...@@ -9,7 +9,7 @@ describe('system note component', () => { ...@@ -9,7 +9,7 @@ describe('system note component', () => {
beforeEach(() => { beforeEach(() => {
props = { props = {
note: { note: {
id: 1424, id: '1424',
author: { author: {
id: 1, id: 1,
name: 'Root', name: 'Root',
......
...@@ -324,3 +324,9 @@ metrics: ...@@ -324,3 +324,9 @@ metrics:
- latest_closed_by - latest_closed_by
- merged_by - merged_by
- pipeline - pipeline
resource_label_events:
- user
- issue
- merge_request
- epic
- label
...@@ -331,6 +331,28 @@ ...@@ -331,6 +331,28 @@
}, },
"events": [] "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 @@ ...@@ -2515,6 +2537,17 @@
"events": [] "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": { "merge_request_diff": {
"id": 27, "id": 27,
"state": "collected", "state": "collected",
......
...@@ -89,6 +89,14 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do ...@@ -89,6 +89,14 @@ describe Gitlab::ImportExport::ProjectTreeRestorer do
expect(ProtectedTag.first.create_access_levels).not_to be_empty expect(ProtectedTag.first.create_access_levels).not_to be_empty
end 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 context 'event at forth level of the tree' do
let(:event) { Event.where(action: 6).first } let(:event) { Event.where(action: 6).first }
......
...@@ -169,6 +169,14 @@ describe Gitlab::ImportExport::ProjectTreeSaver do ...@@ -169,6 +169,14 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
expect(priorities.flatten).not_to be_empty expect(priorities.flatten).not_to be_empty
end 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 it 'saves the correct service type' do
expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService') expect(saved_project_json['services'].first['type']).to eq('CustomIssueTrackerService')
end end
...@@ -291,6 +299,9 @@ describe Gitlab::ImportExport::ProjectTreeSaver do ...@@ -291,6 +299,9 @@ describe Gitlab::ImportExport::ProjectTreeSaver do
project: project, project: project,
commit_id: ci_build.pipeline.sha) 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(:event, :created, target: milestone, project: project, author: user)
create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' }) create(:service, project: project, type: 'CustomIssueTrackerService', category: 'issue_tracker', properties: { one: 'value' })
......
...@@ -579,3 +579,11 @@ Badge: ...@@ -579,3 +579,11 @@ Badge:
- type - type
ProjectCiCdSetting: ProjectCiCdSetting:
- group_runners_enabled - group_runners_enabled
ResourceLabelEvent:
- id
- action
- issue_id
- merge_request_id
- label_id
- user_id
- created_at
# 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
...@@ -3,7 +3,7 @@ ...@@ -3,7 +3,7 @@
require 'rails_helper' require 'rails_helper'
RSpec.describe ResourceLabelEvent, type: :model do RSpec.describe ResourceLabelEvent, type: :model do
subject { build(:resource_label_event) } subject { build(:resource_label_event, issue: issue) }
let(:issue) { create(:issue) } let(:issue) { create(:issue) }
let(:merge_request) { create(:merge_request) } let(:merge_request) { create(:merge_request) }
...@@ -16,8 +16,6 @@ RSpec.describe ResourceLabelEvent, type: :model do ...@@ -16,8 +16,6 @@ RSpec.describe ResourceLabelEvent, type: :model do
describe 'validations' do describe 'validations' do
it { is_expected.to be_valid } 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 describe 'Issuable validation' do
it 'is invalid if issue_id and merge_request_id are missing' do it 'is invalid if issue_id and merge_request_id are missing' do
...@@ -45,4 +43,52 @@ RSpec.describe ResourceLabelEvent, type: :model do ...@@ -45,4 +43,52 @@ RSpec.describe ResourceLabelEvent, type: :model do
end end
end 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 end
# 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
...@@ -12,12 +12,21 @@ describe Issuable::CommonSystemNotesService do ...@@ -12,12 +12,21 @@ describe Issuable::CommonSystemNotesService do
it_behaves_like 'system note creation', { time_estimate: 5 }, 'changed time estimate' it_behaves_like 'system note creation', { time_estimate: 5 }, 'changed time estimate'
context 'when new label is added' do context 'when new label is added' do
let(:label) { create(:label, project: project) }
before do before do
label = create(:label, project: project)
issuable.labels << label issuable.labels << label
issuable.save
end 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 end
context 'when new milestone is assigned' do context 'when new milestone is assigned' do
......
...@@ -122,6 +122,17 @@ describe Issues::MoveService do ...@@ -122,6 +122,17 @@ describe Issues::MoveService do
end end
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 context 'generic issue' do
include_context 'issue move executed' include_context 'issue move executed'
......
...@@ -189,11 +189,12 @@ describe Issues::UpdateService, :mailer do ...@@ -189,11 +189,12 @@ describe Issues::UpdateService, :mailer do
expect(note.note).to include "assigned to #{user2.to_reference}" expect(note.note).to include "assigned to #{user2.to_reference}"
end end
it 'creates system note about issue label edit' do it 'creates a resource label event' do
note = find_note('added ~') event = issue.resource_label_events.last
expect(note).not_to be_nil expect(event).not_to be_nil
expect(note.note).to include "added #{label.to_reference} label" expect(event.label_id).to eq label.id
expect(event.user_id).to eq user.id
end end
it 'creates system note about title change' do it 'creates system note about title change' do
......
...@@ -109,11 +109,12 @@ describe MergeRequests::UpdateService, :mailer do ...@@ -109,11 +109,12 @@ describe MergeRequests::UpdateService, :mailer do
expect(note.note).to include "assigned to #{user2.to_reference}" expect(note.note).to include "assigned to #{user2.to_reference}"
end end
it 'creates system note about merge_request label edit' do it 'creates a resource label event' do
note = find_note('added ~') event = merge_request.resource_label_events.last
expect(note).not_to be_nil expect(event).not_to be_nil
expect(note.note).to include "added #{label.to_reference} label" expect(event.label_id).to eq label.id
expect(event.user_id).to eq user.id
end end
it 'creates system note about title change' do it 'creates system note about title change' do
......
...@@ -18,6 +18,14 @@ describe ResourceEvents::ChangeLabelsService do ...@@ -18,6 +18,14 @@ describe ResourceEvents::ChangeLabelsService do
expect(event.action).to eq(action) expect(event.action).to eq(action)
end 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 context 'when adding a label' do
let(:added) { [labels[0]] } let(:added) { [labels[0]] }
let(:removed) { [] } let(:removed) { [] }
......
# 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
...@@ -197,45 +197,6 @@ describe SystemNoteService do ...@@ -197,45 +197,6 @@ describe SystemNoteService do
end end
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 describe '.change_milestone' do
context 'for a project milestone' do context 'for a project milestone' do
subject { described_class.change_milestone(noteable, project, author, milestone) } subject { described_class.change_milestone(noteable, project, author, milestone) }
......
# 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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment