diff --git a/Gemfile.lock b/Gemfile.lock index 2e91c8770..54846ef7d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -431,7 +431,7 @@ GEM rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) unicode-display_width (>= 1.4.0, < 1.7) - rubocop-rails (2.2.0) + rubocop-rails (2.2.1) rack (>= 1.1) rubocop (>= 0.72.0) rubocop-rspec (1.33.0) diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index e9131cbf6..dfddef42c 100644 --- a/app/models/concerns/photo_capable.rb +++ b/app/models/concerns/photo_capable.rb @@ -6,5 +6,13 @@ module PhotoCapable has_many :photos, through: :photo_associations, as: :photographable scope :has_photos, -> { includes(:photos).where.not(photos: { id: nil }) } + + def default_photo + most_liked_photo + end + + def most_liked_photo + photos.order(likes_count: :desc, created_at: :desc).first + end end end diff --git a/app/models/crop.rb b/app/models/crop.rb index adcec8a5c..12eb01975 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -1,5 +1,6 @@ class Crop < ApplicationRecord extend FriendlyId + include PhotoCapable friendly_id :name, use: %i(slugged finders) ## @@ -57,18 +58,6 @@ class Crop < ApplicationRecord # Elastic search configuration searchkick word_start: %i(name alternate_names scientific_names), case_sensitive: false if ENV["GROWSTUFF_ELASTICSEARCH"] == "true" - def planting_photos - Photo.joins(:plantings).where("plantings.crop_id": id) - end - - def harvest_photos - Photo.joins(:harvests).where("harvests.crop_id": id) - end - - def seed_photos - Photo.joins(:seeds).where("seeds.crop_id": id) - end - def to_s name end @@ -78,14 +67,7 @@ class Crop < ApplicationRecord end def default_scientific_name - scientific_names.first.name unless scientific_names.empty? - end - - # currently returns the first available photo, but exists so that - # later we can choose a default photo based on different criteria, - # eg. popularity - def default_photo - first_photo(:plantings) || first_photo(:harvests) || first_photo(:seeds) + scientific_names.first&.name end # returns hash indicating whether this crop is grown in @@ -212,8 +194,4 @@ class Crop < ApplicationRecord errors.add(:rejection_notes, "must be added if the reason for rejection is \"other\"") end - - def first_photo(type) - Photo.joins(type).where("#{type}": { crop_id: id }).order("photos.created_at DESC").first - end end diff --git a/app/models/garden.rb b/app/models/garden.rb index 6c8318f42..8eb27cc78 100644 --- a/app/models/garden.rb +++ b/app/models/garden.rb @@ -89,8 +89,4 @@ class Garden < ApplicationRecord p.save end end - - def default_photo - photos.order(created_at: :desc).first - end end diff --git a/app/models/harvest.rb b/app/models/harvest.rb index a913ec8b9..6dffbca6a 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -74,6 +74,10 @@ class Harvest < ApplicationRecord harvested_at - planting.planted_at end + def default_photo + most_liked_photo || planting&.default_photo + end + # we're storing the harvest weight in kilograms in the db too # to make data manipulation easier def set_si_weight @@ -135,10 +139,6 @@ class Harvest < ApplicationRecord end.to_s end - def default_photo - photos.order(created_at: :desc).first || crop.default_photo - end - private def crop_must_match_planting diff --git a/app/models/like.rb b/app/models/like.rb index 05911fc95..f74903739 100644 --- a/app/models/like.rb +++ b/app/models/like.rb @@ -1,6 +1,6 @@ class Like < ApplicationRecord belongs_to :member - belongs_to :likeable, polymorphic: true + belongs_to :likeable, polymorphic: true, counter_cache: true validates :member, :likeable, presence: true validates :member, uniqueness: { scope: :likeable } end diff --git a/app/models/planting.rb b/app/models/planting.rb index e57bf6fb0..67bdd281d 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -85,10 +85,6 @@ class Planting < ApplicationRecord I18n.t('plantings.string', crop: crop.name, garden: garden.name, owner: owner) end - def default_photo - photos.order(created_at: :desc).first - end - def finished? finished || (finished_at.present? && finished_at <= Time.zone.today) end diff --git a/app/models/seed.rb b/app/models/seed.rb index 8025f66e2..b335127c0 100644 --- a/app/models/seed.rb +++ b/app/models/seed.rb @@ -55,10 +55,6 @@ class Seed < ApplicationRecord scope :recent, -> { order(created_at: :desc) } scope :active, -> { where('finished_at < ?', Time.zone.now) } - def default_photo - photos.order(created_at: :desc).first - end - def tradable? tradable_to != 'nowhere' end diff --git a/app/views/crops/_photos.html.haml b/app/views/crops/_photos.html.haml index 170867ed8..71358d541 100644 --- a/app/views/crops/_photos.html.haml +++ b/app/views/crops/_photos.html.haml @@ -3,7 +3,7 @@ - [Planting, Harvest, Seed].each do |model_name| - if photos.by_model(model_name).size.positive? %h3 #{@crop.name.capitalize} #{t("activerecord.models.#{model_name.to_s.downcase}.other")} - = render 'photos/gallery', photos: photos.by_model(model_name).limit(8) + = render 'photos/gallery', photos: photos.by_model(model_name).order(likes_count: :desc).limit(8) = link_to 'more photos ยป', crop_photos_path(@crop), class: 'btn' %hr/ diff --git a/app/views/likes/_count.haml b/app/views/likes/_count.haml index 56f72e7e7..79fece151 100644 --- a/app/views/likes/_count.haml +++ b/app/views/likes/_count.haml @@ -1,4 +1,4 @@ %span.badge.like-badge{class: (likeable.liked_by?(current_member)) ? 'liked' : ''} = like_icon   - %span.like-count= likeable.likes.count + %span.like-count= likeable.likes_count diff --git a/app/views/photos/_card.html.haml b/app/views/photos/_card.html.haml index 46afcbf7b..0d17281f5 100644 --- a/app/views/photos/_card.html.haml +++ b/app/views/photos/_card.html.haml @@ -8,4 +8,3 @@ %time{datetime: photo.date_taken} = I18n.l(photo.date_taken.to_date) = render 'photos/likes', photo: photo - diff --git a/db/migrate/20190712003735_add_like_counter_caches.rb b/db/migrate/20190712003735_add_like_counter_caches.rb new file mode 100644 index 000000000..92c292494 --- /dev/null +++ b/db/migrate/20190712003735_add_like_counter_caches.rb @@ -0,0 +1,34 @@ +class AddLikeCounterCaches < ActiveRecord::Migration[5.2] + def change + change_table :photos do |t| + t.integer :likes_count, default: 0 + end + change_table :posts do |t| + t.integer :likes_count, default: 0 + end + reversible do |dir| + dir.up { data } + end + end + + def data + execute <<-SQL.squish + UPDATE photos + SET likes_count = ( + SELECT count(1) + FROM likes + WHERE likes.likeable_id = photos.id + AND likeable_type = 'Photo' + ) + SQL + execute <<-SQL.squish + UPDATE posts + SET likes_count = ( + SELECT count(1) + FROM likes + WHERE likes.likeable_id = posts.id + AND likeable_type = 'Post' + ) + SQL + end +end diff --git a/db/schema.rb b/db/schema.rb index aaf3d4690..e5faf5c12 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -362,6 +362,7 @@ ActiveRecord::Schema.define(version: 2019_07_12_234859) do t.string "link_url", null: false t.string "flickr_photo_id" t.datetime "date_taken" + t.integer "likes_count", default: 0 end create_table "photos_plantings", id: false, force: :cascade do |t| @@ -412,6 +413,7 @@ ActiveRecord::Schema.define(version: 2019_07_12_234859) do t.datetime "updated_at" t.string "slug" t.integer "forum_id" + t.integer "likes_count", default: 0 t.index ["created_at", "author_id"], name: "index_posts_on_created_at_and_author_id" t.index ["slug"], name: "index_posts_on_slug", unique: true end diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index fa642a540..ed17069c9 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -185,9 +185,9 @@ describe Crop do end it { expect(crop.photos.size).to eq 0 } - it { expect(crop.planting_photos.size).to eq 0 } - it { expect(crop.harvest_photos.size).to eq 0 } - it { expect(crop.seed_photos.size).to eq 0 } + it { expect(crop.photos.by_model(Planting).size).to eq 0 } + it { expect(crop.photos.by_model(Harvest).size).to eq 0 } + it { expect(crop.photos.by_model(Seed).size).to eq 0 } end describe 'finding all photos' do @@ -203,9 +203,9 @@ describe Crop do end it { expect(crop.photos.size).to eq 3 } - it { expect(crop.planting_photos.size).to eq 1 } - it { expect(crop.harvest_photos.size).to eq 1 } - it { expect(crop.seed_photos.size).to eq 1 } + it { expect(crop.photos.by_model(Planting).size).to eq 1 } + it { expect(crop.photos.by_model(Harvest).size).to eq 1 } + it { expect(crop.photos.by_model(Seed).size).to eq 1 } end end diff --git a/spec/models/harvest_spec.rb b/spec/models/harvest_spec.rb index 35cb9ba5f..168b2bcdc 100644 --- a/spec/models/harvest_spec.rb +++ b/spec/models/harvest_spec.rb @@ -237,6 +237,7 @@ describe Harvest do @planting = FactoryBot.create(:planting, crop: @harvest.crop) @photo = FactoryBot.create(:photo, owner: @planting.owner) @planting.photos << @photo + @harvest.update(planting: @planting) end it 'has a default photo' do diff --git a/spec/models/like_spec.rb b/spec/models/like_spec.rb index 363585aea..56f35bf1d 100644 --- a/spec/models/like_spec.rb +++ b/spec/models/like_spec.rb @@ -48,6 +48,12 @@ describe 'like' do expect(Like.all).not_to include like end + it 'destroys like if post no longer exists' do + like = Like.create(member: member, likeable: post) + post.destroy + expect(Like.all).not_to include like + end + it 'destroys like if member no longer exists' do like = Like.create(member: member, likeable: post) member.destroy diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index d71a6cd1c..1102253f8 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -2,18 +2,40 @@ require 'rails_helper' describe Photo do let(:photo) { FactoryBot.create(:photo, owner: member) } - let(:member) { FactoryBot.create(:member) } + let(:old_photo) { FactoryBot.create(:photo, owner: member, created_at: 1.year.ago, date_taken: 2.years.ago) } + let(:member) { FactoryBot.create(:member) } + + it_behaves_like "it is likeable" describe 'add/delete functionality' do let(:planting) { FactoryBot.create(:planting, owner: member) } + let(:seed) { FactoryBot.create(:seed, owner: member) } let(:harvest) { FactoryBot.create(:harvest, owner: member) } - let(:garden) { FactoryBot.create(:garden, owner: member) } + let(:post) { FactoryBot.create(:post, author: member) } + let(:garden) { FactoryBot.create(:garden, owner: member) } context "adds photos" do - it 'to a planting' do - planting.photos << photo - expect(planting.photos.size).to eq 1 - expect(planting.photos.first).to eq photo + describe 'to a planting' do + before { planting.photos << photo } + + it { expect(planting.photos.size).to eq 1 } + it { expect(planting.photos.first).to eq photo } + # there's only one photo, so that's the default + it { expect(planting.default_photo).to eq photo } + it { expect(planting.crop.default_photo).to eq photo } + + describe 'with a second older photo' do + # Add an old photo + before { planting.photos << old_photo } + it { expect(planting.default_photo).to eq photo } + it { expect(planting.crop.default_photo).to eq photo } + + describe 'and someone likes the old photo' do + before { FactoryBot.create :like, likeable: old_photo } + it { expect(planting.default_photo).to eq old_photo } + it { expect(planting.crop.default_photo).to eq old_photo } + end + end end it 'to a harvest' do @@ -22,11 +44,23 @@ describe Photo do expect(harvest.photos.first).to eq photo end + it 'to a seed' do + seed.photos << photo + expect(seed.photos.size).to eq 1 + expect(seed.photos.first).to eq photo + end + it 'to a garden' do garden.photos << photo expect(garden.photos.size).to eq 1 expect(garden.photos.first).to eq photo end + + it 'to a post' do + post.photos << photo + expect(post.photos.size).to eq 1 + expect(post.photos.first).to eq photo + end end context "removing photos" do