diff options
20 files changed, 134 insertions, 13 deletions
diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb index fff57472a4f..6d6bcbaf88a 100644 --- a/app/helpers/application_helper.rb +++ b/app/helpers/application_helper.rb @@ -180,16 +180,16 @@ module ApplicationHelper element end - def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', include_author: false) - return if object.updated_at == object.created_at + def edited_time_ago_with_tooltip(object, placement: 'top', html_class: 'time_ago', exclude_author: false) + return if object.last_edited_at == object.created_at || object.last_edited_at.blank? - content_tag :small, class: "edited-text" do - output = content_tag(:span, "Edited ") - output << time_ago_with_tooltip(object.updated_at, placement: placement, html_class: html_class) + content_tag :small, class: 'edited-text' do + output = content_tag(:span, 'Edited ') + output << time_ago_with_tooltip(object.last_edited_at, placement: placement, html_class: html_class) - if include_author && object.updated_by && object.updated_by != object.author - output << content_tag(:span, " by ") - output << link_to_member(object.project, object.updated_by, avatar: false, author_class: nil) + if !exclude_author && object.last_edited_by + output << content_tag(:span, ' by ') + output << link_to_member(object.project, object.last_edited_by, avatar: false, author_class: nil) end output diff --git a/app/helpers/system_note_helper.rb b/app/helpers/system_note_helper.rb index 1ea60e39386..d889d141101 100644 --- a/app/helpers/system_note_helper.rb +++ b/app/helpers/system_note_helper.rb @@ -1,6 +1,7 @@ module SystemNoteHelper ICON_NAMES_BY_ACTION = { 'commit' => 'icon_commit', + 'description' => 'icon_edit', 'merge' => 'icon_merge', 'merged' => 'icon_merged', 'opened' => 'icon_status_open', diff --git a/app/models/concerns/issuable.rb b/app/models/concerns/issuable.rb index 16f04305a43..075ec575f9d 100644 --- a/app/models/concerns/issuable.rb +++ b/app/models/concerns/issuable.rb @@ -27,6 +27,7 @@ module Issuable belongs_to :author, class_name: "User" belongs_to :updated_by, class_name: "User" + belongs_to :last_edited_by, class_name: 'User' belongs_to :milestone has_many :notes, as: :noteable, inverse_of: :noteable, dependent: :destroy do def authors_loaded? diff --git a/app/models/note.rb b/app/models/note.rb index b06985b4a6f..46d0a4f159f 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -18,6 +18,11 @@ class Note < ActiveRecord::Base cache_markdown_field :note, pipeline: :note, issuable_state_filter_enabled: true + # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with notes. + # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102 + alias_attribute :last_edited_at, :updated_at + alias_attribute :last_edited_by, :updated_by + # Attribute containing rendered and redacted Markdown as generated by # Banzai::ObjectRenderer. attr_accessor :redacted_note_html @@ -38,6 +43,7 @@ class Note < ActiveRecord::Base belongs_to :noteable, polymorphic: true, touch: true belongs_to :author, class_name: "User" belongs_to :updated_by, class_name: "User" + belongs_to :last_edited_by, class_name: 'User' has_many :todos, dependent: :destroy has_many :events, as: :target, dependent: :destroy diff --git a/app/models/snippet.rb b/app/models/snippet.rb index d8860718cb5..abfbefdf9a0 100644 --- a/app/models/snippet.rb +++ b/app/models/snippet.rb @@ -12,6 +12,11 @@ class Snippet < ActiveRecord::Base cache_markdown_field :title, pipeline: :single_line cache_markdown_field :content + # Aliases to make application_helper#edited_time_ago_with_tooltip helper work properly with snippets. + # See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/10392/diffs#note_28719102 + alias_attribute :last_edited_at, :updated_at + alias_attribute :last_edited_by, :updated_by + # If file_name changes, it invalidates content alias_method :default_content_html_invalidator, :content_html_invalidated? def content_html_invalidated? diff --git a/app/models/system_note_metadata.rb b/app/models/system_note_metadata.rb index 1e6fc837a75..b44f4fe000c 100644 --- a/app/models/system_note_metadata.rb +++ b/app/models/system_note_metadata.rb @@ -1,6 +1,6 @@ class SystemNoteMetadata < ActiveRecord::Base ICON_TYPES = %w[ - commit merge confidential visible label assignee cross_reference + commit description merge confidential visible label assignee cross_reference title time_tracking branch milestone discussion task moved opened closed merged ].freeze diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb index 6c2777a8d2d..c1e532b504a 100644 --- a/app/services/issuable_base_service.rb +++ b/app/services/issuable_base_service.rb @@ -19,6 +19,10 @@ class IssuableBaseService < BaseService issuable, issuable.project, current_user, old_title) end + def create_description_change_note(issuable) + SystemNoteService.change_description(issuable, issuable.project, current_user) + end + def create_branch_change_note(issuable, branch_type, old_branch, new_branch) SystemNoteService.change_branch( issuable, issuable.project, current_user, branch_type, @@ -211,6 +215,10 @@ class IssuableBaseService < BaseService if issuable.changed? || params.present? issuable.assign_attributes(params.merge(updated_by: current_user)) + if has_title_or_description_changed?(issuable) + issuable.assign_attributes(last_edited_at: Time.now, last_edited_by: current_user) + end + before_update(issuable) if issuable.with_transaction_returning_status { issuable.save } @@ -239,6 +247,10 @@ class IssuableBaseService < BaseService old_label_ids.sort != new_label_ids.sort end + def has_title_or_description_changed?(issuable) + issuable.title_changed? || issuable.description_changed? + end + def change_state(issuable) case params.delete(:state_event) when 'reopen' @@ -294,6 +306,10 @@ class IssuableBaseService < BaseService create_title_change_note(issuable, issuable.previous_changes['title'].first) end + if issuable.previous_changes.include?('description') + create_description_change_note(issuable) + end + if issuable.previous_changes.include?('description') && issuable.tasks? create_task_status_note(issuable) end diff --git a/app/services/system_note_service.rb b/app/services/system_note_service.rb index fb1f56c9cc6..174e7c6e95b 100644 --- a/app/services/system_note_service.rb +++ b/app/services/system_note_service.rb @@ -299,6 +299,23 @@ module SystemNoteService create_note(NoteSummary.new(noteable, project, author, body, action: 'title')) end + # Called when the description of a Noteable is changed + # + # noteable - Noteable object that responds to `description` + # project - Project owning noteable + # author - User performing the change + # + # Example Note text: + # + # "changed the description" + # + # Returns the created Note object + def change_description(noteable, project, author) + body = 'changed the description' + + create_note(NoteSummary.new(noteable, project, author, body, action: 'description')) + end + # Called when the confidentiality changes # # issue - Issue object diff --git a/app/views/shared/notes/_note.html.haml b/app/views/shared/notes/_note.html.haml index 071c48fa2e4..5c1156b06fb 100644 --- a/app/views/shared/notes/_note.html.haml +++ b/app/views/shared/notes/_note.html.haml @@ -40,7 +40,7 @@ .note-body{ class: note_editable ? 'js-task-list-container' : '' } .note-text.md = note.redacted_note_html - = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) + = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago') - if note_editable = render 'shared/notes/edit', note: note .note-awards diff --git a/app/views/shared/snippets/_header.html.haml b/app/views/shared/snippets/_header.html.haml index d084f5e9684..501c09d71d5 100644 --- a/app/views/shared/snippets/_header.html.haml +++ b/app/views/shared/snippets/_header.html.haml @@ -21,4 +21,4 @@ = markdown_field(@snippet, :title) - if @snippet.updated_at != @snippet.created_at - = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago') + = edited_time_ago_with_tooltip(@snippet, placement: 'bottom', html_class: 'snippet-edited-ago', exclude_author: true) diff --git a/changelogs/unreleased/add_system_note_for_editing_issuable.yml b/changelogs/unreleased/add_system_note_for_editing_issuable.yml new file mode 100644 index 00000000000..3cbc7f91bf0 --- /dev/null +++ b/changelogs/unreleased/add_system_note_for_editing_issuable.yml @@ -0,0 +1,4 @@ +--- +title: Add system note on description change of issue/merge request +merge_request: 10392 +author: blackst0ne diff --git a/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb new file mode 100644 index 00000000000..6ac10723c82 --- /dev/null +++ b/db/migrate/20170503021915_add_last_edited_at_and_last_edited_by_id_to_issues.rb @@ -0,0 +1,14 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddLastEditedAtAndLastEditedByIdToIssues < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + add_column :issues, :last_edited_at, :timestamp + add_column :issues, :last_edited_by_id, :integer + end +end diff --git a/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb new file mode 100644 index 00000000000..7a1acdcbf69 --- /dev/null +++ b/db/migrate/20170503022548_add_last_edited_at_and_last_edited_by_id_to_merge_requests.rb @@ -0,0 +1,14 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddLastEditedAtAndLastEditedByIdToMergeRequests < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + def change + add_column :merge_requests, :last_edited_at, :timestamp + add_column :merge_requests, :last_edited_by_id, :integer + end +end diff --git a/db/schema.rb b/db/schema.rb index aad97d5f810..ebe802a3087 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -498,6 +498,8 @@ ActiveRecord::Schema.define(version: 20170504102911) do t.integer "relative_position" t.datetime "closed_at" t.integer "cached_markdown_version" + t.datetime "last_edited_at" + t.integer "last_edited_by_id" end add_index "issues", ["assignee_id"], name: "index_issues_on_assignee_id", using: :btree @@ -684,6 +686,8 @@ ActiveRecord::Schema.define(version: 20170504102911) do t.text "description_html" t.integer "time_estimate" t.integer "cached_markdown_version" + t.datetime "last_edited_at" + t.integer "last_edited_by_id" end add_index "merge_requests", ["assignee_id"], name: "index_merge_requests_on_assignee_id", using: :btree @@ -1425,4 +1429,4 @@ ActiveRecord::Schema.define(version: 20170504102911) do add_foreign_key "timelogs", "merge_requests", name: "fk_timelogs_merge_requests_merge_request_id", on_delete: :cascade add_foreign_key "trending_projects", "projects", on_delete: :cascade add_foreign_key "u2f_registrations", "users" -end
\ No newline at end of file +end diff --git a/lib/gitlab/import_export/relation_factory.rb b/lib/gitlab/import_export/relation_factory.rb index 4a54e7ef2e7..956763fa399 100644 --- a/lib/gitlab/import_export/relation_factory.rb +++ b/lib/gitlab/import_export/relation_factory.rb @@ -15,7 +15,7 @@ module Gitlab priorities: :label_priorities, label: :project_label }.freeze - USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id merge_user_id resolved_by_id].freeze + USER_REFERENCES = %w[author_id assignee_id updated_by_id user_id created_by_id last_edited_by_id merge_user_id resolved_by_id].freeze PROJECT_REFERENCES = %w[project_id source_project_id target_project_id].freeze diff --git a/spec/lib/gitlab/import_export/all_models.yml b/spec/lib/gitlab/import_export/all_models.yml index cd2fa27bb9f..f7654f1aade 100644 --- a/spec/lib/gitlab/import_export/all_models.yml +++ b/spec/lib/gitlab/import_export/all_models.yml @@ -9,6 +9,7 @@ issues: - notes - label_links - labels +- last_edited_by - todos - user_agent_detail - moved_to @@ -27,6 +28,7 @@ notes: - noteable - author - updated_by +- last_edited_by - resolved_by - todos - events @@ -72,6 +74,7 @@ merge_requests: - notes - label_links - labels +- last_edited_by - todos - target_project - source_project diff --git a/spec/lib/gitlab/import_export/safe_model_attributes.yml b/spec/lib/gitlab/import_export/safe_model_attributes.yml index 59c8b48a2be..a66086f8b47 100644 --- a/spec/lib/gitlab/import_export/safe_model_attributes.yml +++ b/spec/lib/gitlab/import_export/safe_model_attributes.yml @@ -23,6 +23,8 @@ Issue: - weight - time_estimate - relative_position +- last_edited_at +- last_edited_by_id Event: - id - target_type @@ -154,6 +156,8 @@ MergeRequest: - approvals_before_merge - rebase_commit_sha - time_estimate +- last_edited_at +- last_edited_by_id MergeRequestDiff: - id - state diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb index 6633ac10236..1954d8739f6 100644 --- a/spec/services/issues/update_service_spec.rb +++ b/spec/services/issues/update_service_spec.rb @@ -132,6 +132,17 @@ describe Issues::UpdateService, services: true do end end + context 'when description changed' do + it 'creates system note about description change' do + update_issue(description: 'Changed description') + + note = find_note('changed the description') + + expect(note).not_to be_nil + expect(note.note).to eq('changed the description') + end + end + context 'when issue turns confidential' do let(:opts) do { diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb index 694ec3a579f..31487c0f794 100644 --- a/spec/services/merge_requests/update_service_spec.rb +++ b/spec/services/merge_requests/update_service_spec.rb @@ -102,6 +102,13 @@ describe MergeRequests::UpdateService, services: true do expect(note.note).to eq 'changed title from **{-Old-} title** to **{+New+} title**' end + it 'creates system note about description change' do + note = find_note('changed the description') + + expect(note).not_to be_nil + expect(note.note).to eq('changed the description') + end + it 'creates system note about branch change' do note = find_note('changed target') diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb index 68816bf36b8..516566eddef 100644 --- a/spec/services/system_note_service_spec.rb +++ b/spec/services/system_note_service_spec.rb @@ -339,6 +339,20 @@ describe SystemNoteService, services: true do end end + describe '.change_description' do + subject { described_class.change_description(noteable, project, author) } + + context 'when noteable responds to `description`' do + it_behaves_like 'a system note' do + let(:action) { 'description' } + end + + it 'sets the note text' do + expect(subject.note).to eq('changed the description') + end + end + end + describe '.change_issue_confidentiality' do subject { described_class.change_issue_confidentiality(noteable, project, author) } |