From b0b6bddfc9008a96c3b0101fd9c5e90ff70a1058 Mon Sep 17 00:00:00 2001 From: esteres Date: Mon, 20 Jan 2025 23:45:12 -0500 Subject: [PATCH 1/2] Fix N+1 query issue by eager loading Companies with People --- app/controllers/people_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 6559b9e..4ba226f 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -1,7 +1,7 @@ class PeopleController < ApplicationController def index - @people = Person.all + @people = Person.includes(:company) end def new From 30545aca593fd5f93fa806a8aa3b5977cef79c2f Mon Sep 17 00:00:00 2001 From: esteres Date: Tue, 21 Jan 2025 00:01:06 -0500 Subject: [PATCH 2/2] Add pagination for People index using Kaminari --- Gemfile | 1 + Gemfile.lock | 13 ++++ app/controllers/people_controller.rb | 2 +- app/views/kaminari/_first_page.html.slim | 2 + app/views/kaminari/_gap.html.slim | 2 + app/views/kaminari/_last_page.html.slim | 2 + app/views/kaminari/_next_page.html.slim | 2 + app/views/kaminari/_page.html.slim | 6 ++ app/views/kaminari/_paginator.html.slim | 12 +++ app/views/kaminari/_prev_page.html.slim | 2 + app/views/people/index.html.slim | 11 ++- config/initializers/kaminari_config.rb | 3 + spec/views/people/index.html.slim_spec.rb | 89 ++++++++++++++++++++++- 13 files changed, 138 insertions(+), 9 deletions(-) create mode 100644 app/views/kaminari/_first_page.html.slim create mode 100644 app/views/kaminari/_gap.html.slim create mode 100644 app/views/kaminari/_last_page.html.slim create mode 100644 app/views/kaminari/_next_page.html.slim create mode 100644 app/views/kaminari/_page.html.slim create mode 100644 app/views/kaminari/_paginator.html.slim create mode 100644 app/views/kaminari/_prev_page.html.slim create mode 100644 config/initializers/kaminari_config.rb diff --git a/Gemfile b/Gemfile index 716a9ba..9d6dda9 100644 --- a/Gemfile +++ b/Gemfile @@ -14,6 +14,7 @@ gem "jbuilder" gem "tzinfo-data", platforms: %i[ mingw mswin x64_mingw jruby ] gem 'slim-rails' gem "jsbundling-rails" +gem "kaminari" group :development, :test do # See https://guides.rubyonrails.org/debugging_rails_applications.html#debugging-with-the-debug-gem diff --git a/Gemfile.lock b/Gemfile.lock index 8aaae9a..0c7279c 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -115,6 +115,18 @@ GEM activesupport (>= 5.0.0) jsbundling-rails (1.3.1) railties (>= 6.0.0) + kaminari (1.2.2) + activesupport (>= 4.1.0) + kaminari-actionview (= 1.2.2) + kaminari-activerecord (= 1.2.2) + kaminari-core (= 1.2.2) + kaminari-actionview (1.2.2) + actionview + kaminari-core (= 1.2.2) + kaminari-activerecord (1.2.2) + activerecord + kaminari-core (= 1.2.2) + kaminari-core (1.2.2) logger (1.6.1) loofah (2.23.1) crass (~> 1.0.2) @@ -290,6 +302,7 @@ DEPENDENCIES faker jbuilder jsbundling-rails + kaminari pry-rails puma (~> 5.0) rails (~> 7.0.1) diff --git a/app/controllers/people_controller.rb b/app/controllers/people_controller.rb index 4ba226f..5915cbe 100644 --- a/app/controllers/people_controller.rb +++ b/app/controllers/people_controller.rb @@ -1,7 +1,7 @@ class PeopleController < ApplicationController def index - @people = Person.includes(:company) + @people = Person.includes(:company).page(params[:page]) end def new diff --git a/app/views/kaminari/_first_page.html.slim b/app/views/kaminari/_first_page.html.slim new file mode 100644 index 0000000..cc5a1cc --- /dev/null +++ b/app/views/kaminari/_first_page.html.slim @@ -0,0 +1,2 @@ +li.page-item + = link_to_unless current_page.first?, raw(t 'views.pagination.first'), url, remote: remote, class: 'page-link' diff --git a/app/views/kaminari/_gap.html.slim b/app/views/kaminari/_gap.html.slim new file mode 100644 index 0000000..ecaec7e --- /dev/null +++ b/app/views/kaminari/_gap.html.slim @@ -0,0 +1,2 @@ +li.page-item.disabled + = link_to raw(t 'views.pagination.truncate'), '#', class: 'page-link' diff --git a/app/views/kaminari/_last_page.html.slim b/app/views/kaminari/_last_page.html.slim new file mode 100644 index 0000000..710f1eb --- /dev/null +++ b/app/views/kaminari/_last_page.html.slim @@ -0,0 +1,2 @@ +li.page-item + = link_to_unless current_page.last?, raw(t 'views.pagination.last'), url, remote: remote, class: 'page-link' diff --git a/app/views/kaminari/_next_page.html.slim b/app/views/kaminari/_next_page.html.slim new file mode 100644 index 0000000..5adc455 --- /dev/null +++ b/app/views/kaminari/_next_page.html.slim @@ -0,0 +1,2 @@ +li.page-item + = link_to_unless current_page.last?, raw(t 'views.pagination.next'), url, rel: 'next', remote: remote, class: 'page-link' diff --git a/app/views/kaminari/_page.html.slim b/app/views/kaminari/_page.html.slim new file mode 100644 index 0000000..80b5e2f --- /dev/null +++ b/app/views/kaminari/_page.html.slim @@ -0,0 +1,6 @@ +- if page.current? + li.page-item.active + = content_tag :a, page, data: { remote: remote }, rel: page.rel, class: 'page-link' +- else + li.page-item + = link_to page, url, remote: remote, rel: page.rel, class: 'page-link' diff --git a/app/views/kaminari/_paginator.html.slim b/app/views/kaminari/_paginator.html.slim new file mode 100644 index 0000000..5dcf566 --- /dev/null +++ b/app/views/kaminari/_paginator.html.slim @@ -0,0 +1,12 @@ += paginator.render do + nav + ul.pagination + == first_page_tag unless current_page.first? + == prev_page_tag unless current_page.first? + - each_page do |page| + - if page.left_outer? || page.right_outer? || page.inside_window? + == page_tag page + - elsif !page.was_truncated? + == gap_tag + == next_page_tag unless current_page.last? + == last_page_tag unless current_page.last? diff --git a/app/views/kaminari/_prev_page.html.slim b/app/views/kaminari/_prev_page.html.slim new file mode 100644 index 0000000..83f33ee --- /dev/null +++ b/app/views/kaminari/_prev_page.html.slim @@ -0,0 +1,2 @@ +li.page-item + = link_to_unless current_page.first?, raw(t 'views.pagination.previous'), url, rel: 'prev', remote: remote, class: 'page-link' diff --git a/app/views/people/index.html.slim b/app/views/people/index.html.slim index ddbf52f..3b4fc1d 100644 --- a/app/views/people/index.html.slim +++ b/app/views/people/index.html.slim @@ -12,10 +12,9 @@ table.table - @people.each do |person| tr th[scope="row"]= person&.id - td= person.try(:name) - td= person.try(:phone) - td= person.try(:email) - td= person.try(:company).try(:name) - - + td= person&.name + td= person&.phone_number + td= person&.email + td= person&.company&.name += paginate @people diff --git a/config/initializers/kaminari_config.rb b/config/initializers/kaminari_config.rb new file mode 100644 index 0000000..10ab8ae --- /dev/null +++ b/config/initializers/kaminari_config.rb @@ -0,0 +1,3 @@ +Kaminari.configure do |config| + config.default_per_page = 10 +end diff --git a/spec/views/people/index.html.slim_spec.rb b/spec/views/people/index.html.slim_spec.rb index 7e2d362..97c0215 100644 --- a/spec/views/people/index.html.slim_spec.rb +++ b/spec/views/people/index.html.slim_spec.rb @@ -1,5 +1,90 @@ require "rails_helper" -describe "people/index.html.slim" do - it "Displays the users" +describe "people/index.html.slim", type: :view do + + def generate_people(count) + Kaminari.paginate_array( + Array.new(count) do |i| + Person.create( + name: "Person #{i + 1}", + phone_number: "1234567890", + email: "person#{i + 1}@example.com", + company: Company.create(name: "Test Company") + ) + end + ) + end + + shared_examples "renders people table" do |pagination_controls| + it "displays the heading 'Viewing people'" do + expect(rendered).to have_selector("h2", text: "Viewing people") + end + + it "renders the table headers correctly" do + expect(rendered).to have_selector("table.table thead tr") do + expect(rendered).to have_selector("th", text: "ID") + expect(rendered).to have_selector("th", text: "Name") + expect(rendered).to have_selector("th", text: "Phone number") + expect(rendered).to have_selector("th", text: "Email address") + expect(rendered).to have_selector("th", text: "Company") + end + end + + it "renders pagination controls as expected" do + if pagination_controls + expect(rendered).to have_selector("nav>ul.pagination") + else + expect(rendered).not_to have_selector("nav>ul.pagination") + end + end + end + + context "when there are more than 10 records" do + let(:people) { generate_people(15) } + before do + assign(:people, people.page(1).per(10)) + render + end + + include_examples "renders people table", true + + + it "renders each person's details in the table" do + people.each_with_index do |person, index| + row_selector = "table.table tbody tr:nth-child(#{index + 1})" + + expect(rendered).to have_selector(row_selector) do |row| + expect(row).to have_selector("th", text: person.id.to_s) + expect(row).to have_selector("td", text: person.name) + expect(row).to have_selector("td", text: person.phone_number) + expect(row).to have_selector("td", text: person.email) + expect(row).to have_selector("td", text: person.company.name) + end + end + end + end + + context "when there are less than 10 records" do + let(:people) { generate_people(5) } + before do + assign(:people, people.page(1).per(10)) + render + end + + include_examples "renders people table" + + it "renders each person's details in the table" do + people.each_with_index do |person, index| + row_selector = "table.table tbody tr:nth-child(#{index + 1})" + + expect(rendered).to have_selector(row_selector) do |row| + expect(row).to have_selector("th", text: person.id.to_s) + expect(row).to have_selector("td", text: person.name) + expect(row).to have_selector("td", text: person.phone_number) + expect(row).to have_selector("td", text: person.email) + expect(row).to have_selector("td", text: person.company.name) + end + end + end + end end