Merge branch '212566-foss-design-management-finders-helpers' into 'master'

Move Design Management helpers and finders to FOSS

See merge request gitlab-org/gitlab!31561

app/finders/todos_finder.rb

@@ -23,7 +23,7 @@ class TodosFinder
 
   NONE = '0'
 
-  TODO_TYPES = Set.new(%w(Issue MergeRequest)).freeze
+  TODO_TYPES = Set.new(%w(Issue MergeRequest DesignManagement::Design)).freeze
 
   attr_accessor :current_user, :params
 

app/helpers/events_helper.rb

@@ -167,6 +167,8 @@ module EventsHelper
       project_issue_url(event.project, id: event.note_target, anchor: dom_id(event.target))
     elsif event.merge_request_note?
       project_merge_request_url(event.project, id: event.note_target, anchor: dom_id(event.target))
+    elsif event.design_note?
+      design_url(event.note_target, anchor: dom_id(event.note))
     else
       polymorphic_url([event.project.namespace.becomes(Namespace),
                        event.project, event.note_target],
@@ -237,6 +239,16 @@ module EventsHelper
       concat content_tag(:span, event.author.to_reference, class: "username")
     end
   end
+
+  private
+
+  def design_url(design, opts)
+    designs_project_issue_url(
+      design.project,
+      design.issue,
+      opts.merge(vueroute: design.filename)
+    )
+  end
 end
 
 EventsHelper.prepend_if_ee('EE::EventsHelper')

app/helpers/export_helper.rb

@@ -9,7 +9,8 @@ module ExportHelper
       _('Project configuration, including services'),
       _('Issues with comments, merge requests with diffs and comments, labels, milestones, snippets, and other project entities'),
       _('LFS objects'),
-      _('Issue Boards')
+      _('Issue Boards'),
+      _('Design Management files and data')
     ]
   end
 

app/helpers/nav_helper.rb

@@ -54,7 +54,8 @@ module NavHelper
     current_path?('merge_requests#show') ||
       current_path?('projects/merge_requests/conflicts#show') ||
       current_path?('issues#show') ||
-      current_path?('milestones#show')
+      current_path?('milestones#show') ||
+      current_path?('issues#designs')
   end
 
   def admin_monitoring_nav_links

app/helpers/system_note_helper.rb

@@ -27,7 +27,11 @@ module SystemNoteHelper
     'locked' => 'lock',
     'unlocked' => 'lock-open',
     'due_date' => 'calendar',
-    'health_status' => 'status-health'
+    'health_status' => 'status-health',
+    'designs_added' => 'doc-image',
+    'designs_modified' => 'doc-image',
+    'designs_removed' => 'doc-image',
+    'designs_discussion_added' => 'doc-image'
   }.freeze
 
   def system_note_icon_name(note)

app/helpers/todos_helper.rb

@@ -53,6 +53,8 @@ module TodosHelper
   end
 
   def todo_target_type_name(todo)
+    return _('design') if todo.for_design?
+
     todo.target_type.titleize.downcase
   end
 
@@ -63,6 +65,8 @@ module TodosHelper
 
     if todo.for_commit?
       project_commit_path(todo.project, todo.target, path_options)
+    elsif todo.for_design?
+      todos_design_path(todo, path_options)
     else
       path = [todo.resource_parent, todo.target]
 
@@ -151,7 +155,8 @@ module TodosHelper
     [
       { id: '', text: 'Any Type' },
       { id: 'Issue', text: 'Issue' },
-      { id: 'MergeRequest', text: 'Merge Request' }
+      { id: 'MergeRequest', text: 'Merge Request' },
+      { id: 'DesignManagement::Design', text: 'Design' }
     ]
   end
 
@@ -188,6 +193,18 @@ module TodosHelper
 
   private
 
+  def todos_design_path(todo, path_options)
+    design = todo.target
+
+    designs_project_issue_path(
+      todo.resource_parent,
+      design.issue,
+      path_options.merge(
+        vueroute: design.filename
+      )
+    )
+  end
+
   def todo_action_subject(todo)
     todo.self_added? ? 'yourself' : 'you'
   end

app/models/design_management/action.rb

 # frozen_string_literal: true
 
+require_dependency 'design_management'
+
 module DesignManagement
   class Action < ApplicationRecord
     include WithUploads

ee/app/finders/ee/todos_finder.rb

@@ -4,7 +4,7 @@ module EE
   module TodosFinder
     extend ActiveSupport::Concern
 
