diff --git a/app/models/leader_board.rb b/app/models/leader_board.rb index a86b249b0..3efc7da9d 100644 --- a/app/models/leader_board.rb +++ b/app/models/leader_board.rb @@ -13,7 +13,7 @@ class LeaderBoard WORKERS = TYPES.map { |t| "refresh_leader_board/#{t}".camelize.constantize } def self.refresh!(type) - self.send(type, force: true) + send(type, force: true) end def self.users_by_description_edits(force: false) @@ -27,15 +27,13 @@ def self.top_users_by_description_edits def self.pens_by_popularity(force: false) Rails .cache - .fetch("LeaderBoard#pens_by_popularity", force: force) do - CollectedPen - .group("lower(concat(brand, model))") - .where(archived_on: nil) - .select( - "initcap(min(brand)) as brand, initcap(min(model)) as model, count(id) as count" - ) - .having("count(distinct user_id) > 1") - .order("count desc") + .fetch("LeaderBoard#pens_by_popularity", force:) do + Pens::Model + .includes(:collected_pens) + .reject { |model| model.collected_pens.size.zero? } + .map do |model| + { name: model.name, count: model.collected_pens.size } + end end end @@ -46,7 +44,7 @@ def self.top_pens_by_popularity def self.ink_review_submissions(force: false) Rails .cache - .fetch("LeaderBoard#ink_review_submissions", force: force) do + .fetch("LeaderBoard#ink_review_submissions", force:) do relation = User .where("users.id <> 1") @@ -65,7 +63,7 @@ def self.top_ink_review_submissions def self.usage_records(force: false) Rails .cache - .fetch("LeaderBoard#usage_records", force: force) do + .fetch("LeaderBoard#usage_records", force:) do relation = User .joins(currently_inkeds: :usage_records) @@ -83,7 +81,7 @@ def self.top_usage_records def self.currently_inked(force: false) Rails .cache - .fetch("LeaderBoard#currently_inked", force: force) do + .fetch("LeaderBoard#currently_inked", force:) do relation = User .joins(:currently_inkeds) @@ -101,7 +99,7 @@ def self.top_currently_inked def self.inks_by_popularity(force: false) Rails .cache - .fetch("LeaderBoard#inks_by_popularity", force: force) do + .fetch("LeaderBoard#inks_by_popularity", force:) do MacroCluster .joins(micro_clusters: :collected_inks) .includes(:brand_cluster) @@ -117,7 +115,7 @@ def self.top_inks_by_popularity end def self.inks(force: false) - Rails.cache.fetch("LeaderBoard#inks", force: force) { extract(build) } + Rails.cache.fetch("LeaderBoard#inks", force:) { extract(build) } end def self.top_inks @@ -127,7 +125,7 @@ def self.top_inks def self.bottles(force: false) Rails .cache - .fetch("LeaderBoard#bottles", force: force) do + .fetch("LeaderBoard#bottles", force:) do extract(build.where(collected_inks: { kind: "bottle" })) end end @@ -139,7 +137,7 @@ def self.top_bottles def self.samples(force: false) Rails .cache - .fetch("LeaderBoard#samples", force: force) do + .fetch("LeaderBoard#samples", force:) do extract(build.where(collected_inks: { kind: "sample" })) end end diff --git a/app/models/leader_board_row/pen_models.rb b/app/models/leader_board_row/pen_models.rb new file mode 100644 index 000000000..53a557c70 --- /dev/null +++ b/app/models/leader_board_row/pen_models.rb @@ -0,0 +1,2 @@ +class LeaderBoardRow::PenModels < LeaderBoardRow +end diff --git a/app/models/pens/model.rb b/app/models/pens/model.rb index dbd569113..fbfc7754f 100644 --- a/app/models/pens/model.rb +++ b/app/models/pens/model.rb @@ -4,10 +4,12 @@ class Pens::Model < ApplicationRecord class_name: "Pens::ModelMicroCluster", dependent: :nullify has_many :model_variants, through: :model_micro_clusters + has_many :micro_clusters, through: :model_variants + has_many :collected_pens, through: :micro_clusters paginates_per 100 - scope :ordered, lambda { order(:brand, :model) } + scope :ordered, -> { order(:brand, :model) } def self.search(query) return self if query.blank? @@ -15,10 +17,14 @@ def self.search(query) query = query.split(/\s+/).join("%") joins(model_micro_clusters: :model_variants).where( <<~SQL, - CONCAT(pens_model_variants.brand, pens_model_variants.model) - ILIKE ? - SQL + CONCAT(pens_model_variants.brand, pens_model_variants.model) + ILIKE ? + SQL "%#{query}%" ).group("pens_models.id") end + + def name + "#{brand} #{model}" + end end diff --git a/app/views/pages/leaderboards.html.slim b/app/views/pages/leaderboards.html.slim index 6fdff39ab..771932fda 100644 --- a/app/views/pages/leaderboards.html.slim +++ b/app/views/pages/leaderboards.html.slim @@ -79,10 +79,10 @@ div.row.mb-3 = " (#{macro_cluster.ci_count})" p= link_to "Show all", page_path("inks_by_popularity") - div class="col-sm-12 fpc-leaderboards__board" + div class="col-sm-12 col-md-6 fpc-leaderboards__board" h2 class="h4" Popular pens p.text-muted Ranked by number of people who own this ol - LeaderBoard.top_pens_by_popularity.each do |pen| - li= "#{pen.brand} #{pen.model} (#{pen.count})" + li= "#{pen[:name]} (#{pen[:count]})" p= link_to "Show all", page_path("pens_by_popularity") diff --git a/app/views/pages/pens_by_popularity.html.slim b/app/views/pages/pens_by_popularity.html.slim index 03020eaa3..22e607e5c 100644 --- a/app/views/pages/pens_by_popularity.html.slim +++ b/app/views/pages/pens_by_popularity.html.slim @@ -9,4 +9,4 @@ ol class="fpc-leaderboard" - LeaderBoard.pens_by_popularity.each do |pen| - li="#{pen.brand} #{pen.model} (#{pen.count})" + li="#{pen[:name]} (#{pen[:count]})" diff --git a/spec/models/leader_board_spec.rb b/spec/models/leader_board_spec.rb index 2d1160a6b..fbdc393c4 100644 --- a/spec/models/leader_board_spec.rb +++ b/spec/models/leader_board_spec.rb @@ -1,61 +1,6 @@ require "rails_helper" describe LeaderBoard do - describe "#pens_by_popularity" do - it "orders the pens by popularity" do - user1 = create(:user) - create(:collected_pen, user: user1, brand: "Brand1", model: "Model1") - create(:collected_pen, user: user1, brand: "Brand1", model: "Model2") - user2 = create(:user) - create(:collected_pen, user: user2, brand: "Brand1", model: "Model1") - create(:collected_pen, user: user2, brand: "Brand1", model: "Model2") - user3 = create(:user) - create(:collected_pen, user: user3, brand: "Brand1", model: "Model1") - - expect( - described_class.pens_by_popularity.map do |pen| - [pen.brand, pen.model, pen.count] - end - ).to eq([["Brand1", "Model1", 3], ["Brand1", "Model2", 2]]) - end - - it "does not count pens that only appear once" do - user = create(:user) - create(:collected_pen, user: user, brand: "Brand1", model: "Model1") - - expect(described_class.pens_by_popularity).to be_empty - end - - it "does not count pens that only one user has (but multiple)" do - user = create(:user) - create(:collected_pen, user: user, brand: "Brand1", model: "Model1") - create(:collected_pen, user: user, brand: "Brand1", model: "Model1") - - expect(described_class.pens_by_popularity).to be_empty - end - - it "does not include archived entries" do - user1 = create(:user) - create( - :collected_pen, - user: user1, - brand: "Brand1", - model: "Model1", - archived_on: Date.today - ) - user2 = create(:user) - create( - :collected_pen, - user: user2, - brand: "Brand1", - model: "Model1", - archived_on: Date.today - ) - - expect(described_class.pens_by_popularity).to be_empty - end - end - describe "#top_pens_by_popularity" do it "returns the first 10 entries" do allow(described_class).to receive(:pens_by_popularity).and_return( @@ -162,7 +107,7 @@ it "returns the user name" do user = create(:user, name: "the name") - create(:collected_ink, user: user) + create(:collected_ink, user:) expect(described_class.inks.first).to include(public_name: "the name") end @@ -173,7 +118,7 @@ it "does not include users with only private inks" do user = create(:user) - create(:collected_ink, user: user, private: true) + create(:collected_ink, user:, private: true) expect(described_class.inks).to be_empty end @@ -191,7 +136,7 @@ it "returns the patreon status" do user = create(:user, patron: true) - create(:collected_ink, user: user) + create(:collected_ink, user:) expect(described_class.inks.first).to include(patron: true) end end