diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 8529959f73cb59832d5a09989923eee7063fde20..c4cfd3b2287a8a6ff446d8cf2f394875e72b2b30 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -66,7 +66,13 @@ def todo_target_type_name(todo) return _('design') if todo.for_design? return _('alert') if todo.for_alert? - todo.target_type.titleize.downcase + target_type = if todo.for_issue_or_work_item? + todo.target.issue_type + else + todo.target_type + end + + target_type.titleize.downcase end def todo_target_path(todo) @@ -80,6 +86,8 @@ def todo_target_path(todo) todos_design_path(todo, path_options) elsif todo.for_alert? details_project_alert_management_path(todo.project, todo.target) + elsif todo.for_issue_or_work_item? + Gitlab::UrlBuilder.build(todo.target, only_path: true) else path = [todo.resource_parent, todo.target] diff --git a/app/models/todo.rb b/app/models/todo.rb index 45ab770a0f6c5b9cc2e80cfdeb7537c0aa83d352..cff7a93f72f3b8d2600b6c9cd1bd14ab2830fb8f 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -230,6 +230,10 @@ def for_alert? target_type == AlertManagement::Alert.name end + def for_issue_or_work_item? + [Issue.name, WorkItem.name].any? { |klass_name| target_type == klass_name } + end + # override to return commits, which are not active record def target if for_commit? diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index adb43d60306cb3384a8d5b2306d72d14c7929c5a..e02cd182b2c4dc37ecf198d92d86646a86c3465a 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -60,6 +60,21 @@ end end + context 'when todo references an issue of type task' do + let(:task) { create(:issue, :task, project: project) } + let!(:task_todo) { create(:todo, :mentioned, user: user, project: project, target: task, author: author) } + + before do + sign_in(user) + + visit dashboard_todos_path + end + + it 'displays the correct issue type name' do + expect(page).to have_content('mentioned you on task') + end + end + context 'user has an unauthorized todo' do before do sign_in(user) @@ -85,6 +100,10 @@ visit dashboard_todos_path end + it 'displays the correct issue type name' do + expect(page).to have_content('mentioned you on issue') + end + it 'has todo present' do expect(page).to have_selector('.todos-list .todo', count: 1) end diff --git a/spec/helpers/todos_helper_spec.rb b/spec/helpers/todos_helper_spec.rb index 922fb1d7c921aad5d6a071b7b65288ed4cf8da36..73a454fd9f74fe18a6da9e2ef866a50df810a831 100644 --- a/spec/helpers/todos_helper_spec.rb +++ b/spec/helpers/todos_helper_spec.rb @@ -5,7 +5,8 @@ RSpec.describe TodosHelper do let_it_be(:user) { create(:user) } let_it_be(:author) { create(:user) } - let_it_be(:issue) { create(:issue, title: 'Issue 1') } + let_it_be(:project) { create(:project) } + let_it_be(:issue) { create(:issue, title: 'Issue 1', project: project) } let_it_be(:design) { create(:design, issue: issue) } let_it_be(:note) do create(:note, @@ -16,7 +17,7 @@ let_it_be(:design_todo) do create(:todo, :mentioned, user: user, - project: issue.project, + project: project, target: design, author: author, note: note) @@ -27,6 +28,15 @@ create(:todo, target: alert) end + let_it_be(:task_todo) do + task = create(:work_item, :task, project: project) + create(:todo, target: task, target_type: task.class.name, project: project) + end + + let_it_be(:issue_todo) do + create(:todo, target: issue) + 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+' @@ -113,27 +123,52 @@ ) end end + + context 'when given a task' do + let(:todo) { task_todo } + + it 'responds with an appropriate path' do + path = helper.todo_target_path(todo) + + expect(path).to eq("/#{todo.project.full_path}/-/work_items/#{todo.target.id}") + end + end end describe '#todo_target_type_name' do + subject { helper.todo_target_type_name(todo) } + 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 + it { is_expected.to eq('design') } end context 'when given an alert todo' do let(:todo) { alert_todo } - it 'responds with an appropriate target type name' do - name = helper.todo_target_type_name(todo) + it { is_expected.to eq('alert') } + end + + context 'when given a task todo' do + let(:todo) { task_todo } + + it { is_expected.to eq('task') } + end + + context 'when given an issue todo' do + let(:todo) { issue_todo } + + it { is_expected.to eq('issue') } + end - expect(name).to eq('alert') + context 'when given a merge request todo' do + let(:todo) do + merge_request = create(:merge_request, source_project: project) + create(:todo, target: merge_request) end + + it { is_expected.to eq('merge request') } end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 651e2cf273f992355c91c80c3873c9204201ff77..7df22078c6da9b4093fc41696a73fc981a8818c0 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -114,6 +114,26 @@ end end + describe '#for_issue_or_work_item?' do + it 'returns true when target is an Issue' do + subject.target_type = 'Issue' + + expect(subject.for_issue_or_work_item?).to be_truthy + end + + it 'returns true when target is a WorkItem' do + subject.target_type = 'WorkItem' + + expect(subject.for_issue_or_work_item?).to be_truthy + end + + it 'returns false when target is not an Issue' do + subject.target_type = 'DesignManagement::Design' + + expect(subject.for_issue_or_work_item?).to be_falsey + end + end + describe '#target' do context 'for commits' do let(:project) { create(:project, :repository) }