-    EE_TODO_TYPES = (::TodosFinder::TODO_TYPES + %w[Epic DesignManagement::Design]).freeze
+    EE_TODO_TYPES = (::TodosFinder::TODO_TYPES + %w[Epic]).freeze
 
     class_methods do
       extend ::Gitlab::Utils::Override

ee/app/helpers/ee/events_helper.rb

@@ -6,23 +6,9 @@ module EE
 
     override :event_note_target_url
     def event_note_target_url(event)
-      if event.epic_note?
-        group_epic_url(event.group, event.note_target, anchor: dom_id(event.target))
-      elsif event.design_note?
-        design_url(event.note_target, anchor: dom_id(event.note))
-      else
-        super
-      end
-    end
-
-    private
+      return group_epic_url(event.group, event.note_target, anchor: dom_id(event.target)) if event.epic_note?
 
-    def design_url(design, opts)
-      designs_project_issue_url(
-        design.project,
-        design.issue,
-        opts.merge(vueroute: design.filename)
-      )
+      super
     end
   end
 end

ee/app/helpers/ee/export_helper.rb

@@ -4,11 +4,6 @@ module EE
   module ExportHelper
     extend ::Gitlab::Utils::Override
 
-    override :project_export_descriptions
-    def project_export_descriptions
-      super + [_('Design Management files and data')]
-    end
-
     override :group_export_descriptions
     def group_export_descriptions
       super + [_('Epics'), _('Events')]

ee/app/helpers/ee/nav_helper.rb

@@ -11,9 +11,7 @@ module EE
 
     override :page_has_markdown?
     def page_has_markdown?
-      super ||
-        current_path?('epics#show') ||
-        current_path?('issues#designs')
+      super || current_path?('epics#show')
     end
 
     override :admin_monitoring_nav_links

ee/app/helpers/ee/system_note_helper.rb

@@ -19,10 +19,6 @@ module EE
       'weight' => 'weight',
       'relate_epic' => 'epic',
       'unrelate_epic' => 'epic',
-      'designs_added' => 'doc-image',
-      'designs_modified' => 'doc-image',
-      'designs_removed' => 'doc-image',
-      'designs_discussion_added' => 'doc-image',
       'vulnerability_confirmed' => 'shield',
       'vulnerability_dismissed' => 'cancel',
       'vulnerability_resolved' => 'status_closed',

ee/app/helpers/ee/todos_helper.rb

@@ -4,45 +4,9 @@ module EE
   module TodosHelper
     extend ::Gitlab::Utils::Override
 
-    override :todo_target_path
-    def todo_target_path(todo)
-      return todos_design_path(todo) if todo.for_design?
-
-      super
-    end
-
-    override :todo_target_type_name
-    def todo_target_type_name(todo)
-      return _('design') if todo.for_design?
-
-      super
-    end
-
     override :todo_types_options
     def todo_types_options
-      super + ee_todo_types_options
-    end
-
-    private
-
-    def todos_design_path(todo)
-      design = todo.target
-      path_options = todo_target_path_options(todo).merge(
-        vueroute: design.filename
-      )
-
-      designs_project_issue_path(
-        todo.resource_parent,
-        design.issue,
-        path_options
-      )
-    end
-
-    def ee_todo_types_options
-      [
-        { id: 'Epic', text: 'Epic' },
-        { id: 'DesignManagement::Design', text: 'Design' }
-      ]
+      super + [{ id: 'Epic', text: 'Epic' }]
     end
   end
 end

ee/app/models/ee/project.rb

@@ -148,7 +148,7 @@ module EE
       scope :aimed_for_deletion, -> (date) { where('marked_for_deletion_at <= ?', date).without_deleted }
       scope :with_repos_templates, -> { where(namespace_id: ::Gitlab::CurrentSettings.current_application_settings.custom_project_templates_group_id) }
       scope :with_groups_level_repos_templates, -> { joins("INNER JOIN namespaces ON projects.namespace_id = namespaces.custom_project_templates_group_id") }
-      scope :with_designs, -> { where(id: DesignManagement::Design.select(:project_id)) }
+      scope :with_designs, -> { where(id: ::DesignManagement::Design.select(:project_id)) }
       scope :with_deleting_user, -> { includes(:deleting_user) }
       scope :with_compliance_framework_settings, -> { preload(:compliance_framework_setting) }
       scope :has_vulnerabilities, -> { joins(:vulnerabilities).group(:id) }

