Commit 60e9405b authored by Jacopo's avatar Jacopo

Improve text on todo list when the todo action comes from yourself

Improved todo message when an issue is assigned to yourself.
- The new message when an user assign to himself an issue or a mr is: "You
assigned issue xx to yourself"
- The new message when an user adds a todo to himself is: "You added a
todo for issue xx"
- The new message when and user mention himself is: "You mentioned
yourself on issue xx"
- The new message when an user directly address himself is: "You
directly addressed yourself on issue xx"
- The new message when an user mark himself as an approver is: "You set
yourself as an approver for merge request xx"
parent 9a905e1b
...@@ -10,11 +10,12 @@ module EventsHelper ...@@ -10,11 +10,12 @@ module EventsHelper
'deleted' => 'icon_trash_o' 'deleted' => 'icon_trash_o'
}.freeze }.freeze
def link_to_author(event) def link_to_author(event, self_added: false)
author = event.author author = event.author
if author if author
link_to author.name, user_path(author.username), title: author.name name = self_added ? 'You' : author.name
link_to name, user_path(author.username), title: name
else else
event.author_name event.author_name
end end
......
...@@ -13,13 +13,13 @@ module TodosHelper ...@@ -13,13 +13,13 @@ module TodosHelper
def todo_action_name(todo) def todo_action_name(todo)
case todo.action case todo.action
when Todo::ASSIGNED then 'assigned you' when Todo::ASSIGNED then todo.self_added? ? 'assigned' : 'assigned you'
when Todo::MENTIONED then 'mentioned you on' when Todo::MENTIONED then "mentioned #{todo_action_subject(todo)} on"
when Todo::BUILD_FAILED then 'The build failed for' when Todo::BUILD_FAILED then 'The build failed for'
when Todo::MARKED then 'added a todo for' when Todo::MARKED then 'added a todo for'
when Todo::APPROVAL_REQUIRED then 'set you as an approver for' when Todo::APPROVAL_REQUIRED then "set #{todo_action_subject(todo)} as an approver for"
when Todo::UNMERGEABLE then 'Could not merge' when Todo::UNMERGEABLE then 'Could not merge'
when Todo::DIRECTLY_ADDRESSED then 'directly addressed you on' when Todo::DIRECTLY_ADDRESSED then "directly addressed #{todo_action_subject(todo)} on"
end end
end end
...@@ -148,6 +148,10 @@ module TodosHelper ...@@ -148,6 +148,10 @@ module TodosHelper
private private
def todo_action_subject(todo)
todo.self_added? ? 'yourself' : 'you'
end
def show_todo_state?(todo) def show_todo_state?(todo)
(todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state) (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state)
end end
......
...@@ -84,6 +84,10 @@ class Todo < ActiveRecord::Base ...@@ -84,6 +84,10 @@ class Todo < ActiveRecord::Base
action == BUILD_FAILED action == BUILD_FAILED
end end
def assigned?
action == ASSIGNED
end
def action_name def action_name
ACTION_NAMES[action] ACTION_NAMES[action]
end end
...@@ -117,6 +121,14 @@ class Todo < ActiveRecord::Base ...@@ -117,6 +121,14 @@ class Todo < ActiveRecord::Base
end end
end end
def self_added?
author == user
end
def self_assigned?
assigned? && self_added?
end
private private
def keep_around_commit def keep_around_commit
......
...@@ -9,7 +9,7 @@ ...@@ -9,7 +9,7 @@
.title-item.author-name .title-item.author-name
- if todo.author - if todo.author
= link_to_author(todo) = link_to_author(todo, self_added: todo.self_added?)
- else - else
(removed) (removed)
...@@ -22,6 +22,10 @@ ...@@ -22,6 +22,10 @@
- else - else
(removed) (removed)
- if todo.self_assigned?
.title-item.action-name
to yourself
.title-item .title-item
&middot; &middot;
......
---
title: Improve text on todo list when the todo action comes from yourself
merge_request: 10594
author: Jacopo Beschi @jacopo-beschi
...@@ -45,8 +45,8 @@ describe 'Dashboard > User filters todos', feature: true, js: true do ...@@ -45,8 +45,8 @@ describe 'Dashboard > User filters todos', feature: true, js: true do
wait_for_ajax wait_for_ajax
expect(find('.todos-list')).to have_content user_1.name expect(find('.todos-list')).to have_content 'merge request'
expect(find('.todos-list')).not_to have_content user_2.name expect(find('.todos-list')).not_to have_content 'issue'
end end
it "shows only authors of existing todos" do it "shows only authors of existing todos" do
......
...@@ -99,6 +99,83 @@ describe 'Dashboard Todos', feature: true do ...@@ -99,6 +99,83 @@ describe 'Dashboard Todos', feature: true do
end end
end end
context 'User created todos for themself' do
before do
login_as(user)
end
context 'issue assigned todo' do
before do
create(:todo, :assigned, user: user, project: project, target: issue, author: user)
visit dashboard_todos_path
end
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")
end
end
end
context 'marked todo' do
before do
create(:todo, :marked, user: user, project: project, target: issue, author: user)
visit dashboard_todos_path
end
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).not_to have_content('to yourself')
end
end
end
context 'mentioned todo' do
before do
create(:todo, :mentioned, user: user, project: project, target: issue, author: user)
visit dashboard_todos_path
end
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).not_to have_content('to yourself')
end
end
end
context 'directly_addressed todo' do
before do
create(:todo, :directly_addressed, user: user, project: project, target: issue, author: user)
visit dashboard_todos_path
end
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).not_to have_content('to yourself')
end
end
end
context 'approval todo' do
let(:merge_request) { create(:merge_request) }
before do
create(:todo, :approval_required, user: user, project: project, target: merge_request, author: user)
visit dashboard_todos_path
end
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).not_to have_content('to yourself')
end
end
end
end
context 'User has done todos', js: true do context 'User has done todos', js: true do
before do before do
create(:todo, :mentioned, :done, user: user, project: project, target: issue, author: author) create(:todo, :mentioned, :done, user: user, project: project, target: issue, author: author)
......
...@@ -125,4 +125,50 @@ describe Todo, models: true do ...@@ -125,4 +125,50 @@ describe Todo, models: true do
expect(subject.target_reference).to eq issue.to_reference(full: true) expect(subject.target_reference).to eq issue.to_reference(full: true)
end end
end end
describe '#self_added?' do
let(:user_1) { build(:user) }
before do
subject.user = user_1
end
it 'is true when the user is the author' do
subject.author = user_1
expect(subject).to be_self_added
end
it 'is false when the user is not the author' do
subject.author = build(:user)
expect(subject).not_to be_self_added
end
end
describe '#self_assigned?' do
let(:user_1) { build(:user) }
before do
subject.user = user_1
subject.author = user_1
subject.action = Todo::ASSIGNED
end
it 'is true when todo is ASSIGNED and self_added' do
expect(subject).to be_self_assigned
end
it 'is false when the todo is not ASSIGNED' do
subject.action = Todo::MENTIONED
expect(subject).not_to be_self_assigned
end
it 'is false when todo is not self_added' do
subject.author = build(:user)
expect(subject).not_to be_self_assigned
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