diff options
| -rw-r--r-- | CHANGELOG | 1 | ||||
| -rw-r--r-- | app/helpers/todos_helper.rb | 21 | ||||
| -rw-r--r-- | app/views/dashboard/todos/_todo.html.haml | 3 | ||||
| -rw-r--r-- | spec/features/todos/target_state_spec.rb | 65 | ||||
| -rw-r--r-- | spec/features/todos/target_state_todos_spec.rb | 84 |
5 files changed, 78 insertions, 96 deletions
diff --git a/CHANGELOG b/CHANGELOG index f18867c5379..57020ca9ca1 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -114,7 +114,6 @@ v 8.7.4 - Fix always showing build notification message when switching between merge requests - Links for Redmine issue references are generated correctly again (Benedikt Huss) - Fix an issue when filtering merge requests with more than one label. !3886 - - Fix links on wiki pages for relative url setups. !4026 (Artem Sidorenko) v 8.7.3 - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented diff --git a/app/helpers/todos_helper.rb b/app/helpers/todos_helper.rb index 150d826ac44..27f33b3eded 100644 --- a/app/helpers/todos_helper.rb +++ b/app/helpers/todos_helper.rb @@ -38,18 +38,15 @@ module TodosHelper end def todo_target_state_pill(todo) - klass = 'status-box ' - klass << "status-box-#{todo.target.state.dasherize}" - - content_tag(:span, nil, class: klass) do - todo.target.state.capitalize + if show_todo_state?(todo) + content_tag(:span, nil, class: 'target-status') do + content_tag(:span, nil, class: "status-box status-box-#{todo.target.state.dasherize}") do + todo.target.state.capitalize + end + end end end - def show_todo_state?(todo) - (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && ['closed', 'merged'].include?(todo.target.state) - end - def todos_filter_params { state: params[:state], @@ -108,4 +105,10 @@ module TodosHelper options_from_collection_for_select(types, 'name', 'title', params[:type]) end + + private + + def show_todo_state?(todo) + (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && ['closed', 'merged'].include?(todo.target.state) + end end diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml index f544f87b581..db567d6df3b 100644 --- a/app/views/dashboard/todos/_todo.html.haml +++ b/app/views/dashboard/todos/_todo.html.haml @@ -4,8 +4,7 @@ .todo-title.title - unless todo.build_failed? - if show_todo_state?(todo) - %span.target-status - = todo_target_state_pill(todo) + = todo_target_state_pill(todo) %span.author-name - if todo.author diff --git a/spec/features/todos/target_state_spec.rb b/spec/features/todos/target_state_spec.rb new file mode 100644 index 00000000000..604b3ee4056 --- /dev/null +++ b/spec/features/todos/target_state_spec.rb @@ -0,0 +1,65 @@ +require 'rails_helper' + +feature 'Todo target states', feature: true do + let(:user) { create(:user) } + let(:author) { create(:user) } + let(:project) { create(:project) } + + before do + login_as user + end + + scenario 'on a closed issue todo has closed label' do + issue_closed = create(:issue, state: 'closed') + create_todo issue_closed + visit dashboard_todos_path + + page.within '.todos-list' do + expect(page).to have_content('Closed') + end + end + + scenario 'on an open issue todo does not have an open label' do + issue_open = create(:issue) + create_todo issue_open + visit dashboard_todos_path + + page.within '.todos-list' do + expect(page).not_to have_content('Open') + end + end + + scenario 'on a merged merge request todo has merged label' do + mr_merged = create(:merge_request, :simple, author: user, state: 'merged') + create_todo mr_merged + visit dashboard_todos_path + + page.within '.todos-list' do + expect(page).to have_content('Merged') + end + end + + scenario 'on a closed merge request todo has closed label' do + mr_closed = create(:merge_request, :simple, author: user, state: 'closed') + create_todo mr_closed + visit dashboard_todos_path + + page.within '.todos-list' do + expect(page).to have_content('Closed') + end + end + + scenario 'on an open merge request todo does not have an open label' do + mr_open = create(:merge_request, :simple, author: user) + create_todo mr_open + visit dashboard_todos_path + + page.within '.todos-list' do + expect(page).not_to have_content('Open') + end + end + + def create_todo(target) + create(:todo, :mentioned, user: user, project: project, target: target, author: author) + end +end diff --git a/spec/features/todos/target_state_todos_spec.rb b/spec/features/todos/target_state_todos_spec.rb deleted file mode 100644 index c7c5cdb66b0..00000000000 --- a/spec/features/todos/target_state_todos_spec.rb +++ /dev/null @@ -1,84 +0,0 @@ -require 'rails_helper' - -describe 'Todos > Target State Labels' do - let(:user) { create(:user) } - let(:author) { create(:user) } - let(:project) { create(:project) } - let(:issue_open) { create(:issue) } - let(:issue_closed) { create(:issue, state: 'closed') } - let(:mr_open) { create(:merge_request, :simple, author: user) } - let(:mr_merged) { create(:merge_request, :simple, author: user, state: 'merged') } - let(:mr_closed) { create(:merge_request, :simple, author: user, state: 'closed') } - - describe 'GET /dashboard/todos' do - context 'On a todo for a Closed Issue' do - before do - create(:todo, :mentioned, user: user, project: project, target: issue_closed, author: author) - login_as user - visit dashboard_todos_path - end - - it 'has closed label' do - page.within '.todos-list' do - expect(page).to have_content('Closed') - end - end - end - - context 'On a todo for a Open Issue' do - before do - create(:todo, :mentioned, user: user, project: project, target: issue_open, author: author) - login_as user - visit dashboard_todos_path - end - - it 'does not have a open label' do - page.within '.todos-list' do - expect(page).not_to have_content('Open') - end - end - end - - context 'On a todo for a merged Merge Request' do - before do - create(:todo, :mentioned, user: user, project: project, target: mr_merged, author: author) - login_as user - visit dashboard_todos_path - end - - it 'has merged label' do - page.within '.todos-list' do - expect(page).to have_content('Merged') - end - end - end - - context 'On a todo for a closed Merge Request' do - before do - create(:todo, :mentioned, user: user, project: project, target: mr_closed, author: author) - login_as user - visit dashboard_todos_path - end - - it 'has closed label' do - page.within '.todos-list' do - expect(page).to have_content('Closed') - end - end - end - - context 'On a todo for a open Merge Request' do - before do - create(:todo, :mentioned, user: user, project: project, target: mr_open, author: author) - login_as user - visit dashboard_todos_path - end - - it 'does not have a open label' do - page.within '.todos-list' do - expect(page).not_to have_content('Open') - end - end - end - end -end |