Merge branch '28020-improve-todo-list-when-comes-from-yourself' into 'master'

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

Closes #28020

See merge request !10594

app/helpers/events_helper.rb

@@ -10,11 +10,12 @@ module EventsHelper
     'deleted' => 'icon_trash_o'
   }.freeze
 
-  def link_to_author(event)
+  def link_to_author(event, self_added: false)
     author = event.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
       event.author_name
     end

app/helpers/todos_helper.rb

@@ -13,13 +13,13 @@ module TodosHelper
 
   def todo_action_name(todo)
     case todo.action
-    when Todo::ASSIGNED then 'assigned you'
-    when Todo::MENTIONED then 'mentioned you on'
+    when Todo::ASSIGNED then todo.self_added? ? 'assigned' : 'assigned you'
+    when Todo::MENTIONED then "mentioned #{todo_action_subject(todo)} on"
     when Todo::BUILD_FAILED then 'The build failed 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::DIRECTLY_ADDRESSED then 'directly addressed you on'
+    when Todo::DIRECTLY_ADDRESSED then "directly addressed #{todo_action_subject(todo)} on"
     end
   end
 
@@ -148,6 +148,10 @@ module TodosHelper
 
   private
 
+  def todo_action_subject(todo)
+    todo.self_added? ? 'yourself' : 'you'
+  end
+
   def show_todo_state?(todo)
     (todo.target.is_a?(MergeRequest) || todo.target.is_a?(Issue)) && %w(closed merged).include?(todo.target.state)
   end

app/models/todo.rb

@@ -84,6 +84,10 @@ class Todo < ActiveRecord::Base
     action == BUILD_FAILED
   end
 
+  def assigned?
+    action == ASSIGNED
+  end
+
   def action_name
     ACTION_NAMES[action]
   end
@@ -117,6 +121,14 @@ class Todo < ActiveRecord::Base
     end
   end
 
+  def self_added?
+    author == user
+  end
+
+  def self_assigned?
+    assigned? && self_added?
+  end
+
   private
 
   def keep_around_commit

app/views/dashboard/todos/_todo.html.haml

@@ -9,7 +9,7 @@
 
         .title-item.author-name
           - if todo.author
-            = link_to_author(todo)
+            = link_to_author(todo, self_added: todo.self_added?)
           - else
             (removed)
 
@@ -22,6 +22,10 @@
         - else
           (removed)
 
+      - if todo.self_assigned?
+        .title-item.action-name
+          to yourself
+
       .title-item
         ·
 

changelogs/unreleased/28020-improve-todo-list-when-comes-from-yourself.yml

+---
+title: Improve text on todo list when the todo action comes from yourself
+merge_request: 10594
+author: Jacopo Beschi @jacopo-beschi

spec/features/todos/todos_filtering_spec.rb

@@ -45,8 +45,8 @@ describe 'Dashboard > User filters todos', feature: true, js: true do
 
       wait_for_ajax
 
-      expect(find('.todos-list')).to     have_content user_1.name
-      expect(find('.todos-list')).not_to have_content user_2.name
+      expect(find('.todos-list')).to     have_content 'merge request'
+      expect(find('.todos-list')).not_to have_content 'issue'
     end
 
     it "shows only authors of existing todos" do

spec/features/todos/todos_spec.rb

@@ -99,6 +99,83 @@ describe 'Dashboard Todos', feature: true do
       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
       before do
         create(:todo, :mentioned, :done, user: user, project: project, target: issue, author: author)

spec/models/todo_spec.rb

@@ -125,4 +125,50 @@ describe Todo, models: true do
       expect(subject.target_reference).to eq issue.to_reference(full: true)
     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