From 485b637e042daa47fec950c7b9ca9ba4a4416b07 Mon Sep 17 00:00:00 2001 From: "dependabot-preview[bot]" <27856297+dependabot-preview[bot]@users.noreply.github.com> Date: Mon, 15 Jul 2019 08:52:06 +0000 Subject: [PATCH 01/14] Bump rubocop-rails from 2.2.0 to 2.2.1 Bumps [rubocop-rails](https://github.com/rubocop-hq/rubocop-rails) from 2.2.0 to 2.2.1. - [Release notes](https://github.com/rubocop-hq/rubocop-rails/releases) - [Changelog](https://github.com/rubocop-hq/rubocop-rails/blob/master/CHANGELOG.md) - [Commits](https://github.com/rubocop-hq/rubocop-rails/compare/v2.2.0...v2.2.1) Signed-off-by: dependabot-preview[bot] --- Gemfile.lock | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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) From d4cfcb37baf92908a19c4e54049b3b724420f6ad Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 11 Jul 2019 12:08:15 +1200 Subject: [PATCH 02/14] more percy specs --- spec/features/members/deletion_spec.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/spec/features/members/deletion_spec.rb b/spec/features/members/deletion_spec.rb index 7a5ccf273..9d51ef6b1 100644 --- a/spec/features/members/deletion_spec.rb +++ b/spec/features/members/deletion_spec.rb @@ -81,6 +81,7 @@ describe "member deletion" do visit member_path(member) click_link 'Edit profile' click_link 'Delete Account' + Percy.snapshot(page, name: 'Account deletion') fill_in "current_pw_for_delete", with: "password1", match: :prefer_exact click_button "Delete" logout From 9413d86b055efa4f1841e73cb7e48247b5931087 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 11 Jul 2019 12:14:54 +1200 Subject: [PATCH 03/14] fixes percy error, running same snapshot more than once --- spec/features/members/deletion_spec.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/spec/features/members/deletion_spec.rb b/spec/features/members/deletion_spec.rb index 9d51ef6b1..7a5ccf273 100644 --- a/spec/features/members/deletion_spec.rb +++ b/spec/features/members/deletion_spec.rb @@ -81,7 +81,6 @@ describe "member deletion" do visit member_path(member) click_link 'Edit profile' click_link 'Delete Account' - Percy.snapshot(page, name: 'Account deletion') fill_in "current_pw_for_delete", with: "password1", match: :prefer_exact click_button "Delete" logout From acf58fc5e280755a99ea4b6494ae5554862e3c5f Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 13:08:23 +1200 Subject: [PATCH 04/14] Choose crop photos by most liked --- app/models/concerns/likeable.rb | 2 +- app/models/crop.rb | 6 ++-- app/models/like.rb | 2 +- app/views/crops/_photos.html.haml | 2 +- app/views/likes/_count.haml | 2 +- app/views/photos/_card.html.haml | 1 - .../20190712003735_add_like_counter_caches.rb | 34 +++++++++++++++++++ db/schema.rb | 4 ++- 8 files changed, 44 insertions(+), 9 deletions(-) create mode 100644 db/migrate/20190712003735_add_like_counter_caches.rb diff --git a/app/models/concerns/likeable.rb b/app/models/concerns/likeable.rb index 4937af53e..4760a8f14 100644 --- a/app/models/concerns/likeable.rb +++ b/app/models/concerns/likeable.rb @@ -2,7 +2,7 @@ module Likeable extend ActiveSupport::Concern included do - has_many :likes, as: :likeable, inverse_of: :likeable, dependent: :destroy + has_many :likes, as: :likeable, inverse_of: :likeable, dependent: :destroy, counter_cache: true has_many :members, through: :likes end diff --git a/app/models/crop.rb b/app/models/crop.rb index 4e56d4645..86159d7c4 100644 --- a/app/models/crop.rb +++ b/app/models/crop.rb @@ -85,7 +85,7 @@ class Crop < ApplicationRecord # 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) + most_liked_photo end # returns hash indicating whether this crop is grown in @@ -213,7 +213,7 @@ 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 + def most_liked_photo + photos.order(likes_count: :desc, created_at: :desc).first end end 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/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 d6cb3cbf4..6bbc72242 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2019_03_26_224347) do +ActiveRecord::Schema.define(version: 2019_07_12_003735) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -362,6 +362,7 @@ ActiveRecord::Schema.define(version: 2019_03_26_224347) 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_03_26_224347) 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 From e5e54d651b6d51be7676fe00b4ebd293ec7bb32c Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 13:18:18 +1200 Subject: [PATCH 05/14] Remove duplicate and unused photo methods --- app/models/concerns/photo_capable.rb | 8 ++++++++ app/models/crop.rb | 26 ++------------------------ app/models/harvest.rb | 4 ---- app/models/planting.rb | 4 ---- app/models/seed.rb | 4 ---- 5 files changed, 10 insertions(+), 36 deletions(-) diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index 3ae7303d9..c944d06a5 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: :photographings, 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 86159d7c4..2c8f372b4 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 - most_liked_photo + 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 most_liked_photo - photos.order(likes_count: :desc, created_at: :desc).first - end end diff --git a/app/models/harvest.rb b/app/models/harvest.rb index a913ec8b9..2b24e2fa7 100644 --- a/app/models/harvest.rb +++ b/app/models/harvest.rb @@ -135,10 +135,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/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 From d8df4338a79450a99f5798fc0f6a25858826a32a Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 13:21:36 +1200 Subject: [PATCH 06/14] DRY garden model - get default photo from photo_capable concern --- app/models/garden.rb | 4 ---- 1 file changed, 4 deletions(-) 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 From b654fd483b4d7a85452d230423936eb9088cd183 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:07:41 +1200 Subject: [PATCH 07/14] counter cache was on the wrong side of the relationship --- app/models/concerns/likeable.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/concerns/likeable.rb b/app/models/concerns/likeable.rb index 4760a8f14..4937af53e 100644 --- a/app/models/concerns/likeable.rb +++ b/app/models/concerns/likeable.rb @@ -2,7 +2,7 @@ module Likeable extend ActiveSupport::Concern included do - has_many :likes, as: :likeable, inverse_of: :likeable, dependent: :destroy, counter_cache: true + has_many :likes, as: :likeable, inverse_of: :likeable, dependent: :destroy has_many :members, through: :likes end From 2879979db49b6e23f7fbb3f7c98cf96f06ebcc37 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:08:02 +1200 Subject: [PATCH 08/14] tests that likes are deleted when their photo is deleted --- spec/models/like_spec.rb | 6 ++++++ 1 file changed, 6 insertions(+) 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 From 32695e32412546e354b5637d5f789b1594fca673 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:08:39 +1200 Subject: [PATCH 09/14] photos behave like likeable --- spec/models/photo_spec.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index d71a6cd1c..bbcd2c74b 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -4,6 +4,8 @@ describe Photo do let(:photo) { FactoryBot.create(:photo, owner: member) } let(:member) { FactoryBot.create(:member) } + it_behaves_like "it is likeable" + describe 'add/delete functionality' do let(:planting) { FactoryBot.create(:planting, owner: member) } let(:harvest) { FactoryBot.create(:harvest, owner: member) } From b1ebf67960841bdf4546686882a775ce820f0723 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:13:10 +1200 Subject: [PATCH 10/14] added more possible photo associations in photo model spec --- spec/models/photo_spec.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index bbcd2c74b..1729655c4 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -8,7 +8,9 @@ describe Photo do 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(:post) { FactoryBot.create(:post, author: member) } let(:garden) { FactoryBot.create(:garden, owner: member) } context "adds photos" do @@ -24,11 +26,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 From 60a510e73f65f6f35a3daecced2ab18f004a1672 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:21:18 +1200 Subject: [PATCH 11/14] tests the default photo changes when someone likes a photo --- spec/models/photo_spec.rb | 30 ++++++++++++++++++++++++------ 1 file changed, 24 insertions(+), 6 deletions(-) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index 1729655c4..f21a49270 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -2,7 +2,8 @@ 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" @@ -11,13 +12,30 @@ describe Photo do let(:seed) { FactoryBot.create(:seed, owner: member) } let(:harvest) { FactoryBot.create(:harvest, owner: member) } let(:post) { FactoryBot.create(:post, author: member) } - let(:garden) { FactoryBot.create(:garden, owner: 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 { Like.create(likeable: old_photo) } + it { expect(planting.default_photo).to eq photo } + it { expect(planting.crop.default_photo).to eq photo } + end + end end it 'to a harvest' do From 99c119c07fdd1598db3281f5909f2c01c66d1834 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 15:29:05 +1200 Subject: [PATCH 12/14] fixed looking up photo counts in spec --- spec/models/crop_spec.rb | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) 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 From efe14d43cbff1e619a3dce6a96dcd0d30fe63b50 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 16:36:54 +1200 Subject: [PATCH 13/14] read photo from a harvest's planting --- app/models/harvest.rb | 4 ++++ spec/models/harvest_spec.rb | 1 + 2 files changed, 5 insertions(+) diff --git a/app/models/harvest.rb b/app/models/harvest.rb index 2b24e2fa7..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 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 From 3cd67bcc9ff136e80d134bc3999a5d347b68362e Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 12 Jul 2019 22:27:55 +1200 Subject: [PATCH 14/14] Fix liked photo spec --- spec/models/photo_spec.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/spec/models/photo_spec.rb b/spec/models/photo_spec.rb index f21a49270..1102253f8 100644 --- a/spec/models/photo_spec.rb +++ b/spec/models/photo_spec.rb @@ -31,9 +31,9 @@ describe Photo do it { expect(planting.crop.default_photo).to eq photo } describe 'and someone likes the old photo' do - before { Like.create(likeable: old_photo) } - it { expect(planting.default_photo).to eq photo } - it { expect(planting.crop.default_photo).to eq photo } + 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