From 17fd6f61a54598d1764db6e25b0d3b459b4f55a0 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 29 Nov 2017 17:53:23 +1300 Subject: [PATCH 01/14] Added polymorphic relationship to photos --- app/models/concerns/photo_capable.rb | 12 ++----- app/models/photo.rb | 36 ++++++++----------- app/models/photographing.rb | 4 +++ .../20171129041341_create_photographings.rb | 14 ++++++++ db/schema.rb | 13 ++++++- 5 files changed, 47 insertions(+), 32 deletions(-) create mode 100644 app/models/photographing.rb create mode 100644 db/migrate/20171129041341_create_photographings.rb diff --git a/app/models/concerns/photo_capable.rb b/app/models/concerns/photo_capable.rb index 4b1f90821..177a2f832 100644 --- a/app/models/concerns/photo_capable.rb +++ b/app/models/concerns/photo_capable.rb @@ -1,18 +1,10 @@ -require_relative '../../constants/photo_models.rb' module PhotoCapable extend ActiveSupport::Concern included do - has_and_belongs_to_many :photos # rubocop:disable Rails/HasAndBelongsToMany + has_many :photos, through: :photographings, as: :photographable + has_many :photographings, as: :photographable, dependent: :destroy - before_destroy :remove_from_list scope :has_photos, -> { includes(:photos).where.not(photos: { id: nil }) } end - - def remove_from_list - photolist = photos.to_a # save a temp copy of the photo list - photos.clear # clear relationship b/w object and photo - - photolist.each(&:destroy_if_unused) - end end diff --git a/app/models/photo.rb b/app/models/photo.rb index 402eebd7e..6a3b88113 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,31 +1,17 @@ -require_relative '../constants/photo_models.rb' class Photo < ActiveRecord::Base belongs_to :owner, class_name: 'Member' - Growstuff::Constants::PhotoModels.relations.each do |relation| - has_and_belongs_to_many relation.to_sym # rubocop:disable Rails/HasAndBelongsToMany + has_many :photographings, foreign_key: :photo_id, dependent: :destroy + # creates a relationship for each assignee type + %w(Garden Planting Harvest Seed).each do |type| + has_many type.downcase.pluralize.to_s.to_sym, + through: :photographings, + source: :photographable, + source_type: type end - before_destroy { all_associations.clear } - default_scope { joins(:owner).order(created_at: :desc) } - def associations? - plantings.any? || harvests.any? || gardens.any? || seeds.any? - end - - def all_associations - associations = [] - Growstuff::Constants::PhotoModels.relations.each do |association_name| - associations << send(association_name.to_s).to_a - end - associations.flatten! - end - - def destroy_if_unused - destroy if all_associations.empty? - end - # This is split into a side-effect free method and a side-effecting method # for easier stubbing and testing. def flickr_metadata @@ -43,6 +29,14 @@ class Photo < ActiveRecord::Base } end + def associations? + photographings.size.positive? + end + + def destroy_if_unused + destroy unless associations? + end + def calculate_title(info) if id && title # already has a title saved title diff --git a/app/models/photographing.rb b/app/models/photographing.rb new file mode 100644 index 000000000..aeaa65296 --- /dev/null +++ b/app/models/photographing.rb @@ -0,0 +1,4 @@ +class Photographing < ActiveRecord::Base + belongs_to :photo + belongs_to :photographable, polymorphic: true +end diff --git a/db/migrate/20171129041341_create_photographings.rb b/db/migrate/20171129041341_create_photographings.rb new file mode 100644 index 000000000..ebc374679 --- /dev/null +++ b/db/migrate/20171129041341_create_photographings.rb @@ -0,0 +1,14 @@ +class CreatePhotographings < ActiveRecord::Migration + def change + create_table :photographings do |t| + t.integer :photo_id + t.integer :photographable_id + t.string :photographable_type + t.timestamps null: false + end + add_index :photographings, %i(photographable_id photographable_type photo_id), + name: 'polymorphic_many_to_many_idx' + add_index :photographings, %i(photographable_id photographable_type), + name: 'polymorphic_photographable_idx' + end +end diff --git a/db/schema.rb b/db/schema.rb index 5b11fb94f..c1568cad1 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -11,7 +11,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171105011017) do +ActiveRecord::Schema.define(version: 20171129041341) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -383,6 +383,17 @@ ActiveRecord::Schema.define(version: 20171105011017) do t.integer "product_id" end + create_table "photographings", force: :cascade do |t| + t.integer "photo_id" + t.integer "photographable_id" + t.string "photographable_type" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + end + + add_index "photographings", ["photographable_id", "photographable_type", "photo_id"], name: "polymorphic_many_to_many_idx", using: :btree + add_index "photographings", ["photographable_id", "photographable_type"], name: "polymorphic_photographable_idx", using: :btree + create_table "photos", force: :cascade do |t| t.integer "owner_id", null: false t.string "thumbnail_url", null: false From ff57176d6026acb5da66206794251426d75a0e56 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 29 Nov 2017 21:58:33 +1300 Subject: [PATCH 02/14] Simplified the photos controller a bit --- .../photo_associations_controller.rb | 17 +++++++++++++---- app/controllers/photos_controller.rb | 17 ++++------------- app/models/photo.rb | 4 +++- 3 files changed, 20 insertions(+), 18 deletions(-) diff --git a/app/controllers/photo_associations_controller.rb b/app/controllers/photo_associations_controller.rb index 3259fe3c8..13c71af43 100644 --- a/app/controllers/photo_associations_controller.rb +++ b/app/controllers/photo_associations_controller.rb @@ -4,10 +4,19 @@ class PhotoAssociationsController < ApplicationController def destroy @photo = Photo.find_by!(id: params[:photo_id], owner: current_member) - collection = Growstuff::Constants::PhotoModels.get_relation(@photo, params[:type]) - item_class = Growstuff::Constants::PhotoModels.get_item(params[:type]) - @item = item_class.find_by!(id: params[:id], owner_id: current_member.id) - collection.delete(@item) + + raise "Photos not supported" unless Photo::PHOTO_CAPABLE.include? item_class + @item = @photo.photographings.where(photographable_id: item_id, photographable_type: item_class) + @item.photos.delete(@photo) + # @photo.destroy_if_unused respond_with(@photo) end + + def item_class + params[:type].capitalize + end + + def item_id + params[:id] + end end diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 1f091b19b..4a3b19050 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -36,7 +36,7 @@ class PhotosController < ApplicationController @photo = find_or_create_photo_from_flickr_photo @item = item_to_link_to raise "Could not find this #{type} owned by you" unless @item - collection << @item unless collection.include?(@item) + @item.photos << @photo @photo.save! if @photo.present? end respond_with @photo @@ -74,21 +74,12 @@ class PhotosController < ApplicationController end # Item with photos attached - # def item_to_link_to raise "No item id provided" if item_id.nil? raise "No item type provided" if item_type.nil? - raise "Missing or invalid type provided" unless photos_supported_on_type?(item_type) - item_class = Growstuff::Constants::PhotoModels.get_item(item_type) - item_class.find_by!(id: params[:id], owner_id: current_member.id) - end - - def collection - Growstuff::Constants::PhotoModels.get_relation(@photo, item_type) - end - - def photos_supported_on_type?(_type) - Growstuff::Constants::PhotoModels.types.include?(item_type) + item_class = item_type.capitalize + raise "Photos not supported" unless Photo::PHOTO_CAPABLE.include? item_class + item_class.constantize.find_by!(id: params[:id], owner_id: current_member.id) end # diff --git a/app/models/photo.rb b/app/models/photo.rb index 0241b6872..5800ec6f9 100644 --- a/app/models/photo.rb +++ b/app/models/photo.rb @@ -1,9 +1,11 @@ class Photo < ActiveRecord::Base belongs_to :owner, class_name: 'Member' + PHOTO_CAPABLE = %w(Garden Planting Harvest Seed).freeze + has_many :photographings, foreign_key: :photo_id, dependent: :destroy # creates a relationship for each assignee type - %w(Garden Planting Harvest Seed).each do |type| + PHOTO_CAPABLE.each do |type| has_many type.downcase.pluralize.to_s.to_sym, through: :photographings, source: :photographable, From ccbdfe51f6868be5720199700f5ecc3c585b0308 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 29 Nov 2017 22:30:11 +1300 Subject: [PATCH 03/14] Modified a spec to avoid a chicken and an egg --- spec/features/crops/crop_photos_spec.rb | 26 ++++++++++++++----------- 1 file changed, 15 insertions(+), 11 deletions(-) diff --git a/spec/features/crops/crop_photos_spec.rb b/spec/features/crops/crop_photos_spec.rb index 59eba8f38..260283057 100644 --- a/spec/features/crops/crop_photos_spec.rb +++ b/spec/features/crops/crop_photos_spec.rb @@ -2,27 +2,31 @@ require 'rails_helper' feature "crop detail page", js: true do let(:member) { create :member } + let(:crop) { create :crop, plantings: [planting], harvests: [harvest] } - let(:planting) { create :planting, owner: member, photos: [photo1, photo2] } - let(:harvest) { create :harvest, owner: member, photos: [photo3, photo4] } + let(:planting) { create :planting, owner: member } + let(:harvest) { create :harvest, owner: member } + let(:photo1) do - create(:photo, owner: member, title: 'photo 1', - fullsize_url: 'photo1.jpg', thumbnail_url: 'thumb1.jpg') + create(:photo, owner: member, title: 'photo 1', fullsize_url: 'photo1.jpg', thumbnail_url: 'thumb1.jpg') end let(:photo2) do - create(:photo, owner: member, title: 'photo 2', - fullsize_url: 'photo2.jpg', thumbnail_url: 'thumb2.jpg') + create(:photo, owner: member, title: 'photo 2', fullsize_url: 'photo2.jpg', thumbnail_url: 'thumb2.jpg') end let(:photo3) do - create(:photo, owner: member, title: 'photo 3', - fullsize_url: 'photo3.jpg', thumbnail_url: 'thumb3.jpg') + create(:photo, owner: member, title: 'photo 3', fullsize_url: 'photo3.jpg', thumbnail_url: 'thumb3.jpg') end let(:photo4) do - create(:photo, owner: member, title: 'photo 4', - fullsize_url: 'photo4.jpg', thumbnail_url: 'thumb4.jpg') + create(:photo, owner: member, title: 'photo 4', fullsize_url: 'photo4.jpg', thumbnail_url: 'thumb4.jpg') end - before { visit crop_path(crop) } + before do + planting.photos << photo1 + planting.photos << photo2 + harvest.photos << photo3 + harvest.photos << photo4 + visit crop_path(crop) + end subject { page } shared_examples "shows photos" do From 35ce8b84dbf180f4e0a3aa45cd48509062392d48 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 30 Nov 2017 09:56:47 +1300 Subject: [PATCH 04/14] Adding a unique index on the items <--> photos table --- db/migrate/20171129041341_create_photographings.rb | 4 ++-- db/schema.rb | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/db/migrate/20171129041341_create_photographings.rb b/db/migrate/20171129041341_create_photographings.rb index 52efeea5c..8054ba9cd 100644 --- a/db/migrate/20171129041341_create_photographings.rb +++ b/db/migrate/20171129041341_create_photographings.rb @@ -10,9 +10,9 @@ class CreatePhotographings < ActiveRecord::Migration add_foreign_key :photographings, :photos add_index :photographings, %i(photographable_id photographable_type photo_id), - name: 'polymorphic_many_to_many_idx' + unique: true, name: 'items_to_photos_idx' add_index :photographings, %i(photographable_id photographable_type), - name: 'polymorphic_photographable_idx' + name: 'photographable_idx' migrate_data end diff --git a/db/schema.rb b/db/schema.rb index 3356cc405..473e756fa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -391,8 +391,8 @@ ActiveRecord::Schema.define(version: 20171129041341) do t.datetime "updated_at", null: false end - add_index "photographings", ["photographable_id", "photographable_type", "photo_id"], name: "polymorphic_many_to_many_idx", using: :btree - add_index "photographings", ["photographable_id", "photographable_type"], name: "polymorphic_photographable_idx", using: :btree + add_index "photographings", ["photographable_id", "photographable_type", "photo_id"], name: "items_to_photos_idx", unique: true, using: :btree + add_index "photographings", ["photographable_id", "photographable_type"], name: "photographable_idx", using: :btree create_table "photos", force: :cascade do |t| t.integer "owner_id", null: false From 697779e67c9b58c5860f97b316eed8453a036fd2 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 30 Nov 2017 10:15:05 +1300 Subject: [PATCH 05/14] Don't add item to photo is already there --- app/controllers/photos_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/photos_controller.rb b/app/controllers/photos_controller.rb index 4a3b19050..7fe20a989 100644 --- a/app/controllers/photos_controller.rb +++ b/app/controllers/photos_controller.rb @@ -36,7 +36,7 @@ class PhotosController < ApplicationController @photo = find_or_create_photo_from_flickr_photo @item = item_to_link_to raise "Could not find this #{type} owned by you" unless @item - @item.photos << @photo + @item.photos << @photo unless @item.photos.include? @photo @photo.save! if @photo.present? end respond_with @photo From 14b16b0d8c1b8cb0ffdac43621cd47ea221e12b4 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 30 Nov 2017 11:23:34 +1300 Subject: [PATCH 06/14] Moved photo.items's item lookup to a class method --- app/controllers/photo_associations_controller.rb | 5 ++--- app/models/photographing.rb | 8 ++++++++ 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/app/controllers/photo_associations_controller.rb b/app/controllers/photo_associations_controller.rb index 13c71af43..126aaf9b3 100644 --- a/app/controllers/photo_associations_controller.rb +++ b/app/controllers/photo_associations_controller.rb @@ -3,10 +3,9 @@ class PhotoAssociationsController < ApplicationController respond_to :json, :html def destroy - @photo = Photo.find_by!(id: params[:photo_id], owner: current_member) - raise "Photos not supported" unless Photo::PHOTO_CAPABLE.include? item_class - @item = @photo.photographings.where(photographable_id: item_id, photographable_type: item_class) + @photo = Photo.find_by!(id: params[:photo_id], owner: current_member) + @item = Photographing.item(item_id, item_class) @item.photos.delete(@photo) # @photo.destroy_if_unused respond_with(@photo) diff --git a/app/models/photographing.rb b/app/models/photographing.rb index aeaa65296..3edb78351 100644 --- a/app/models/photographing.rb +++ b/app/models/photographing.rb @@ -1,4 +1,12 @@ class Photographing < ActiveRecord::Base belongs_to :photo belongs_to :photographable, polymorphic: true + + def self.item(item_id, item_type) + find_by!(photographable_id: item_id, photographable_type: item_type).photographable + end + + def item + find_by!(photographable_id: photographable_id, photographable_type: photographable_type).photographable + end end From c4c659f15930b62b560e338d6eaa91d84839a15b Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 30 Nov 2017 13:37:47 +1300 Subject: [PATCH 07/14] DRY specs, and use create_list in crops model spec --- spec/models/crop_spec.rb | 129 ++++++++++++++------------------------- 1 file changed, 46 insertions(+), 83 deletions(-) diff --git a/spec/models/crop_spec.rb b/spec/models/crop_spec.rb index 5d9b671c0..f89cc3377 100644 --- a/spec/models/crop_spec.rb +++ b/spec/models/crop_spec.rb @@ -269,78 +269,55 @@ describe Crop do end context 'interesting' do - it 'lists interesting crops' do - # first, a couple of candidate crops - @crop1 = FactoryBot.create(:crop) - @crop2 = FactoryBot.create(:crop) + let(:photo) { FactoryBot.create :photo } + # first, a couple of candidate crops + let(:crop1) { FactoryBot.create(:crop) } + let(:crop2) { FactoryBot.create(:crop) } - # they need 3+ plantings each to be interesting - (1..3).each do - FactoryBot.create(:planting, crop: @crop1) - end - (1..3).each do - FactoryBot.create(:planting, crop: @crop2) + let(:crop1_planting) { crop1.plantings.first } + let(:crop2_planting) { crop2.plantings.first } + + subject { Crop.interesting } + + describe 'lists interesting crops' do + before do + # they need 3+ plantings each to be interesting + FactoryBot.create_list(:planting, 3, crop: crop1) + FactoryBot.create_list(:planting, 3, crop: crop2) + # crops need 3+ photos to be interesting + crop1_planting.photos = FactoryBot.create_list :photo, 3 + crop2_planting.photos = FactoryBot.create_list :photo, 3 end - # crops need 3+ photos to be interesting - @photo = FactoryBot.create(:photo) - [@crop1, @crop2].each do |c| - (1..3).each do - c.plantings.first.photos << @photo - c.plantings.first.save - end - end - - Crop.interesting.should include @crop1 - Crop.interesting.should include @crop2 - Crop.interesting.size.should == 2 + it { is_expected.to include crop1 } + it { is_expected.to include crop2 } + it { expect(subject.size).to eq 2 } end - it 'ignores crops without plantings' do - # first, a couple of candidate crops - @crop1 = FactoryBot.create(:crop) - @crop2 = FactoryBot.create(:crop) - - # only crop1 has plantings - (1..3).each do - FactoryBot.create(:planting, crop: @crop1) + describe 'crops without plantings are not interesting' do + before do + # only crop1 has plantings + FactoryBot.create_list(:planting, 3, crop: crop1) + # ... and photos + crop1_planting.photos = FactoryBot.create_list(:photo, 3) end - - # ... and photos - @photo = FactoryBot.create(:photo) - (1..3).each do - @crop1.plantings.first.photos << @photo - @crop1.plantings.first.save - end - - Crop.interesting.should include @crop1 - Crop.interesting.should_not include @crop2 - Crop.interesting.size.should == 1 + it { is_expected.to include crop1 } + it { is_expected.not_to include crop2 } + it { expect(subject.size).to eq 1 } end - it 'ignores crops without photos' do - # first, a couple of candidate crops - @crop1 = FactoryBot.create(:crop) - @crop2 = FactoryBot.create(:crop) + describe 'crops without photos are not interesting' do + before do + # both crops have plantings + FactoryBot.create_list(:planting, 3, crop: crop1) + FactoryBot.create_list(:planting, 3, crop: crop2) - # both crops have plantings - (1..3).each do - FactoryBot.create(:planting, crop: @crop1) + # but only crop1 has photos + crop1_planting.photos = FactoryBot.create_list(:photo, 3) end - (1..3).each do - FactoryBot.create(:planting, crop: @crop2) - end - - # but only crop1 has photos - @photo = FactoryBot.create(:photo) - (1..3).each do - @crop1.plantings.first.photos << @photo - @crop1.plantings.first.save - end - - Crop.interesting.should include @crop1 - Crop.interesting.should_not include @crop2 - Crop.interesting.size.should == 1 + it { is_expected.to include crop1 } + it { is_expected.not_to include crop2 } + it { expect(subject.size).to eq 1 } end end @@ -349,34 +326,20 @@ describe Crop do let(:pp2) { FactoryBot.create(:plant_part) } context "harvests" do - let(:h1) do - FactoryBot.create(:harvest, - crop: maize, - plant_part: pp1) - end - - let(:h2) do - FactoryBot.create(:harvest, - crop: maize, - plant_part: pp2) - end - + let(:h1) { FactoryBot.create(:harvest, crop: maize, plant_part: pp1) } + let(:h2) { FactoryBot.create(:harvest, crop: maize, plant_part: pp2) } + let!(:crop) { FactoryBot.create(:crop) } + let!(:harvest) { FactoryBot.create(:harvest, crop: crop) } it "has harvests" do - crop = FactoryBot.create(:crop) - harvest = FactoryBot.create(:harvest, crop: crop) - crop.harvests.should eq [harvest] + expect(crop.harvests).to eq [harvest] end end it "doesn't duplicate plant_parts" do @maize = FactoryBot.create(:maize) @pp1 = FactoryBot.create(:plant_part) - @h1 = FactoryBot.create(:harvest, - crop: @maize, - plant_part: @pp1) - @h2 = FactoryBot.create(:harvest, - crop: @maize, - plant_part: @pp1) + @h1 = FactoryBot.create(:harvest, crop: @maize, plant_part: @pp1) + @h2 = FactoryBot.create(:harvest, crop: @maize, plant_part: @pp1) @maize.plant_parts.should eq [@pp1] end From 1c8b75b7cbd76ced27f29062b593e34f85fc42f1 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Wed, 13 Dec 2017 22:32:59 +1300 Subject: [PATCH 08/14] add and show photos for seeds close #1457 --- app/views/seeds/_actions.html.haml | 40 ++++++++++++++++++++++++++++++ app/views/seeds/show.html.haml | 15 ++++++----- 2 files changed, 49 insertions(+), 6 deletions(-) create mode 100644 app/views/seeds/_actions.html.haml diff --git a/app/views/seeds/_actions.html.haml b/app/views/seeds/_actions.html.haml new file mode 100644 index 000000000..f162f5184 --- /dev/null +++ b/app/views/seeds/_actions.html.haml @@ -0,0 +1,40 @@ +- if can? :edit, @seed + = link_to edit_seed_path(@seed), class: 'btn btn-default btn-xs' do + %span.glyphicon.glyphicon-pencil{ title: "Edit" } + Edit + = link_to new_photo_path(id: seed.id, type: 'seed'), class: 'btn btn-default btn-xs' do + %span.glyphicon.glyphicon-camera{ title: "Add photo" } + Add photo +- if can? :destroy, @seed + = link_to @seed, method: :delete, data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs' do + %span.glyphicon.glyphicon-trash{ title: "Delete" } + Delete + + +-# - if can?(:edit, planting) || can?(:destroy, planting) +-# - if can? :edit, planting +-# = link_to edit_planting_path(planting), class: 'btn btn-default btn-xs' do +-# %span.glyphicon.glyphicon-pencil{ title: "Edit" } +-# Edit +-# - if can? :destroy, planting +-# = link_to planting, method: :delete, +-# data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs' do +-# %span.glyphicon.glyphicon-trash{ title: "Delete" } +-# Delete + +-# - unless planting.finished +-# = link_to planting_path(planting, planting: { finished: 1 }), +-# method: :put, class: 'btn btn-default btn-xs append-date' do + +-# %span.glyphicon.glyphicon-ok{ title: "Finished" } +-# Mark as finished + +-# - if can?(:create, Harvest) +-# = link_to new_planting_harvest_path(planting), class: 'btn btn-default btn-xs' do +-# %span.glyphicon.glyphicon-leaf{ title: "Harvest" } +-# Harvest + +-# - if can?(:edit, planting) && can?(:create, Photo) +-# = link_to new_photo_path(id: planting.id, type: 'planting'), class: 'btn btn-default btn-xs' do +-# %span.glyphicon.glyphicon-camera{ title: "Add photo" } +-# Add photo diff --git a/app/views/seeds/show.html.haml b/app/views/seeds/show.html.haml index 11579acd0..e1d63addc 100644 --- a/app/views/seeds/show.html.haml +++ b/app/views/seeds/show.html.haml @@ -58,15 +58,11 @@ subject: "Interested in your #{@seed.crop} seeds"), class: 'btn btn-primary' - else - = render partial: 'shared/signin_signup', locals: { to: 'request seeds' } + = render 'shared/signin_signup', to: 'request seeds' - if can?(:edit, @seed) || can?(:destroy, @seed) %p - - if can? :edit, @seed - = link_to 'Edit', edit_seed_path(@seed), class: 'btn btn-default btn-xs' - - if can? :destroy, @seed - = link_to 'Delete', @seed, method: :delete, data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs' - + = render 'actions', seed: @seed .col-md-6 = render partial: "crops/index_card", locals: { crop: @seed.crop } - if @seed.owner.location @@ -79,3 +75,10 @@ Or = link_to "purchase seeds via Ebay", crop_ebay_seeds_url(@seed.crop), target: "_blank", rel: "noopener noreferrer" +.row + - @seed.photos.includes(:owner).each do |p| + .col-md-2.six-across + = render 'photos/thumbnail', photo: p + - if can?(:create, Photo) && can?(:edit, @seed) + .col-md-2 + = link_to "Add photo", new_photo_path(type: "seed", id: @seed.id), class: 'btn btn-primary' From fcd3e36e36761ea4cc57dfe9b7019312d7e2e50f Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Thu, 14 Dec 2017 08:42:51 +1300 Subject: [PATCH 09/14] Removed commented out code --- app/views/seeds/_actions.html.haml | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/app/views/seeds/_actions.html.haml b/app/views/seeds/_actions.html.haml index f162f5184..9584182f0 100644 --- a/app/views/seeds/_actions.html.haml +++ b/app/views/seeds/_actions.html.haml @@ -9,32 +9,3 @@ = link_to @seed, method: :delete, data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs' do %span.glyphicon.glyphicon-trash{ title: "Delete" } Delete - - --# - if can?(:edit, planting) || can?(:destroy, planting) --# - if can? :edit, planting --# = link_to edit_planting_path(planting), class: 'btn btn-default btn-xs' do --# %span.glyphicon.glyphicon-pencil{ title: "Edit" } --# Edit --# - if can? :destroy, planting --# = link_to planting, method: :delete, --# data: { confirm: 'Are you sure?' }, class: 'btn btn-default btn-xs' do --# %span.glyphicon.glyphicon-trash{ title: "Delete" } --# Delete - --# - unless planting.finished --# = link_to planting_path(planting, planting: { finished: 1 }), --# method: :put, class: 'btn btn-default btn-xs append-date' do - --# %span.glyphicon.glyphicon-ok{ title: "Finished" } --# Mark as finished - --# - if can?(:create, Harvest) --# = link_to new_planting_harvest_path(planting), class: 'btn btn-default btn-xs' do --# %span.glyphicon.glyphicon-leaf{ title: "Harvest" } --# Harvest - --# - if can?(:edit, planting) && can?(:create, Photo) --# = link_to new_photo_path(id: planting.id, type: 'planting'), class: 'btn btn-default btn-xs' do --# %span.glyphicon.glyphicon-camera{ title: "Add photo" } --# Add photo From ff652ae23484eb2d34ef4cc361bc621453ee613b Mon Sep 17 00:00:00 2001 From: deppbot Date: Thu, 14 Dec 2017 20:10:52 +0800 Subject: [PATCH 10/14] Bundle Update on 2017-12-14 --- Gemfile.lock | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index de0b44b30..d18bb7ea6 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -58,7 +58,7 @@ GEM public_suffix (>= 2.0.2, < 4.0) arel (6.0.4) ast (2.3.0) - autoprefixer-rails (7.2.2) + autoprefixer-rails (7.2.3) execjs bcrypt (3.1.11) better_errors (2.2.0) @@ -353,8 +353,8 @@ GEM omniauth-oauth (1.1.0) oauth omniauth (~> 1.0) - omniauth-oauth2 (1.4.0) - oauth2 (~> 1.0) + omniauth-oauth2 (1.5.0) + oauth2 (~> 1.1) omniauth (~> 1.2) omniauth-twitter (1.4.0) omniauth-oauth (~> 1.1) @@ -376,7 +376,7 @@ GEM moneta (~> 0.8.1) plupload-rails (1.2.1) rails (>= 3.1) - poltergeist (1.16.0) + poltergeist (1.17.0) capybara (~> 2.1) cliver (~> 0.3.1) websocket-driver (>= 0.2.0) From 8697e56323badc51479ca759f141930233da66f3 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 15 Dec 2017 13:26:48 +1300 Subject: [PATCH 11/14] Order the plantings on the front page --- app/models/planting.rb | 1 + app/views/home/_crops.html.haml | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/app/models/planting.rb b/app/models/planting.rb index 85e5277c0..c51166264 100644 --- a/app/models/planting.rb +++ b/app/models/planting.rb @@ -26,6 +26,7 @@ class Planting < ActiveRecord::Base scope :finished, -> { where(finished: true) } scope :current, -> { where(finished: false) } scope :interesting, -> { has_photos.one_per_owner } + scope :recent, -> { order(created_at: :desc) } scope :one_per_owner, lambda { joins("JOIN members m ON (m.id=plantings.owner_id) LEFT OUTER JOIN plantings p2 diff --git a/app/views/home/_crops.html.haml b/app/views/home/_crops.html.haml index 274cd4957..6f881dc32 100644 --- a/app/views/home/_crops.html.haml +++ b/app/views/home/_crops.html.haml @@ -14,7 +14,7 @@ .col-md-4.hidden-xs - cache cache_key_for(Planting) do %h2= t('.recently_planted') - = render partial: 'plantings/list', locals: { plantings: Planting.includes(:owner, :photos).interesting.first(6) } + = render 'plantings/list', plantings: Planting.includes(:owner, :photos).interesting.recent.first(6) .row .col-md-12 From 3d3a3b669e2d0d7971dd63c1a44532e7478695c6 Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 15 Dec 2017 13:28:41 +1300 Subject: [PATCH 12/14] Order posts by most recent --- app/models/post.rb | 2 +- app/views/home/_discuss.html.haml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/post.rb b/app/models/post.rb index 76dfcd5d8..43885e99d 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -43,7 +43,7 @@ class Post < ActiveRecord::Base # return posts sorted by recent activity def self.recently_active - Post.all.sort do |a, b| + Post.order(created_at: :desc).sort do |a, b| b.recent_activity <=> a.recent_activity end end diff --git a/app/views/home/_discuss.html.haml b/app/views/home/_discuss.html.haml index 9ffc6b502..36cc1d0f9 100644 --- a/app/views/home/_discuss.html.haml +++ b/app/views/home/_discuss.html.haml @@ -2,7 +2,7 @@ - posts = Post.limit(6) - if posts - = render partial: "posts/summary", locals: { posts: posts, howmany: 6 } + = render "posts/summary", posts: posts, howmany: 6 - cache cache_key_for(Forum) do - forums = Forum.all.order(:name) From 3aa2f0bd3b6559603e8a81ce09503333d9a8332c Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Fri, 15 Dec 2017 10:12:08 +1300 Subject: [PATCH 13/14] Paginate photos on seeds#show (inc feature specs) --- .hound.yml | 4 +-- app/controllers/seeds_controller.rb | 1 + app/views/seeds/show.html.haml | 6 ++++- spec/factories/photos.rb | 8 +++--- spec/features/seeds/seed_photos.rb | 41 +++++++++++++++++++++++++++++ 5 files changed, 53 insertions(+), 7 deletions(-) create mode 100644 spec/features/seeds/seed_photos.rb diff --git a/.hound.yml b/.hound.yml index d62d2897a..869e42463 100644 --- a/.hound.yml +++ b/.hound.yml @@ -6,5 +6,5 @@ haml: config_file: .haml-lint.yml scss: config_file: .scss-lint.yml -eslint: - config_file: .eslintrc +# eslint: +# config_file: .eslintrc diff --git a/app/controllers/seeds_controller.rb b/app/controllers/seeds_controller.rb index f7cf530eb..12445877e 100644 --- a/app/controllers/seeds_controller.rb +++ b/app/controllers/seeds_controller.rb @@ -19,6 +19,7 @@ class SeedsController < ApplicationController # GET /seeds/1 # GET /seeds/1.json def show + @photos = @seed.photos.includes(:owner).order(created_at: :desc).paginate(page: params[:page]) respond_with(@seed) end diff --git a/app/views/seeds/show.html.haml b/app/views/seeds/show.html.haml index e1d63addc..7caf02972 100644 --- a/app/views/seeds/show.html.haml +++ b/app/views/seeds/show.html.haml @@ -76,7 +76,11 @@ = link_to "purchase seeds via Ebay", crop_ebay_seeds_url(@seed.crop), target: "_blank", rel: "noopener noreferrer" .row - - @seed.photos.includes(:owner).each do |p| + .pagination + = page_entries_info @photos + = will_paginate @photos +.row + - @photos.each do |p| .col-md-2.six-across = render 'photos/thumbnail', photo: p - if can?(:create, Photo) && can?(:edit, @seed) diff --git a/spec/factories/photos.rb b/spec/factories/photos.rb index aef506bf0..455d721c7 100644 --- a/spec/factories/photos.rb +++ b/spec/factories/photos.rb @@ -4,12 +4,12 @@ FactoryBot.define do factory :photo do owner flickr_photo_id 1 - title "Still life with chillies" + title { Faker::HarryPotter.quote } license_name "CC-BY" license_url "http://example.com/license.html" - thumbnail_url "http://example.com/thumb.jpg" - fullsize_url "http://example.com/full.jpg" - link_url "http://example.com/" + thumbnail_url { "http://example.com/#{Faker::File.file_name}.jpg" } + fullsize_url { "http://example.com/#{Faker::File.file_name}.jpg" } + link_url { Faker::Internet.url } factory :unlicensed_photo do license_name "All rights reserved" diff --git a/spec/features/seeds/seed_photos.rb b/spec/features/seeds/seed_photos.rb new file mode 100644 index 000000000..635b4dc4d --- /dev/null +++ b/spec/features/seeds/seed_photos.rb @@ -0,0 +1,41 @@ +require 'rails_helper' +require 'custom_matchers' + +feature "Seeds", :js do + let(:member) { FactoryBot.create :member } + let!(:seed) { FactoryBot.create :seed, owner: member } + + subject do + login_as member + visit seed_path(seed) + page + end + + it { is_expected.to have_content 'Add photo' } + + # context 'no photos' do + # it { is_expected.to have_content 'no photos' } + # end + context 'has one photo' do + before { seed.photos = [photo] } + let!(:photo) { FactoryBot.create :photo, title: 'hello photo' } + it { is_expected.to have_xpath("//img[contains(@src,'#{photo.thumbnail_url}')]") } + it { is_expected.to have_xpath("//a[contains(@href,'#{photo_path(photo)}')]") } + end + context 'has 50 photos' do + before { seed.photos = photos } + let!(:photos) { FactoryBot.create_list :photo, 50 } + it "shows newest photo" do + is_expected.to have_xpath("//img[contains(@src,'#{photos.last.thumbnail_url}')]") + end + it "links to newest photo" do + is_expected.to have_xpath("//a[contains(@href,'#{photo_path(photos.last)}')]") + end + it "does not show oldest photo" do + is_expected.not_to have_xpath("//img[contains(@src,'#{photos.first.thumbnail_url}')]") + end + it "does not link to oldest photo" do + is_expected.not_to have_xpath("//a[contains(@href,'#{photo_path(photos.first)}')]") + end + end +end From 6f2b4b13cd5999b17eda811290b52bc4ec912dfb Mon Sep 17 00:00:00 2001 From: Brenda Wallace Date: Sun, 17 Dec 2017 09:55:52 +1300 Subject: [PATCH 14/14] Only show photo pagination if there are photos --- app/views/seeds/show.html.haml | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/app/views/seeds/show.html.haml b/app/views/seeds/show.html.haml index 7caf02972..b1573c7b0 100644 --- a/app/views/seeds/show.html.haml +++ b/app/views/seeds/show.html.haml @@ -75,10 +75,12 @@ Or = link_to "purchase seeds via Ebay", crop_ebay_seeds_url(@seed.crop), target: "_blank", rel: "noopener noreferrer" -.row - .pagination - = page_entries_info @photos - = will_paginate @photos + +- if @photos.size.positive? + .row + .pagination + = page_entries_info @photos + = will_paginate @photos .row - @photos.each do |p| .col-md-2.six-across