Merge remote-tracking branch 'upstream/dev' into renamed-photographings-associations

Conflicts:
	db/schema.rb

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)

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

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

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

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

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

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

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

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/

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

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
-

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

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

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
 

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

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

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