Fix todos dashboard rendering

This adds supports for Todos to link to Designs.

Add designs as a dropdown option

https://gitlab.com/gitlab-org/gitlab-ee/issues/13494
https://gitlab.com/gitlab-org/gitlab-ee/merge_requests/15129Co-Authored-By: Alex Kalderimis <akalderimis@gitlab.com>
Co-Authored-By: Luke Duncalfe <lduncalfe@eml.cc>

app/helpers/todos_helper.rb

@@ -189,4 +189,4 @@ module TodosHelper
   end
 end
 
-TodosHelper.prepend_if_ee('EE::NotesHelper')
+TodosHelper.prepend_if_ee('EE::NotesHelper'); TodosHelper.prepend_if_ee('EE::TodosHelper') # rubocop: disable Style/Semicolon

ee/app/helpers/ee/todos_helper.rb

@@ -4,9 +4,45 @@ 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 << { id: 'Epic', text: 'Epic' }
+      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.parent,
+        design.issue,
+        path_options
+      )
+    end
+
+    def ee_todo_types_options
+      [
+        { id: 'Epic', text: 'Epic' },
+        { id: 'DesignManagement::Design', text: 'Design' }
+      ]
     end
   end
 end

ee/app/models/concerns/ee/noteable.rb

@@ -19,8 +19,11 @@ module EE
 
     override :note_etag_key
     def note_etag_key
-      if self.is_a?(Epic)
+      case self
+      when Epic
         ::Gitlab::Routing.url_helpers.group_epic_notes_path(group, self)
+      when DesignManagement::Design
+        ::Gitlab::Routing.url_helpers.designs_project_issue_path(project, issue, { vueroute: filename })
       else
         super
       end

ee/app/models/design_management/design.rb

@@ -18,6 +18,8 @@ module DesignManagement
     validates :filename, uniqueness: { scope: :issue_id }
     validate :validate_file_is_image
 
+    alias_attribute :title, :filename
+
     scope :visible_at_version, -> (version) do
       created_before_version = DesignManagement::DesignVersion.select(1)
                                .where("#{table_name}.id = #{DesignManagement::DesignVersion.table_name}.design_id")
@@ -26,6 +28,14 @@ module DesignManagement
       where('EXISTS(?)', created_before_version)
     end
 
+    def to_reference(_opts)
+      filename
+    end
+
+    def description
+      ''
+    end
+
     def new_design?
       versions.none?
     end

ee/app/models/ee/todo.rb

@@ -8,5 +8,9 @@ module EE
     def parent
       project || group
     end
+
+    def for_design?
+      target_type == DesignManagement::Design.name
+    end
   end
 end

ee/lib/ee/api/todos.rb

@@ -7,6 +7,8 @@ module EE
 
       prepended do
         helpers do
+          extend ::Gitlab::Utils::Override
+
           # rubocop: disable CodeReuse/ActiveRecord
           def epic
             @epic ||= user_group.epics.find_by(iid: params[:epic_iid])
@@ -16,6 +18,14 @@ module EE
           def authorize_can_read!
             authorize!(:read_epic, epic)
           end
+
+          override :find_todos
+          def find_todos
+            # Todos for Designs are intentionally not supported yet.
+            # https://gitlab.com/gitlab-org/gitlab-ee/issues/13543
+            # https://gitlab.com/gitlab-org/gitlab-ee/issues/13494#note_203518559
+            super.where.not(target_type: ::DesignManagement::Design.name) # rubocop: disable CodeReuse/ActiveRecord
+          end
         end
 
         resource :groups, requirements: ::API::API::NAMESPACE_OR_PROJECT_REQUIREMENTS do

ee/spec/features/dashboards/todos_spec.rb

@@ -3,8 +3,45 @@
 require 'spec_helper'
 
 describe 'Dashboard todos' do
-  let(:user) { create(:user) }
+  set(:user) { create(:user) }
+  set(:author)  { create(:user) }
+  set(:project) { create(:project, :public) }
+  set(:issue) { create(:issue, project: project) }
   let(:page_path) { dashboard_todos_path }
 
   it_behaves_like 'dashboard gold trial callout'
+
+  context 'User has a todo regarding a design' do
+    set(:target) { create(:design, issue: issue) }
+    set(:note) { create(:note, project: project, note: "I am note, hear me roar") }
+    set(:todo) do
+      create(:todo, :mentioned,
+             user: user,
+             project: project,
+             target: target,
+             author: author,
+             note: note)
+    end
+
+    before do
+      sign_in(user)
+      project.add_developer(user)
+
+      visit page_path
+    end
+
+    it 'has todo present' do
+      expect(page).to have_selector('.todos-list .todo', count: 1)
+    end
+
+    it 'has a link that will take me to the design page', :js do
+      click_link "design #{target.filename}"
+
+      expectation = Gitlab::Routing.url_helpers.designs_project_issue_path(
+        target.project, target.issue, target.filename
+      )
+
+      expect(current_path).to eq(expectation)
+    end
+  end
 end

