diff options
| -rw-r--r-- | app/assets/stylesheets/pages/todos.scss | 16 | ||||
| -rw-r--r-- | app/helpers/todos_helper.rb | 18 | ||||
| -rw-r--r-- | app/models/todo.rb | 4 | ||||
| -rw-r--r-- | app/views/dashboard/todos/_todo.html.haml | 34 | ||||
| -rw-r--r-- | changelogs/unreleased/todos-include-issue-mr-titles.yml | 5 | ||||
| -rw-r--r-- | locale/gitlab.pot | 3 | ||||
| -rw-r--r-- | spec/features/dashboard/todos/todos_filtering_spec.rb | 12 | ||||
| -rw-r--r-- | spec/features/dashboard/todos/todos_sorting_spec.rb | 36 | ||||
| -rw-r--r-- | spec/features/dashboard/todos/todos_spec.rb | 18 | ||||
| -rw-r--r-- | spec/models/todo_spec.rb | 4 |
10 files changed, 90 insertions, 60 deletions
diff --git a/app/assets/stylesheets/pages/todos.scss b/app/assets/stylesheets/pages/todos.scss index 7b64c67ae34..ece0ac04baf 100644 --- a/app/assets/stylesheets/pages/todos.scss +++ b/app/assets/stylesheets/pages/todos.scss @@ -72,12 +72,7 @@ @include transition(opacity); .todo-title { - display: flex; - > .title-item { - flex: 0 0 auto; - margin: 0 2px; - &:first-child { margin-left: 0; } @@ -105,8 +100,12 @@ font-size: 14px; } - .action-name { - font-weight: $gl-font-weight-normal; + .todo-label, + .todo-project { + a { + color: $blue-600; + font-weight: $gl-font-weight-normal; + } } .todo-body { @@ -170,7 +169,7 @@ } } -@include media-breakpoint-down(xs) { +@include media-breakpoint-down(sm) { .todo { .avatar { display: none; @@ -179,7 +178,6 @@ .todo-item { .todo-title { - flex-flow: row wrap; margin-bottom: 10px; .todo-label { diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 38142bc68cb..f5333bb332e 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -33,7 +33,23 @@ module TodosHelper todo.target_reference end - link_to text, todo_target_path(todo), class: 'has-tooltip', title: todo.target.title + link_to text, todo_target_path(todo) + end + + def todo_target_title(todo) + if todo.target + "\"#{todo.target.title}\"" + else + "" + end + end + + def todo_parent_path(todo) + if todo.parent.is_a?(Group) + link_to todo.parent.name, group_path(todo.parent) + else + link_to_project(todo.project) + end end def todo_target_type_name(todo) diff --git a/app/models/todo.rb b/app/models/todo.rb index 240c91da5b6..1ec04189482 100644 --- a/app/models/todo.rb +++ b/app/models/todo.rb @@ -186,9 +186,9 @@ class Todo < ApplicationRecord def target_reference if for_commit? - target.reference_link_text(full: true) + target.reference_link_text else - target.to_reference(full: true) + target.to_reference end end diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml index 8cdfc7369a0..fdb71d3a221 100644 --- a/app/views/dashboard/todos/_todo.html.haml +++ b/app/views/dashboard/todos/_todo.html.haml @@ -2,41 +2,49 @@ .todo-avatar = author_avatar(todo, size: 40) - .todo-item.todo-block - .todo-title.title + .todo-item.todo-block.align-self-center + .todo-title - unless todo.build_failed? || todo.unmergeable? = todo_target_state_pill(todo) - .title-item.author-name + %span.title-item.author-name.bold - if todo.author = link_to_author(todo, self_added: todo.self_added?) - else (removed) - .title-item.action-name + %span.title-item.action-name = todo_action_name(todo) - .title-item.todo-label + %span.title-item.todo-label.todo-target-link - if todo.target = todo_target_link(todo) - else - (removed) + = _("(removed)") + + %span.title-item.todo-target-title + = todo_target_title(todo) + + %span.title-item.todo-project.todo-label + at + = todo_parent_path(todo) - if todo.self_assigned? - .title-item.action-name + %span.title-item.action-name to yourself - .title-item + %span.title-item · - .title-item + %span.title-item.todo-timestamp #{time_ago_with_tooltip(todo.created_at)} = todo_due_date(todo) - .todo-body - .todo-note.break-word - .md - = first_line_in_markdown(todo, :body, 150, project: todo.project) + - if todo.note.present? + .todo-body + .todo-note.break-word + .md + = first_line_in_markdown(todo, :body, 150, project: todo.project) - if todo.pending? .todo-actions diff --git a/changelogs/unreleased/todos-include-issue-mr-titles.yml b/changelogs/unreleased/todos-include-issue-mr-titles.yml new file mode 100644 index 00000000000..a9fb9116070 --- /dev/null +++ b/changelogs/unreleased/todos-include-issue-mr-titles.yml @@ -0,0 +1,5 @@ +--- +title: Add Issue and Merge Request titles to Todo items +merge_request: 30435 +author: Arun Kumar Mohan +type: changed diff --git a/locale/gitlab.pot b/locale/gitlab.pot index e44c18979cd..c8827c683f8 100644 --- a/locale/gitlab.pot +++ b/locale/gitlab.pot @@ -309,6 +309,9 @@ msgstr "" msgid "(external source)" msgstr "" +msgid "(removed)" +msgstr "" + msgid "+ %{amount} more" msgstr "" diff --git a/spec/features/dashboard/todos/todos_filtering_spec.rb b/spec/features/dashboard/todos/todos_filtering_spec.rb index f273e416597..efa163042f9 100644 --- a/spec/features/dashboard/todos/todos_filtering_spec.rb +++ b/spec/features/dashboard/todos/todos_filtering_spec.rb @@ -31,9 +31,9 @@ describe 'Dashboard > User filters todos', :js do end it 'displays all todos without a filter' do - expect(page).to have_content issue1.to_reference(full: true) - expect(page).to have_content merge_request.to_reference(full: true) - expect(page).to have_content issue2.to_reference(full: true) + expect(page).to have_content issue1.to_reference(full: false) + expect(page).to have_content merge_request.to_reference(full: false) + expect(page).to have_content issue2.to_reference(full: false) end it 'filters by project' do @@ -58,9 +58,9 @@ describe 'Dashboard > User filters todos', :js do wait_for_requests - expect(page).to have_content issue1.to_reference(full: true) - expect(page).to have_content merge_request.to_reference(full: true) - expect(page).not_to have_content issue2.to_reference(full: true) + expect(page).to have_content "issue #{issue1.to_reference} \"issue\" at #{group1.name} / project_1" + expect(page).to have_content "merge request #{merge_request.to_reference}" + expect(page).not_to have_content "issue #{issue2.to_reference} \"issue\" at #{group2.name} / project_3" end context 'Author filter' do diff --git a/spec/features/dashboard/todos/todos_sorting_spec.rb b/spec/features/dashboard/todos/todos_sorting_spec.rb index 3870c661784..421a66c6d48 100644 --- a/spec/features/dashboard/todos/todos_sorting_spec.rb +++ b/spec/features/dashboard/todos/todos_sorting_spec.rb @@ -42,33 +42,33 @@ describe 'Dashboard > User sorts todos' do click_link 'Last created' results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content('merge_request_1') - expect(results_list.all('p')[1]).to have_content('issue_1') - expect(results_list.all('p')[2]).to have_content('issue_3') - expect(results_list.all('p')[3]).to have_content('issue_2') - expect(results_list.all('p')[4]).to have_content('issue_4') + expect(results_list.all('.todo-title')[0]).to have_content('merge_request_1') + expect(results_list.all('.todo-title')[1]).to have_content('issue_1') + expect(results_list.all('.todo-title')[2]).to have_content('issue_3') + expect(results_list.all('.todo-title')[3]).to have_content('issue_2') + expect(results_list.all('.todo-title')[4]).to have_content('issue_4') end it 'sorts with newest created todos first' do click_link 'Oldest created' results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content('issue_4') - expect(results_list.all('p')[1]).to have_content('issue_2') - expect(results_list.all('p')[2]).to have_content('issue_3') - expect(results_list.all('p')[3]).to have_content('issue_1') - expect(results_list.all('p')[4]).to have_content('merge_request_1') + expect(results_list.all('.todo-title')[0]).to have_content('issue_4') + expect(results_list.all('.todo-title')[1]).to have_content('issue_2') + expect(results_list.all('.todo-title')[2]).to have_content('issue_3') + expect(results_list.all('.todo-title')[3]).to have_content('issue_1') + expect(results_list.all('.todo-title')[4]).to have_content('merge_request_1') end it 'sorts by label priority' do click_link 'Label priority' results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content('issue_3') - expect(results_list.all('p')[1]).to have_content('merge_request_1') - expect(results_list.all('p')[2]).to have_content('issue_1') - expect(results_list.all('p')[3]).to have_content('issue_2') - expect(results_list.all('p')[4]).to have_content('issue_4') + expect(results_list.all('.todo-title')[0]).to have_content('issue_3') + expect(results_list.all('.todo-title')[1]).to have_content('merge_request_1') + expect(results_list.all('.todo-title')[2]).to have_content('issue_1') + expect(results_list.all('.todo-title')[3]).to have_content('issue_2') + expect(results_list.all('.todo-title')[4]).to have_content('issue_4') end end @@ -93,9 +93,9 @@ describe 'Dashboard > User sorts todos' do click_link 'Label priority' results_list = page.find('.todos-list') - expect(results_list.all('p')[0]).to have_content('issue_1') - expect(results_list.all('p')[1]).to have_content('issue_2') - expect(results_list.all('p')[2]).to have_content('merge_request_1') + expect(results_list.all('.todo-title')[0]).to have_content('issue_1') + expect(results_list.all('.todo-title')[1]).to have_content('issue_2') + expect(results_list.all('.todo-title')[2]).to have_content('merge_request_1') end end end diff --git a/spec/features/dashboard/todos/todos_spec.rb b/spec/features/dashboard/todos/todos_spec.rb index b98a04b0bda..867281da1e6 100644 --- a/spec/features/dashboard/todos/todos_spec.rb +++ b/spec/features/dashboard/todos/todos_spec.rb @@ -3,10 +3,10 @@ require 'spec_helper' describe 'Dashboard Todos' do - let(:user) { create(:user) } + let(:user) { create(:user, username: 'john') } let(:author) { create(:user) } let(:project) { create(:project, :public) } - let(:issue) { create(:issue, due_date: Date.today) } + let(:issue) { create(:issue, due_date: Date.today, title: "Fix bug") } context 'User does not have todos' do before do @@ -135,7 +135,7 @@ describe 'Dashboard Todos' do it 'shows issue assigned to yourself message' do page.within('.js-todos-all') do - expect(page).to have_content("You assigned issue #{issue.to_reference(full: true)} to yourself") + expect(page).to have_content("You assigned issue #{issue.to_reference} \"Fix bug\" at #{project.namespace.owner_name} / #{project.name} to yourself") end end end @@ -148,7 +148,7 @@ describe 'Dashboard Todos' do it 'shows you added a todo message' do page.within('.js-todos-all') do - expect(page).to have_content("You added a todo for issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You added a todo for issue #{issue.to_reference} \"Fix bug\" at #{project.namespace.owner_name} / #{project.name}") expect(page).not_to have_content('to yourself') end end @@ -162,7 +162,7 @@ describe 'Dashboard Todos' do it 'shows you mentioned yourself message' do page.within('.js-todos-all') do - expect(page).to have_content("You mentioned yourself on issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You mentioned yourself on issue #{issue.to_reference} \"Fix bug\" at #{project.namespace.owner_name} / #{project.name}") expect(page).not_to have_content('to yourself') end end @@ -176,14 +176,14 @@ describe 'Dashboard Todos' do it 'shows you directly addressed yourself message' do page.within('.js-todos-all') do - expect(page).to have_content("You directly addressed yourself on issue #{issue.to_reference(full: true)}") + expect(page).to have_content("You directly addressed yourself on issue #{issue.to_reference} \"Fix bug\" at #{project.namespace.owner_name} / #{project.name}") expect(page).not_to have_content('to yourself') end end end context 'approval todo' do - let(:merge_request) { create(:merge_request) } + let(:merge_request) { create(:merge_request, title: "Fixes issue") } before do create(:todo, :approval_required, user: user, project: project, target: merge_request, author: user) @@ -192,7 +192,7 @@ describe 'Dashboard Todos' do it 'shows you set yourself as an approver message' do page.within('.js-todos-all') do - expect(page).to have_content("You set yourself as an approver for merge request #{merge_request.to_reference(full: true)}") + expect(page).to have_content("You set yourself as an approver for merge request #{merge_request.to_reference} \"Fixes issue\" at #{project.namespace.owner_name} / #{project.name}") expect(page).not_to have_content('to yourself') end end @@ -354,7 +354,7 @@ describe 'Dashboard Todos' do it 'links to the pipelines for the merge request' do href = pipelines_project_merge_request_path(project, todo.target) - expect(page).to have_link "merge request #{todo.target.to_reference(full: true)}", href: href + expect(page).to have_link "merge request #{todo.target.to_reference}", href: href end end end diff --git a/spec/models/todo_spec.rb b/spec/models/todo_spec.rb index 9aeef7c3b4b..ce17704acbd 100644 --- a/spec/models/todo_spec.rb +++ b/spec/models/todo_spec.rb @@ -121,12 +121,12 @@ describe Todo do subject.target_type = 'Commit' subject.commit_id = commit.id - expect(subject.target_reference).to eq commit.reference_link_text(full: true) + expect(subject.target_reference).to eq commit.reference_link_text(full: false) end it 'returns full reference for issuables' do subject.target = issue - expect(subject.target_reference).to eq issue.to_reference(full: true) + expect(subject.target_reference).to eq issue.to_reference(full: false) end end |