ee/spec/factories/events.rb

@@ -9,15 +9,5 @@ FactoryBot.modify do
       action { Event::CREATED }
       project { nil }
     end
-
-    trait :for_design do
-      transient do
-        design { create(:design, issue: create(:issue, project: project)) }
-        note { create(:note, author: author, project: project, noteable: design) }
-      end
-
-      action { Event::COMMENTED }
-      target { note }
-    end
   end
 end

ee/spec/helpers/ee/events_helper_spec.rb

@@ -4,20 +4,16 @@ require 'spec_helper'
 
 describe EventsHelper do
   describe '#event_note_target_url' do
-    let(:project) { event.project }
-    let(:project_base_url) { project_url(project) }
-
     subject { helper.event_note_target_url(event) }
 
-    context 'for design note events' do
-      let(:event) { create(:event, :for_design) }
+    context 'for epic note events' do
+      let_it_be(:group) { create(:group, :public) }
+      let_it_be(:event) { create(:event, group: group) }
 
-      it 'returns an appropriate URL' do
-        iid      = event.note_target.issue.iid
-        filename = event.note_target.filename
-        note_id  = event.target.id
+      it 'returns an epic url' do
+        event.target = create(:note_on_epic, note: 'foo')
 
-        expect(subject).to eq("#{project_base_url}/-/issues/#{iid}/designs/#{filename}#note_#{note_id}")
+        expect(subject).to match("#{group.to_param}/-/epics/#{event.note_target.iid}#note_#{event.target.id}")
       end
     end
   end

ee/spec/helpers/ee/todos_helper_spec.rb

@@ -3,59 +3,11 @@
 require 'spec_helper'
 
 describe ::TodosHelper do
-  let_it_be(:user) { create(:user) }
-  let_it_be(:author) { create(:user) }
-  let_it_be(:issue) { create(:issue) }
-  let_it_be(:design) { create(:design, issue: issue) }
-  let_it_be(:note) do
-    create(:note,
-           project: issue.project,
-           note: "I am note, hear me roar")
-  end
-  let_it_be(:design_todo) do
-    create(:todo, :mentioned,
-           user: user,
-           project: issue.project,
-           target: design,
-           author: author,
-           note: note)
-  end
-  let(:project) { issue.project }
-
-  describe '#todo_target_link' do
-    context 'when given a design' do
-      let(:todo) { design_todo }
-
-      it 'produces a good link' do
-        path = helper.todo_target_path(todo)
-        link = helper.todo_target_link(todo)
-        expected = "<a href=\"#{path}\">design #{design.to_reference}</a>"
-
-        expect(link).to eq(expected)
-      end
-    end
-  end
-
-  describe '#todo_target_path' do
-    context 'when given a design' do
-      let(:todo) { design_todo }
-
-      it 'responds with an appropriate path' do
-        path = helper.todo_target_path(todo)
-        issue_path = Gitlab::Routing.url_helpers
-          .project_issue_path(project, issue)
-
-        expect(path).to eq("#{issue_path}/designs/#{design.filename}##{dom_id(design_todo.note)}")
-      end
-    end
-  end
-
   describe '#todo_types_options' do
-    it 'includes a match for a design todo' do
-      options = helper.todo_types_options
-      design_option = options.find { |o| o[:id] == design_todo.target_type }
-
-      expect(design_option).to include(text: 'Design')
+    it 'includes options for an epic todo' do
+      expect(helper.todo_types_options).to include(
+        { id: 'Epic', text: 'Epic' }
+      )
     end
   end
 end

ee/spec/services/ee/system_notes/design_management_service_spec.rb

@@ -81,7 +81,7 @@ describe EE::SystemNotes::DesignManagementService do
         end
 
         it 'has a valid action' do
-          expect(EE::SystemNoteHelper::EE_ICON_NAMES_BY_ACTION)
+          expect(::SystemNoteHelper::ICON_NAMES_BY_ACTION)
             .to include(metadata.action)
         end
       end

spec/factories/events.rb

@@ -32,6 +32,16 @@ FactoryBot.define do
         wiki_page { create(:wiki_page, container: project) }
       end
     end
+
+    trait :for_design do
+      transient do
+        design { create(:design, issue: create(:issue, project: project)) }
+        note { create(:note, author: author, project: project, noteable: design) }
+      end
+
+      action { Event::COMMENTED }
+      target { note }
+    end
   end
 
   factory :push_event, class: 'PushEvent' do

