From 426f1c20b19e5bd98ee0ab8bd5f8851b683a79c6 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Fri, 14 Jun 2019 10:20:03 +1200 Subject: Remove N+1 query for project and group boards - Add test for N+1 queries - Add destroyable lists scope to Board and List - Preload lists for both project and group boards --- app/models/board.rb | 3 +++ app/serializers/board_serializer.rb | 8 ++++++++ lib/api/boards.rb | 2 +- lib/api/boards_responses.rb | 2 +- lib/api/entities.rb | 2 +- lib/api/group_boards.rb | 2 +- spec/support/api/boards_shared_examples.rb | 10 ++++++++++ 7 files changed, 25 insertions(+), 4 deletions(-) diff --git a/app/models/board.rb b/app/models/board.rb index e08db764f65..891d26897ed 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -5,10 +5,13 @@ class Board < ApplicationRecord belongs_to :project has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :destroyable_lists, -> { destroyable }, class_name: "List" validates :project, presence: true, if: :project_needed? validates :group, presence: true, unless: :project + scope :with_associations, -> { preload(:destroyable_lists) } + def project_needed? !group end diff --git a/app/serializers/board_serializer.rb b/app/serializers/board_serializer.rb index 70a4c9ae282..3877a1bdcbc 100644 --- a/app/serializers/board_serializer.rb +++ b/app/serializers/board_serializer.rb @@ -2,4 +2,12 @@ class BoardSerializer < BaseSerializer entity BoardSimpleEntity + + def represent(resource, opts = {}) + if resource.respond_to?(:with_associations) + resource = resource.with_associations + end + + super + end end diff --git a/lib/api/boards.rb b/lib/api/boards.rb index b7c77730afb..4e31f74f18a 100644 --- a/lib/api/boards.rb +++ b/lib/api/boards.rb @@ -27,7 +27,7 @@ module API end get '/' do authorize!(:read_board, user_project) - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end desc 'Find a project board' do diff --git a/lib/api/boards_responses.rb b/lib/api/boards_responses.rb index 86d9b24802f..68497a08fb8 100644 --- a/lib/api/boards_responses.rb +++ b/lib/api/boards_responses.rb @@ -11,7 +11,7 @@ module API end def board_lists - board.lists.destroyable + board.destroyable_lists end def create_list diff --git a/lib/api/entities.rb b/lib/api/entities.rb index ead01dc53f7..d783591c238 100644 --- a/lib/api/entities.rb +++ b/lib/api/entities.rb @@ -1101,7 +1101,7 @@ module API expose :project, using: Entities::BasicProjectDetails expose :lists, using: Entities::List do |board| - board.lists.destroyable + board.destroyable_lists end end diff --git a/lib/api/group_boards.rb b/lib/api/group_boards.rb index 9a20ee8c8b9..feb2254963e 100644 --- a/lib/api/group_boards.rb +++ b/lib/api/group_boards.rb @@ -37,7 +37,7 @@ module API use :pagination end get '/' do - present paginate(board_parent.boards), with: Entities::Board + present paginate(board_parent.boards.with_associations), with: Entities::Board end end diff --git a/spec/support/api/boards_shared_examples.rb b/spec/support/api/boards_shared_examples.rb index 592962ebf7c..3abb5096a7a 100644 --- a/spec/support/api/boards_shared_examples.rb +++ b/spec/support/api/boards_shared_examples.rb @@ -14,6 +14,16 @@ shared_examples_for 'group and project boards' do |route_definition, ee = false| end end + it 'avoids N+1 queries' do + pat = create(:personal_access_token, user: user) + control = ActiveRecord::QueryRecorder.new { get api(root_url, personal_access_token: pat) } + + create(:milestone, "#{board_parent.class.name.underscore}": board_parent) + create(:board, "#{board_parent.class.name.underscore}": board_parent) + + expect { get api(root_url, personal_access_token: pat) }.not_to exceed_query_limit(control) + end + describe "GET #{route_definition}" do context "when unauthenticated" do it "returns authentication error" do -- cgit v1.2.1 From 5aaef1334de1a765ed29e1065bb52d2bfa4f8944 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Mon, 24 Jun 2019 16:40:55 +0200 Subject: Remove `with_associations` from BoardSerializer --- app/serializers/board_serializer.rb | 8 -------- 1 file changed, 8 deletions(-) diff --git a/app/serializers/board_serializer.rb b/app/serializers/board_serializer.rb index 3877a1bdcbc..70a4c9ae282 100644 --- a/app/serializers/board_serializer.rb +++ b/app/serializers/board_serializer.rb @@ -2,12 +2,4 @@ class BoardSerializer < BaseSerializer entity BoardSimpleEntity - - def represent(resource, opts = {}) - if resource.respond_to?(:with_associations) - resource = resource.with_associations - end - - super - end end -- cgit v1.2.1 From 82b87ae509e744dab135573da1f9b8ada4f81ed9 Mon Sep 17 00:00:00 2001 From: charlieablett Date: Wed, 26 Jun 2019 21:49:53 +1200 Subject: Compound lists scopes to preserve ordering --- app/models/board.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/board.rb b/app/models/board.rb index 891d26897ed..cde5ee7c5b5 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -5,7 +5,7 @@ class Board < ApplicationRecord belongs_to :project has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent - has_many :destroyable_lists, -> { destroyable }, class_name: "List" + has_many :destroyable_lists, -> { lists.destroyable }, class_name: "List" validates :project, presence: true, if: :project_needed? validates :group, presence: true, unless: :project -- cgit v1.2.1 From 04d2d8f9b7e8593cb0ea3d8db7b57b843387fa2b Mon Sep 17 00:00:00 2001 From: charlieablett Date: Thu, 27 Jun 2019 10:32:51 +1200 Subject: Move order lambda to list scope - apply ordering to both list scopes in `Board` --- app/models/board.rb | 4 ++-- app/models/list.rb | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/app/models/board.rb b/app/models/board.rb index cde5ee7c5b5..50b6ca9b70f 100644 --- a/app/models/board.rb +++ b/app/models/board.rb @@ -4,8 +4,8 @@ class Board < ApplicationRecord belongs_to :group belongs_to :project - has_many :lists, -> { order(:list_type, :position) }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent - has_many :destroyable_lists, -> { lists.destroyable }, class_name: "List" + has_many :lists, -> { ordered }, dependent: :delete_all # rubocop:disable Cop/ActiveRecordDependent + has_many :destroyable_lists, -> { destroyable.ordered }, class_name: "List" validates :project, presence: true, if: :project_needed? validates :group, presence: true, unless: :project diff --git a/app/models/list.rb b/app/models/list.rb index 17b1a8510cf..d28a9bda82d 100644 --- a/app/models/list.rb +++ b/app/models/list.rb @@ -16,6 +16,7 @@ class List < ApplicationRecord scope :destroyable, -> { where(list_type: list_types.slice(*destroyable_types).values) } scope :movable, -> { where(list_type: list_types.slice(*movable_types).values) } scope :preload_associations, -> { preload(:board, :label) } + scope :ordered, -> { order(:list_type, :position) } class << self def destroyable_types -- cgit v1.2.1