Commit fd0c40fc authored by Alfredo Sumaran's avatar Alfredo Sumaran Committed by Jacob Schatz

Address feedback

Signed-off-by: default avatarAlfredo Sumaran <alfredo@gitlab.com>
parent 7d7ded5e
...@@ -114,7 +114,6 @@ v 8.7.4 ...@@ -114,7 +114,6 @@ v 8.7.4
- Fix always showing build notification message when switching between merge requests - Fix always showing build notification message when switching between merge requests
- Links for Redmine issue references are generated correctly again (Benedikt Huss) - 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 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 v 8.7.3
- Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented - Emails, Gitlab::Email::Message, Gitlab::Diff, and Premailer::Adapter::Nokogiri are now instrumented
......
...@@ -38,18 +38,15 @@ module TodosHelper ...@@ -38,18 +38,15 @@ module TodosHelper
end end
def todo_target_state_pill(todo) def todo_target_state_pill(todo)
klass = 'status-box ' if show_todo_state?(todo)
klass << "status-box-#{todo.target.state.dasherize}" content_tag(:span, nil, class: 'target-status') do
content_tag(:span, nil, class: "status-box status-box-#{todo.target.state.dasherize}") do
content_tag(:span, nil, class: klass) do todo.target.state.capitalize
todo.target.state.capitalize end
end
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 def todos_filter_params
{ {
state: params[:state], state: params[:state],
...@@ -108,4 +105,10 @@ module TodosHelper ...@@ -108,4 +105,10 @@ module TodosHelper
options_from_collection_for_select(types, 'name', 'title', params[:type]) options_from_collection_for_select(types, 'name', 'title', params[:type])
end 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 end
...@@ -4,8 +4,7 @@ ...@@ -4,8 +4,7 @@
.todo-title.title .todo-title.title
- unless todo.build_failed? - unless todo.build_failed?
- if show_todo_state?(todo) - if show_todo_state?(todo)
%span.target-status = todo_target_state_pill(todo)
= todo_target_state_pill(todo)
%span.author-name %span.author-name
- if todo.author - if todo.author
......
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
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
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment