summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorJacopo <beschi.jacopo@gmail.com>2017-01-29 10:44:30 +0100
committerJacopo <beschi.jacopo@gmail.com>2017-03-16 07:15:00 +0100
commit5f9ace8eb1d71b35c92156177465b066ebbc4a3e (patch)
tree5af8e80ecace78f4d165cb470062e6a2cd53c853
parent5e05d6b8cf26c424378ef6091a4b38c2d424808e (diff)
downloadgitlab-ce-5f9ace8eb1d71b35c92156177465b066ebbc4a3e.tar.gz
Add 'Undo mark all as done' to Todos
Added the ability to 'Undo mark all as done' todos marked as complete with 'Mark all as done' in the 'Todo' tab of the Todo dashboard. The operation undos only the todo previously marked as done with the 'Mark al as done' button.
-rw-r--r--app/assets/javascripts/todos.js67
-rw-r--r--app/controllers/dashboard/todos_controller.rb10
-rw-r--r--app/services/todo_service.rb8
-rw-r--r--app/views/dashboard/todos/_todo.html.haml6
-rw-r--r--app/views/dashboard/todos/index.html.haml21
-rw-r--r--changelogs/unreleased/27114-add-undo-mark-all-as-done-to-todos.yml4
-rw-r--r--config/routes/dashboard.rb1
-rw-r--r--features/steps/dashboard/todos.rb6
-rw-r--r--lib/api/v3/todos.rb4
-rw-r--r--spec/controllers/dashboard/todos_controller_spec.rb14
-rw-r--r--spec/features/todos/todos_spec.rb56
-rw-r--r--spec/services/todo_service_spec.rb24
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 d7e0a8e4b2c..0923f5fb6ab 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