Merge branch '27114-add-undo-mark-all-as-done-to-todos' into 'master'

Add 'Undo mark all as done' to Todos

Closes #27114

See merge request !9890

12 files changed, 166 insertions, 55 deletions

diff --git a/app/assets/javascripts/todos.js b/app/assets/javascripts/todos.js
index caaf6484a34..8be58023c84 100644
--- a/app/assets/javascripts/todos.js
+++ b/app/assets/javascripts/todos.js
@@ -5,6 +5,7 @@ class Todos {
   constructor() {
     this.initFilters();
     this.bindEvents();
+    this.todo_ids = [];
 
     this.cleanupWrapper = this.cleanup.bind(this);
     document.addEventListener('beforeunload', this.cleanupWrapper);
@@ -17,16 +18,16 @@ class Todos {
 
   unbindEvents() {
     $('.js-done-todo, .js-undo-todo, .js-add-todo').off('click', this.updateRowStateClickedWrapper);
-    $('.js-todos-mark-all').off('click', this.allDoneClickedWrapper);
+    $('.js-todos-mark-all', '.js-todos-undo-all').off('click', this.updateallStateClickedWrapper);
     $('.todo').off('click', this.goToTodoUrl);
   }
 
   bindEvents() {
     this.updateRowStateClickedWrapper = this.updateRowStateClicked.bind(this);
-    this.allDoneClickedWrapper = this.allDoneClicked.bind(this);
+    this.updateAllStateClickedWrapper = this.updateAllStateClicked.bind(this);
 
     $('.js-done-todo, .js-undo-todo, .js-add-todo').on('click', this.updateRowStateClickedWrapper);
-    $('.js-todos-mark-all').on('click', this.allDoneClickedWrapper);
+    $('.js-todos-mark-all, .js-todos-undo-all').on('click', this.updateAllStateClickedWrapper);
     $('.todo').on('click', this.goToTodoUrl);
   }
 
@@ -57,14 +58,14 @@ class Todos {
     e.preventDefault();
 
     const target = e.target;
-    target.setAttribute('disabled', '');
+    target.setAttribute('disabled', true);
     target.classList.add('disabled');
     $.ajax({
       type: 'POST',
-      url: target.getAttribute('href'),
+      url: target.dataset.href,
       dataType: 'json',
       data: {
-        '_method': target.getAttribute('data-method'),
+        '_method': target.dataset.method,
       },
       success: (data) => {
         this.updateRowState(target);
@@ -73,25 +74,6 @@ class Todos {
     });
   }
 
-  allDoneClicked(e) {
-    e.preventDefault();
-    const $target = $(e.currentTarget);
-    $target.disable();
-    $.ajax({
-      type: 'POST',
-      url: $target.attr('href'),
-      dataType: 'json',
-      data: {
-        '_method': 'delete',
-      },
-      success: (data) => {
-        $target.remove();
-        $('.js-todos-all').html('<div class="nothing-here-block">You\'re all done!</div>');
-        this.updateBadges(data);
-      },
-    });
-  }
-
   updateRowState(target) {
     const row = target.closest('li');
     const restoreBtn = row.querySelector('.js-undo-todo');
@@ -112,6 +94,41 @@ class Todos {
     }
   }
 
+  updateAllStateClicked(e) {
+    e.preventDefault();
+
+    const target = e.currentTarget;
+    const requestData = { '_method': target.dataset.method, ids: this.todo_ids };
+    target.setAttribute('disabled', true);
+    target.classList.add('disabled');
+    $.ajax({
+      type: 'POST',
+      url: target.dataset.href,
+      dataType: 'json',
+      data: requestData,
+      success: (data) => {
+        this.updateAllState(target, data);
+        return this.updateBadges(data);
+      },
+    });
+  }
+
+  updateAllState(target, data) {
+    const markAllDoneBtn = document.querySelector('.js-todos-mark-all');
+    const undoAllBtn = document.querySelector('.js-todos-undo-all');
+    const todoListContainer = document.querySelector('.js-todos-list-container');
+    const nothingHereContainer = document.querySelector('.js-nothing-here-container');
+
+    target.removeAttribute('disabled');
+    target.classList.remove('disabled');
+
+    this.todo_ids = (target === markAllDoneBtn) ? data.updated_ids : [];
+    undoAllBtn.classList.toggle('hidden');
+    markAllDoneBtn.classList.toggle('hidden');
+    todoListContainer.classList.toggle('hidden');
+    nothingHereContainer.classList.toggle('hidden');
+  }
+
   updateBadges(data) {
     $(document).trigger('todo:toggle', data.count);
     document.querySelector('.todos-pending .badge').innerHTML = data.count;
diff --git a/app/controllers/dashboard/todos_controller.rb b/app/controllers/dashboard/todos_controller.rb
index 5848ca62777..498690e8f11 100644
--- a/app/controllers/dashboard/todos_controller.rb
+++ b/app/controllers/dashboard/todos_controller.rb
@@ -22,12 +22,12 @@ class Dashboard::TodosController < Dashboard::ApplicationController
   end
 
   def destroy_all
-    TodoService.new.mark_todos_as_done(@todos, current_user)
+    updated_ids = TodoService.new.mark_todos_as_done(@todos, current_user)
 
     respond_to do |format|
       format.html { redirect_to dashboard_todos_path, notice: 'All todos were marked as done.' }
       format.js { head :ok }
-      format.json { render json: todos_counts }
+      format.json { render json: todos_counts.merge(updated_ids: updated_ids) }
     end
   end
 
@@ -37,6 +37,12 @@ class Dashboard::TodosController < Dashboard::ApplicationController
     render json: todos_counts
   end
 
+  def bulk_restore
+    TodoService.new.mark_todos_as_pending_by_ids(params[:ids], current_user)
+
+    render json: todos_counts
+  end
+
   # Used in TodosHelper also
   def self.todos_count_format(count)
     count >= 100 ? '99+' : count
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb
index 8787a1c93a9..bf7e76ec59e 100644
--- a/app/services/todo_service.rb
+++ b/app/services/todo_service.rb
@@ -201,10 +201,12 @@ class TodoService
   def update_todos_state_by_ids(ids, current_user, state)
     todos = current_user.todos.where(id: ids)
 
-    # Only return those that are not really on that state
-    marked_todos = todos.where.not(state: state).update_all(state: state)
+    # Only update those that are not really on that state
+    todos = todos.where.not(state: state)
+    todos_ids = todos.pluck(:id)
+    todos.update_all(state: state)
     current_user.update_todos_count_cache
-    marked_todos
+    todos_ids
   end
 
   def create_todos(users, attributes)
diff --git a/app/views/dashboard/todos/_todo.html.haml b/app/views/dashboard/todos/_todo.html.haml
index 388190642aa..d0c12aa57ae 100644
--- a/app/views/dashboard/todos/_todo.html.haml
+++ b/app/views/dashboard/todos/_todo.html.haml
@@ -36,14 +36,14 @@
 
   - if todo.pending?
     .todo-actions
-      = link_to [:dashboard, todo], method: :delete, class: 'btn btn-loading js-done-todo' do
+      = link_to dashboard_todo_path(todo), method: :delete, class: 'btn btn-loading js-done-todo', data: { href: dashboard_todo_path(todo) } do
         Done
         = icon('spinner spin')
-      = link_to restore_dashboard_todo_path(todo), method: :patch, class: 'btn btn-loading js-undo-todo hidden'  do
+      = link_to restore_dashboard_todo_path(todo), method: :patch, class: 'btn btn-loading js-undo-todo hidden', data: { href: restore_dashboard_todo_path(todo) } do
         Undo
         = icon('spinner spin')
   - else
     .todo-actions
-      = link_to restore_dashboard_todo_path(todo), method: :patch, class: 'btn btn-loading js-add-todo' do
+      = link_to restore_dashboard_todo_path(todo), method: :patch, class: 'btn btn-loading js-add-todo', data: { href: restore_dashboard_todo_path(todo) } do
         Add todo
         = icon('spinner spin')
diff --git a/app/views/dashboard/todos/index.html.haml b/app/views/dashboard/todos/index.html.haml
index 3ed67d9258c..d31ced004a0 100644
--- a/app/views/dashboard/todos/index.html.haml
+++ b/app/views/dashboard/todos/index.html.haml
@@ -19,9 +19,12 @@
 
     .nav-controls
       - if @todos.any?(&:pending?)
-        = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'btn btn-loading js-todos-mark-all', method: :delete do
+        = link_to destroy_all_dashboard_todos_path(todos_filter_params), class: 'btn btn-loading js-todos-mark-all', method: :delete, data: { href: destroy_all_dashboard_todos_path(todos_filter_params) } do
           Mark all as done
           = icon('spinner spin')
+        = link_to bulk_restore_dashboard_todos_path, class: 'btn btn-loading js-todos-undo-all hidden', method: :patch , data: { href: bulk_restore_dashboard_todos_path(todos_filter_params) } do
+          Undo mark all as done
+          = icon('spinner spin')
 
   .todos-filters
     .row-content-block.second-block
@@ -67,12 +70,16 @@
 
 .js-todos-all
   - if @todos.any?
-    .js-todos-options{ data: {per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages} }
-      .panel.panel-default.panel-small.panel-without-border
-        %ul.content-list.todos-list
-          = render @todos
-    = paginate @todos, theme: "gitlab"
-
+    .js-todos-list-container
+      .js-todos-options{ data: { per_page: @todos.limit_value, current_page: @todos.current_page, total_pages: @todos.total_pages } }
+        .panel.panel-default.panel-small.panel-without-border
+          %ul.content-list.todos-list
+            = render @todos
+      = paginate @todos, theme: "gitlab"
+    .js-nothing-here-container.todos-all-done.hidden
+      = render "shared/empty_states/icons/todos_all_done.svg"
+      %h4.text-center
+        You're all done!
   - elsif current_user.todos.any?
     .todos-all-done
       = render "shared/empty_states/icons/todos_all_done.svg"
diff --git a/changelogs/unreleased/27114-add-undo-mark-all-as-done-to-todos.yml b/changelogs/unreleased/27114-add-undo-mark-all-as-done-to-todos.yml
new file mode 100644
index 00000000000..44aae486574
--- /dev/null
+++ b/changelogs/unreleased/27114-add-undo-mark-all-as-done-to-todos.yml
@@ -0,0 +1,4 @@
+---
+title: Add Undo mark all as done to Todos
+merge_request: 9890
+author: Jacopo Beschi @jacopo-beschi
diff --git a/config/routes/dashboard.rb b/config/routes/dashboard.rb
index adc3ad207cc..8e380a0b0ac 100644
--- a/config/routes/dashboard.rb
+++ b/config/routes/dashboard.rb
@@ -13,6 +13,7 @@ resource :dashboard, controller: 'dashboard', only: [] do
     resources :todos, only: [:index, :destroy] do
       collection do
         delete :destroy_all
+        patch :bulk_restore
       end
       member do
         patch :restore
diff --git a/features/steps/dashboard/todos.rb b/features/steps/dashboard/todos.rb
index eb906a55a83..9f01dff776f 100644
--- a/features/steps/dashboard/todos.rb
+++ b/features/steps/dashboard/todos.rb
@@ -159,7 +159,11 @@ class Spinach::Features::DashboardTodos < Spinach::FeatureSteps
   end
 
   def should_not_see_todo(title)
-    expect(page).not_to have_content title
+    expect(page).not_to have_visible_content title
+  end
+
+  def have_visible_content(text)
+    have_css('*', text: text, visible: true)
   end
 
   def john_doe
diff --git a/lib/api/v3/todos.rb b/lib/api/v3/todos.rb
index e60cb25e57b..e3b311d61cd 100644
--- a/lib/api/v3/todos.rb
+++ b/lib/api/v3/todos.rb
@@ -20,9 +20,9 @@ module API
         desc 'Mark all todos as done'
         delete do
           status(200)
-          
+
           todos = TodosFinder.new(current_user, params).execute
-          TodoService.new.mark_todos_as_done(todos, current_user)
+          TodoService.new.mark_todos_as_done(todos, current_user).size
         end
       end
     end
diff --git a/spec/controllers/dashboard/todos_controller_spec.rb b/spec/controllers/dashboard/todos_controller_spec.rb
index 7072bd5e87c..71a4a2c43c7 100644
--- a/spec/controllers/dashboard/todos_controller_spec.rb
+++ b/spec/controllers/dashboard/todos_controller_spec.rb
@@ -49,4 +49,18 @@ describe Dashboard::TodosController do
       expect(json_response).to eq({ "count" => "1", "done_count" => "0" })
     end
   end
+
+  describe 'PATCH #bulk_restore' do
+    let(:todos) { create_list(:todo, 2, :done, user: user, project: project, author: author) }
+
+    it 'restores the todos to pending state' do
+      patch :bulk_restore, ids: todos.map(&:id)
+
+      todos.each do |todo|
+        expect(todo.reload).to be_pending
+      end
+      expect(response).to have_http_status(200)
+      expect(json_response).to eq({ 'count' => '2', 'done_count' => '0' })
+    end
+  end
 end
diff --git a/spec/features/todos/todos_spec.rb b/spec/features/todos/todos_spec.rb
index 5c2df949ac5..850020109d4 100644
--- a/spec/features/todos/todos_spec.rb
+++ b/spec/features/todos/todos_spec.rb
@@ -31,7 +31,7 @@ describe 'Dashboard Todos', feature: true do
       end
 
       it 'shows due date as today' do
-        page.within first('.todo') do
+        within first('.todo') do
           expect(page).to have_content 'Due today'
         end
       end
@@ -184,6 +184,60 @@ describe 'Dashboard Todos', feature: true do
           expect(page).to have_content "You're all done!"
           expect(page).not_to have_selector('.gl-pagination')
         end
+
+        it 'shows "Undo mark all as done" button' do
+          expect(page).to have_selector('.js-todos-mark-all', visible: false)
+          expect(page).to have_selector('.js-todos-undo-all', visible: true)
+        end
+      end
+
+      describe 'undo mark all as done', js: true do
+        before do
+          visit dashboard_todos_path
+        end
+
+        it 'shows the restored todo list' do
+          mark_all_and_undo
+
+          expect(page).to have_selector('.todos-list .todo', count: 1)
+          expect(page).to have_selector('.gl-pagination')
+          expect(page).not_to have_content "You're all done!"
+        end
+
+        it 'updates todo count' do
+          mark_all_and_undo
+
+          expect(page).to have_content 'To do 2'
+          expect(page).to have_content 'Done 0'
+        end
+
+        it 'shows "Mark all as done" button' do
+          mark_all_and_undo
+
+          expect(page).to have_selector('.js-todos-mark-all', visible: true)
+          expect(page).to have_selector('.js-todos-undo-all', visible: false)
+        end
+
+        context 'User has deleted a todo' do
+          before do
+            within first('.todo') do
+              click_link 'Done'
+            end
+          end
+
+          it 'shows the restored todo list with the deleted todo' do
+            mark_all_and_undo
+
+            expect(page).to have_selector('.todos-list .todo.todo-pending', count: 1)
+          end
+        end
+
+        def mark_all_and_undo
+          click_link 'Mark all as done'
+          wait_for_ajax
+          click_link 'Undo mark all as done'
+          wait_for_ajax
+        end
       end
     end
 
diff --git a/spec/services/todo_service_spec.rb b/spec/services/todo_service_spec.rb
index a8395cb48ea..3645b73b039 100644
--- a/spec/services/todo_service_spec.rb
+++ b/spec/services/todo_service_spec.rb
@@ -298,6 +298,10 @@ describe TodoService, services: true do
         expect(second_todo.reload.state?(new_state)).to be true
       end
 
+      it 'returns the updated ids' do
+        expect(service.send(meth, collection, john_doe)).to match_array([first_todo.id, second_todo.id])
+      end
+
       describe 'cached counts' do
         it 'updates when todos change' do
           expect(john_doe.todos.where(state: new_state).count).to eq(0)
@@ -706,7 +710,7 @@ describe TodoService, services: true do
         should_create_todo(user: admin, author: admin, target: mr_unassigned, action: Todo::UNMERGEABLE)
       end
     end
-    
+
     describe '#mark_todo' do
       it 'creates a todo from a merge request' do
         service.mark_todo(mr_unassigned, author)
@@ -779,29 +783,27 @@ describe TodoService, services: true do
         .to change { todo.reload.state }.from('pending').to('done')
     end
 
-    it 'returns the number of updated todos' do # Needed on API
+    it 'returns the ids of updated todos' do # Needed on API
       todo = create(:todo, :mentioned, user: john_doe, target: issue, project: project)
 
-      expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq(1)
+      expect(TodoService.new.mark_todos_as_done([todo], john_doe)).to eq([todo.id])
     end
 
     context 'when some of the todos are done already' do
-      before do
-        create(:todo, :mentioned, user: john_doe, target: issue, project: project)
-        create(:todo, :mentioned, user: john_doe, target: another_issue, project: project)
-      end
+      let!(:first_todo) { create(:todo, :mentioned, user: john_doe, target: issue, project: project) }
+      let!(:second_todo) { create(:todo, :mentioned, user: john_doe, target: another_issue, project: project) }
 
-      it 'returns the number of those still pending' do
+      it 'returns the ids of those still pending' do
         TodoService.new.mark_pending_todos_as_done(issue, john_doe)
 
-        expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(1)
+        expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq([second_todo.id])
       end
 
-      it 'returns 0 if all are done' do
+      it 'returns an empty array if all are done' do
         TodoService.new.mark_pending_todos_as_done(issue, john_doe)
         TodoService.new.mark_pending_todos_as_done(another_issue, john_doe)
 
-        expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq(0)
+        expect(TodoService.new.mark_todos_as_done(Todo.all, john_doe)).to eq([])
       end
     end