Merge branch '57783-better-indication-that-an-issue-has-been-duplicated-ee' into 'master'

[EE] Better indication that an issue has been duplicated

See merge request gitlab-org/gitlab!16204

app/helpers/issues_helper.rb

@@ -150,6 +150,24 @@ module IssuesHelper
       can?(current_user, :create_merge_request_in, @project)
   end
 
+  def issue_closed_link(issue, current_user, css_class: '')
+    if issue.moved? && can?(current_user, :read_issue, issue.moved_to)
+      link_to(s_('IssuableStatus|moved'), issue.moved_to, class: css_class)
+    elsif issue.duplicated? && can?(current_user, :read_issue, issue.duplicated_to)
+      link_to(s_('IssuableStatus|duplicated'), issue.duplicated_to, class: css_class)
+    end
+  end
+
+  def issue_closed_text(issue, current_user)
+    link = issue_closed_link(issue, current_user, css_class: 'text-white text-underline')
+
+    if link
+      s_('IssuableStatus|Closed (%{link})').html_safe % { link: link }
+    else
+      s_('IssuableStatus|Closed')
+    end
+  end
+
   # Required for Banzai::Filter::IssueReferenceFilter
   module_function :url_for_issue
   module_function :url_for_internal_issue

app/models/issue.rb

@@ -27,6 +27,7 @@ class Issue < ApplicationRecord
 
   belongs_to :project
   belongs_to :moved_to, class_name: 'Issue'
+  belongs_to :duplicated_to, class_name: 'Issue'
   belongs_to :closed_by, class_name: 'User'
 
   has_internal_id :iid, scope: :project, init: ->(s) { s&.project&.issues&.maximum(:iid) }
@@ -181,6 +182,10 @@ class Issue < ApplicationRecord
     !moved_to_id.nil?
   end
 
+  def duplicated?
+    !duplicated_to_id.nil?
+  end
+
   def can_move?(user, to_project = nil)
     if to_project
       return false unless user.can?(:admin_issue, to_project)

app/serializers/issue_entity.rb

@@ -20,11 +20,17 @@ class IssueEntity < IssuableEntity
   expose :project_id
 
   expose :moved_to_id do |issue|
-    if issue.moved_to_id.present? && can?(request.current_user, :read_issue, issue.moved_to)
+    if issue.moved? && can?(request.current_user, :read_issue, issue.moved_to)
       issue.moved_to_id
     end
   end
 
+  expose :duplicated_to_id do |issue|
+    if issue.duplicated? && can?(request.current_user, :read_issue, issue.duplicated_to)
+      issue.duplicated_to_id
+    end
+  end
+
   expose :web_url do |issue|
     project_issue_path(issue.project, issue)
   end

app/services/issues/duplicate_service.rb

@@ -11,6 +11,7 @@ module Issues
       create_issue_canonical_note(canonical_issue, duplicate_issue)
 
       close_service.new(project, current_user, {}).execute(duplicate_issue)
+      duplicate_issue.update(duplicated_to: canonical_issue)
     end
 
     private

app/views/projects/issues/show.html.haml

@@ -15,13 +15,7 @@
     .issuable-status-box.status-box.status-box-issue-closed{ class: issue_button_visibility(@issue, false) }
       = sprite_icon('mobile-issue-close', size: 16, css_class: 'd-block d-sm-none')
       .d-none.d-sm-block
-        - if @issue.moved? && can?(current_user, :read_issue, @issue.moved_to)
-          - moved_link_start = "<a href=\"#{issue_path(@issue.moved_to)}\" class=\"text-white text-underline\">".html_safe
-          - moved_link_end = '</a>'.html_safe
-          = s_('IssuableStatus|Closed (%{moved_link_start}moved%{moved_link_end})').html_safe % {moved_link_start: moved_link_start,
-            moved_link_end: moved_link_end}
-        - else
-          = _("Closed")
+        = issue_closed_text(@issue, current_user)
     .issuable-status-box.status-box.status-box-open{ class: issue_button_visibility(@issue, true) }
       = sprite_icon('issue-open-m', size: 16, css_class: 'd-block d-sm-none')
       %span.d-none.d-sm-block Open