spec/finders/todos_finder_spec.rb

@@ -260,9 +260,9 @@ describe TodosFinder do
     it 'returns the expected types' do
       expected_result =
         if Gitlab.ee?
-          %w[DesignManagement::Design Epic Issue MergeRequest]
+          %w[Epic Issue MergeRequest DesignManagement::Design]
         else
-          %w[Issue MergeRequest]
+          %w[Issue MergeRequest DesignManagement::Design]
         end
 
       expect(described_class.todo_types).to contain_exactly(*expected_result)

spec/helpers/events_helper_spec.rb

@@ -199,5 +199,17 @@ describe EventsHelper do
 
       expect(subject).to eq("#{project_base_url}/-/merge_requests/#{event.note_target.iid}#note_#{event.target.id}")
     end
+
+    context 'for design note events' do
+      let(:event) { create(:event, :for_design, project: project) }
+
+      it 'returns an appropriate URL' do
+        iid = event.note_target.issue.iid
+        filename = event.note_target.filename
+        note_id  = event.target.id
+
+        expect(subject).to eq("#{project_base_url}/-/issues/#{iid}/designs/#{filename}#note_#{note_id}")
+      end
+    end
   end
 end

spec/helpers/nav_helper_spec.rb

@@ -117,4 +117,27 @@ describe NavHelper, :do_not_mock_admin_mode do
 
     it { is_expected.to all(be_a(String)) }
   end
+
+  describe '#page_has_markdown?' do
+    using RSpec::Parameterized::TableSyntax
+
+    where path: %w(
+      merge_requests#show
+      projects/merge_requests/conflicts#show
+      issues#show
+      milestones#show
+      issues#designs
+    )
+
+    with_them do
+      before do
+        allow(helper).to receive(:current_path?).and_call_original
+        allow(helper).to receive(:current_path?).with(path).and_return(true)
+      end
+
+      subject { helper.page_has_markdown? }
+
+      it { is_expected.to eq(true) }
+    end
+  end
 end

spec/helpers/todos_helper_spec.rb

 # frozen_string_literal: true
 
-require "spec_helper"
+require 'spec_helper'
 
 describe TodosHelper do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:author) { create(:user) }
+  let_it_be(:issue) { create(:issue) }
+  let_it_be(:design) { create(:design, issue: issue) }
+  let_it_be(:note) do
+    create(:note,
+           project: issue.project,
+           note: 'I am note, hear me roar')
+  end
+  let_it_be(:design_todo) do
+    create(:todo, :mentioned,
+           user: user,
+           project: issue.project,
+           target: design,
+           author: author,
+           note: note)
+  end
+
   describe '#todos_count_format' do
     it 'shows fuzzy count for 100 or more items' do
       expect(helper.todos_count_format(100)).to eq '99+'
@@ -35,4 +53,53 @@ describe TodosHelper do
       expect(Gitlab::Json.parse(helper.todo_projects_options)).to match_array(expected_results)
     end
   end
+
+  describe '#todo_target_link' do
+    context 'when given a design' do
+      let(:todo) { design_todo }
+
+      it 'produces a good link' do
+        path = helper.todo_target_path(todo)
+        link = helper.todo_target_link(todo)
+        expected = "<a href=\"#{path}\">design #{design.to_reference}</a>"
+
+        expect(link).to eq(expected)
+      end
+    end
+  end
+
+  describe '#todo_target_path' do
+    context 'when given a design' do
+      let(:todo) { design_todo }
+
+      it 'responds with an appropriate path' do
+        path = helper.todo_target_path(todo)
+        issue_path = Gitlab::Routing.url_helpers
+          .project_issue_path(issue.project, issue)
+
+        expect(path).to eq("#{issue_path}/designs/#{design.filename}##{dom_id(design_todo.note)}")
+      end
+    end
+  end
+
+  describe '#todo_target_type_name' do
+    context 'when given a design todo' do
+      let(:todo) { design_todo }
+
+      it 'responds with an appropriate target type name' do
+        name = helper.todo_target_type_name(todo)
+
+        expect(name).to eq('design')
+      end
+    end
+  end
+
+  describe '#todo_types_options' do
+    it 'includes a match for a design todo' do
+      options = helper.todo_types_options
+      design_option = options.find { |o| o[:id] == design_todo.target_type }
+
+      expect(design_option).to include(text: 'Design')
+    end
+  end
 end