ee/spec/helpers/ee/todos_helper_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ::TodosHelper do
+  set(:user) { create(:user) }
+  set(:author) { create(:user) }
+  set(:issue) { create(:issue) }
+  set(:design) { create(:design, issue: issue) }
+  set(:note) do
+    create(:note,
+           project: issue.project,
+           note: "I am note, hear me roar")
+  end
+  set(: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 class=\"has-tooltip\" title=\"#{design.filename}\"",
+                    "href=\"#{path}\">design #{design.filename}</a>"].join(' ')
+
+        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')
+    end
+  end
+end

ee/spec/models/design_management/design_spec.rb

@@ -106,4 +106,14 @@ describe DesignManagement::Design do
       expect(design.repository).to be_a(DesignManagement::Repository)
     end
   end
+
+  describe '#note_etag_key' do
+    it 'returns a correct etag key' do
+      design = create(:design)
+
+      expect(design.note_etag_key).to eq(
+        ::Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, { vueroute: design.filename })
+      )
+    end
+  end
 end

ee/spec/models/ee/todo_spec.rb

+require 'spec_helper'
+
+describe Todo do
+  describe '#for_design?' do
+    it 'returns true when target is a Design' do
+      todo = build(:todo, target: build(:design))
+
+      expect(todo.for_design?).to eq(true)
+    end
+
+    it 'returns false when target is not a Design' do
+      todo = build(:todo)
+
+      expect(todo.for_design?).to eq(false)
+    end
+  end
+end

ee/spec/requests/api/todos_spec.rb

 require 'spec_helper'
 
 describe API::Todos do
-  let(:group) { create(:group) }
+  set(:group) { create(:group) }
   let(:user) { create(:user) }
   let(:epic) { create(:epic, group: group) }
+  set(:project) { create(:project, group: group) }
 
   describe 'GET /todos' do
     let(:author_1) { create(:user) }
@@ -12,8 +13,6 @@ describe API::Todos do
     before do
       group.add_developer(user)
       group.add_developer(author_1)
-
-      create_todo_for_new_epic
     end
 
     def create_todo_for_new_epic
@@ -25,18 +24,58 @@ describe API::Todos do
       create(:todo, project: nil, group: new_group, author: author_1, user: user, target: new_epic)
     end
 
-    it 'avoids N+1 queries', :request_store do
-      create_todo_for_new_epic
+    def create_todo_for_mentioned_in_design
+      issue = create(:issue, project: project)
+      create(:todo, :mentioned,
+             user: user,
+             project: project,
+             target: create(:design, issue: issue),
+             author: create(:user),
+             note: create(:note, project: project, note: "I am note, hear me roar"))
+    end
+
+    context 'when there is an Epic Todo' do
+      let!(:epic_todo) { create_todo_for_new_epic }
+
+      before do
+        get api('/todos', personal_access_token: pat)
+      end
+
+      it 'responds without error' do
+        expect(response.status).to eq(200)
+      end
 
-      get api('/todos', personal_access_token: pat)
+      it 'avoids N+1 queries', :request_store do
+        create_todo_for_new_epic
 
-      control = ActiveRecord::QueryRecorder.new { get api('/todos', personal_access_token: pat) }
+        control = ActiveRecord::QueryRecorder.new { get api('/todos', personal_access_token: pat) }
 
-      create_todo_for_new_epic
+        create_todo_for_new_epic
 
-      expect { get api('/todos', personal_access_token: pat) }.not_to exceed_query_limit(control)
+        expect { get api('/todos', personal_access_token: pat) }.not_to exceed_query_limit(control)
+      end
 
-      expect(response.status).to eq(200)
+      it 'includes the Epic Todo in the response' do
+        expect(json_response).to include(
+          a_hash_including('id' => epic_todo.id)
+        )
+      end
+    end
+
+    context 'when there is a Design Todo' do
+      let!(:design_todo) { create_todo_for_mentioned_in_design }
+
+      before do
+        get api('/todos', personal_access_token: pat)
+      end
+
+      it 'responds without error' do
+        expect(response.status).to eq(200)
+      end
+
+      it 'does not include the Design Todo in the response' do
+        expect(json_response).to be_empty
+      end
     end
   end
 

locale/gitlab.pot

@@ -17934,6 +17934,9 @@ msgstr[1] ""
 msgid "deleted"
 msgstr ""
 
+msgid "design"
+msgstr ""
+
 msgid "detached"
 msgstr ""