changelogs/unreleased/57783-better-indication-that-an-issue-has-been-duplicated.yml

+---
+title: Indicate on Issue Status if an Issue was Duplicated
+merge_request: 32472
+author:
+type: changed

config/pseudonymizer.yml

@@ -98,6 +98,7 @@ tables:
     - weight
     - due_date
     - moved_to_id
+    - duplicated_to_id
     - lock_version
     - time_estimate
     - last_edited_at

db/migrate/20190830140240_add_duplicated_to_to_issue.rb

+# frozen_string_literal: true
+
+class AddDuplicatedToToIssue < ActiveRecord::Migration[5.2]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_column :issues, :duplicated_to_id, :integer unless duplicated_to_id_exists?
+    add_concurrent_foreign_key :issues, :issues, column: :duplicated_to_id, on_delete: :nullify
+    add_concurrent_index :issues, :duplicated_to_id, where: 'duplicated_to_id IS NOT NULL'
+  end
+
+  def down
+    remove_foreign_key_without_error(:issues, column: :duplicated_to_id)
+    remove_concurrent_index(:issues, :duplicated_to_id)
+    remove_column(:issues, :duplicated_to_id) if duplicated_to_id_exists?
+  end
+
+  private
+
+  def duplicated_to_id_exists?
+    column_exists?(:issues, :duplicated_to_id)
+  end
+end

db/schema.rb

@@ -1808,10 +1808,12 @@ ActiveRecord::Schema.define(version: 2019_09_12_061145) do
     t.datetime_with_timezone "closed_at"
     t.integer "closed_by_id"
     t.integer "state_id", limit: 2
+    t.integer "duplicated_to_id"
     t.index ["author_id"], name: "index_issues_on_author_id"
     t.index ["closed_by_id"], name: "index_issues_on_closed_by_id"
     t.index ["confidential"], name: "index_issues_on_confidential"
     t.index ["description"], name: "index_issues_on_description_trigram", opclass: :gin_trgm_ops, using: :gin
+    t.index ["duplicated_to_id"], name: "index_issues_on_duplicated_to_id", where: "(duplicated_to_id IS NOT NULL)"
     t.index ["milestone_id"], name: "index_issues_on_milestone_id"
     t.index ["moved_to_id"], name: "index_issues_on_moved_to_id", where: "(moved_to_id IS NOT NULL)"
     t.index ["project_id", "created_at", "id", "state"], name: "index_issues_on_project_id_and_created_at_and_id_and_state"
@@ -3934,6 +3936,7 @@ ActiveRecord::Schema.define(version: 2019_09_12_061145) do
   add_foreign_key "issue_links", "issues", column: "target_id", name: "fk_e71bb44f1f", on_delete: :cascade
   add_foreign_key "issue_metrics", "issues", on_delete: :cascade
   add_foreign_key "issue_tracker_data", "services", on_delete: :cascade
+  add_foreign_key "issues", "issues", column: "duplicated_to_id", name: "fk_9c4516d665", on_delete: :nullify
   add_foreign_key "issues", "issues", column: "moved_to_id", name: "fk_a194299be1", on_delete: :nullify
   add_foreign_key "issues", "milestones", name: "fk_96b1dd429c", on_delete: :nullify
   add_foreign_key "issues", "projects", name: "fk_899c8f3231", on_delete: :cascade

lib/gitlab/hook_data/issue_builder.rb

@@ -24,6 +24,7 @@ module Gitlab
           last_edited_by_id
           milestone_id
           moved_to_id
+          duplicated_to_id
           project_id
           relative_position
           state

locale/gitlab.pot

@@ -8434,7 +8434,13 @@ msgstr ""
 msgid "Is using license seat:"
 msgstr ""
 
