diff options
author | Toon Claes <toon@gitlab.com> | 2017-04-21 12:36:34 +0300 |
---|---|---|
committer | Toon Claes <toon@iotcl.com> | 2017-08-03 17:31:05 +0300 |
commit | a723cba57493ec3220596ca8543a8b1b1ec118fe (patch) | |
tree | fc1048fe60a21339cef711caf69aff59edf5a5fb /app/services/todo_service.rb | |
parent | c39daf937bd62eedcbe7e3b44f107bb7e87452e7 (diff) |
Avoid plucking Todo ids and use sub-queries instead
TodoService should not call `.select(&:id)` on todos, because this is
bad performance. So instead use sub-queries, which will result in a
single SQL query to the database.
https://docs.gitlab.com/ee/development/sql.html#plucking-ids
Diffstat (limited to 'app/services/todo_service.rb')
-rw-r--r-- | app/services/todo_service.rb | 16 |
1 files changed, 10 insertions, 6 deletions
diff --git a/app/services/todo_service.rb b/app/services/todo_service.rb index 322c6286365..6ee96d6a0f8 100644 --- a/app/services/todo_service.rb +++ b/app/services/todo_service.rb @@ -170,20 +170,22 @@ class TodoService # When user marks some todos as done def mark_todos_as_done(todos, current_user) - update_todos_state_by_ids(todos.select(&:id), current_user, :done) + update_todos_state(todos, current_user, :done) end def mark_todos_as_done_by_ids(ids, current_user) - update_todos_state_by_ids(ids, current_user, :done) + todos = todos_by_ids(ids, current_user) + mark_todos_as_done(todos, current_user) end # When user marks some todos as pending def mark_todos_as_pending(todos, current_user) - update_todos_state_by_ids(todos.select(&:id), current_user, :pending) + update_todos_state(todos, current_user, :pending) end def mark_todos_as_pending_by_ids(ids, current_user) - update_todos_state_by_ids(ids, current_user, :pending) + todos = todos_by_ids(ids, current_user) + mark_todos_as_pending(todos, current_user) end # When user marks an issue as todo @@ -198,9 +200,11 @@ class TodoService private - def update_todos_state_by_ids(ids, current_user, state) - todos = current_user.todos.where(id: ids) + def todos_by_ids(ids, current_user) + current_user.todos.where(id: Array(ids)) + end + def update_todos_state(todos, current_user, state) # Only update those that are not really on that state todos = todos.where.not(state: state) todos_ids = todos.pluck(:id) |