-msgid "IssuableStatus|Closed (%{moved_link_start}moved%{moved_link_end})"
+msgid "IssuableStatus|Closed"
+msgstr ""
+
+msgid "IssuableStatus|Closed (%{link})"
+msgstr ""
+
+msgid "IssuableStatus|duplicated"
 msgstr ""
 
 msgid "IssuableStatus|moved"

spec/helpers/issues_helper_spec.rb

@@ -142,7 +142,7 @@ describe IssuesHelper do
         expect(link_to_discussions_to_resolve(merge_request, nil)).to include(expected_path)
       end
 
-      it "containst the reference to the merge request" do
+      it "contains the reference to the merge request" do
         expect(link_to_discussions_to_resolve(merge_request, nil)).to include(merge_request.to_reference)
       end
     end
@@ -185,4 +185,79 @@ describe IssuesHelper do
       expect(helper.show_new_issue_link?(project)).to be_truthy
     end
   end
+
+  describe '#issue_closed_link' do
+    let(:new_issue) { create(:issue, project: project) }
+    let(:guest)     { create(:user) }
+
+    before do
+      allow(helper).to receive(:can?) do |*args|
+        Ability.allowed?(*args)
+      end
+    end
+
+    shared_examples 'successfully displays link to issue and with css class' do |action|
+      it 'returns link' do
+        link = "<a class=\"#{css_class}\" href=\"/#{new_issue.project.full_path}/issues/#{new_issue.iid}\">(#{action})</a>"
+
+        expect(helper.issue_closed_link(issue, user, css_class: css_class)).to match(link)
+      end
+    end
+
+    shared_examples 'does not display link' do
+      it 'returns nil' do
+        expect(helper.issue_closed_link(issue, user)).to be_nil
+      end
+    end
+
+    context 'with linked issue' do
+      context 'with moved issue' do
+        before do
+          issue.update(moved_to: new_issue)
+        end
+
+        context 'when user has permission to see new issue' do
+          let(:user)      { project.owner }
+          let(:css_class) { 'text-white text-underline' }
+
+          it_behaves_like 'successfully displays link to issue and with css class', 'moved'
+        end
+
+        context 'when user has no permission to see new issue' do
+          let(:user) { guest }
+
+          it_behaves_like 'does not display link'
+        end
+      end
+
+      context 'with duplicated issue' do
+        before do
+          issue.update(duplicated_to: new_issue)
+        end
+
+        context 'when user has permission to see new issue' do
+          let(:user)      { project.owner }
+          let(:css_class) { 'text-white text-underline' }
+
+          it_behaves_like 'successfully displays link to issue and with css class', 'duplicated'
+        end
+
+        context 'when user has no permission to see new issue' do
+          let(:user) { guest }
+
+          it_behaves_like 'does not display link'
+        end
+      end
+    end
+
+    context 'without linked issue' do
+      let(:user) { project.owner }
+
+      before do
+        issue.update(moved_to: nil, duplicated_to: nil)
+      end
+
+      it_behaves_like 'does not display link'
+    end
+  end
 end

spec/lib/gitlab/hook_data/issue_builder_spec.rb

@@ -23,6 +23,7 @@ describe Gitlab::HookData::IssueBuilder do
         last_edited_by_id
         milestone_id
         moved_to_id
+        duplicated_to_id
         project_id
         relative_position
         state

spec/lib/gitlab/import_export/all_models.yml

@@ -14,6 +14,7 @@ issues:
 - todos
 - user_agent_detail
 - moved_to
+- duplicated_to
 - events
 - merge_requests_closing_issues
 - metrics

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -19,6 +19,7 @@ Issue:
 - closed_by_id
 - due_date
 - moved_to_id
+- duplicated_to_id
 - lock_version
 - milestone_id
 - weight

spec/models/issue_spec.rb

@@ -7,6 +7,10 @@ describe Issue do
 
   describe "Associations" do
     it { is_expected.to belong_to(:milestone) }
+    it { is_expected.to belong_to(:project) }
+    it { is_expected.to belong_to(:moved_to).class_name('Issue') }
+    it { is_expected.to belong_to(:duplicated_to).class_name('Issue') }
+    it { is_expected.to belong_to(:closed_by).class_name('User') }
     it { is_expected.to have_many(:assignees) }
   end
 
@@ -310,6 +314,22 @@ describe Issue do
     end
   end
 
+  describe '#duplicated?' do
+    let(:issue) { create(:issue) }
+    subject { issue.duplicated? }
+
+    context 'issue not duplicated' do
+      it { is_expected.to eq false }
+    end
+
+    context 'issue already duplicated' do
+      let(:duplicated_to_issue) { create(:issue) }
+      let(:issue) { create(:issue, duplicated_to: duplicated_to_issue) }
+
+      it { is_expected.to eq true }
+    end
+  end
+
   describe '#suggested_branch_name' do
     let(:repository) { double }
 

spec/serializers/issue_entity_spec.rb

@@ -50,4 +50,44 @@ describe IssueEntity do
       end
     end
   end
+
+  context 'when issue got duplicated' do
+    let(:private_project) { create(:project, :private) }
+    let(:member) { create(:user) }
+    let(:issue) { create(:issue, project: project) }
+    let(:new_issue) { create(:issue, project: private_project) }
+
+    before do
+      Issues::DuplicateService
+        .new(project, member)
+        .execute(issue, new_issue)
+    end
+
+    context 'when user cannot read new issue' do
+      let(:non_member) { create(:user) }
+
+      it 'does not return duplicated_to_id' do
+        request = double('request', current_user: non_member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:duplicated_to_id]).to be_nil
+      end
+    end
+
+    context 'when user can read target project' do
+      before do
+        project.add_developer(member)
+        private_project.add_developer(member)
+      end
+
+      it 'returns duplicated duplicated_to_id' do
+        request = double('request', current_user: member)
+
+        response = described_class.new(issue, request: request).as_json
+
+        expect(response[:duplicated_to_id]).to eq(issue.duplicated_to_id)
+      end
+    end
+  end
 end

spec/services/issues/duplicate_service_spec.rb

@@ -77,6 +77,12 @@ describe Issues::DuplicateService do
 
         subject.execute(duplicate_issue, canonical_issue)
       end
+
+      it 'updates duplicate issue with canonical issue id' do
+        subject.execute(duplicate_issue, canonical_issue)
+
+        expect(duplicate_issue.reload.duplicated_to).to eq(canonical_issue)
+      end
     end
   end
 end

spec/views/projects/issues/show.html.haml_spec.rb

@@ -56,7 +56,41 @@ describe 'projects/issues/show' do
       end
     end
 
-    it 'shows "Closed" if an issue has not been moved' do
+    context 'when the issue was duplicated' do
+      let(:new_issue) { create(:issue, project: project, author: user) }
+
+      before do
+        issue.duplicated_to = new_issue
+      end
+
+      context 'when user can see the duplicated issue' do
+        before do
+          project.add_developer(user)
+        end
+
+        it 'shows "Closed (duplicated)" if an issue has been duplicated' do
+          render
+
+          expect(rendered).to have_selector('.status-box-issue-closed:not(.hidden)', text: 'Closed (duplicated)')
+        end
+
+        it 'links "duplicated" to the new issue the original issue was duplicated to' do
+          render
+
+          expect(rendered).to have_selector("a[href=\"#{issue_path(new_issue)}\"]", text: 'duplicated')
+        end
+      end
+
+      context 'when user cannot see duplicated issue' do
+        it 'does not show duplicated issue link' do
+          render
+
+          expect(rendered).not_to have_selector("a[href=\"#{issue_path(new_issue)}\"]", text: 'duplicated')
+        end
+      end
+    end
+
+    it 'shows "Closed" if an issue has not been moved or duplicated' do
       render
 
       expect(rendered).to have_selector('.status-box-issue-closed:not(.hidden)', text: 'Closed')