From de67be3021b7e41e17dc436ff6909dcdd55e2669 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Wed, 6 Sep 2017 17:24:18 -0500 Subject: [PATCH 01/11] add migration for new starts_at and ends_at columns --- ...647_add_starts_at_and_ends_at_columns_to_spree_slides.rb | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 db/migrate/20170906023647_add_starts_at_and_ends_at_columns_to_spree_slides.rb diff --git a/db/migrate/20170906023647_add_starts_at_and_ends_at_columns_to_spree_slides.rb b/db/migrate/20170906023647_add_starts_at_and_ends_at_columns_to_spree_slides.rb new file mode 100644 index 0000000..75eb33c --- /dev/null +++ b/db/migrate/20170906023647_add_starts_at_and_ends_at_columns_to_spree_slides.rb @@ -0,0 +1,6 @@ +class AddStartsAtAndEndsAtColumnsToSpreeSlides < ActiveRecord::Migration + def change + add_column :spree_slides, :starts_at, :datetime + add_column :spree_slides, :ends_at, :datetime + end +end From 78d54612499c7c963db367665ebbf99f6e8963a1 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Wed, 6 Sep 2017 17:30:27 -0500 Subject: [PATCH 02/11] add fields for starts_at and ends_at in slider admin This makes possible to fill these fields from the slider admin. --- app/controllers/spree/admin/slides_controller.rb | 11 ++++++++++- app/views/spree/admin/slides/_form.html.erb | 16 ++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/app/controllers/spree/admin/slides_controller.rb b/app/controllers/spree/admin/slides_controller.rb index 4a86b4c..0c13da4 100644 --- a/app/controllers/spree/admin/slides_controller.rb +++ b/app/controllers/spree/admin/slides_controller.rb @@ -18,7 +18,16 @@ def location_after_save end def slide_params - params.require(:slide).permit(:name, :body, :link_url, :published, :image, :position, :product_id) + params.require(:slide) + .permit(:name, + :body, + :link_url, + :published, + :image, + :position, + :product_id, + :starts_at, + :ends_at) end end end diff --git a/app/views/spree/admin/slides/_form.html.erb b/app/views/spree/admin/slides/_form.html.erb index 529321e..d87b0ef 100644 --- a/app/views/spree/admin/slides/_form.html.erb +++ b/app/views/spree/admin/slides/_form.html.erb @@ -36,6 +36,22 @@ <% end %> +
+ <%= f.field_container :starts_at do %> + <%= f.label :starts_at, 'Starts at' %>
+
The slide will not be shown before this date, even if published
+ <%= f.date_field :starts_at, 'Starts at' %>
+ <% end %> +
+ +
+ <%= f.field_container :ends_at do %> + <%= f.label :ends_at, 'Ends at' %>
+
The slide will not be shown after this date, even if published
+ <%= f.date_field :ends_at, 'Ends at' %>
+ <% end %> +
+
<%= render 'spree/admin/slides/edit_slider_locations', f: f %>
From 69c3c1e6ebada9cc8d800b717180f33898aac374 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Wed, 6 Sep 2017 18:57:57 -0500 Subject: [PATCH 03/11] add scope to load all in time slides from database --- app/models/spree/slide.rb | 17 +++++- app/views/spree/admin/slides/_form.html.erb | 4 +- spec/models/spree/slide_decorator_spec.rb | 68 +++++++++++++++++++++ 3 files changed, 84 insertions(+), 5 deletions(-) create mode 100644 spec/models/spree/slide_decorator_spec.rb diff --git a/app/models/spree/slide.rb b/app/models/spree/slide.rb index c8d8774..b56f31f 100644 --- a/app/models/spree/slide.rb +++ b/app/models/spree/slide.rb @@ -1,5 +1,4 @@ class Spree::Slide < ActiveRecord::Base - has_and_belongs_to_many :slide_locations, class_name: 'Spree::SlideLocation', join_table: 'spree_slide_slide_locations' @@ -8,13 +7,21 @@ class Spree::Slide < ActiveRecord::Base url: '/spree/slides/:id/:style/:basename.:extension', path: ':rails_root/public/spree/slides/:id/:style/:basename.:extension', convert_options: { all: '-strip -auto-orient -colorspace sRGB' } - validates_attachment :image, content_type: { content_type: ["image/jpg", "image/jpeg", "image/png", "image/gif"] } + validates_attachment :image, content_type: { content_type: ['image/jpg', 'image/jpeg', 'image/png', 'image/gif'] } scope :published, -> { where(published: true).order('position ASC') } - scope :location, -> (location) { joins(:slide_locations).where('spree_slide_locations.name = ?', location) } + scope :location, ->(location) { joins(:slide_locations).where('spree_slide_locations.name = ?', location) } belongs_to :product, touch: true + def self.in_time + where '(starts_at is NULL AND ends_at is NULL) + OR (starts_at <= ? AND ends_at is NULL) + OR (starts_at is NULL AND ends_at >= ?) + OR (starts_at <= ? AND ends_at >= ?)', + *([Time.now] * 4) + end + def initialize(attrs = nil) attrs ||= { published: true } super @@ -31,4 +38,8 @@ def slide_link def slide_image !image.file? && product.present? && product.images.any? ? product.images.first.attachment : image end + + def in_time? + Time.now.between?(starts_at || 1.second.ago, ends_at || 1.second.from_now) + end end diff --git a/app/views/spree/admin/slides/_form.html.erb b/app/views/spree/admin/slides/_form.html.erb index d87b0ef..5ca0bd3 100644 --- a/app/views/spree/admin/slides/_form.html.erb +++ b/app/views/spree/admin/slides/_form.html.erb @@ -40,7 +40,7 @@ <%= f.field_container :starts_at do %> <%= f.label :starts_at, 'Starts at' %>
The slide will not be shown before this date, even if published
- <%= f.date_field :starts_at, 'Starts at' %>
+ <%= f.date_field :starts_at, class: 'fullwidth form-control' %>
<% end %> @@ -48,7 +48,7 @@ <%= f.field_container :ends_at do %> <%= f.label :ends_at, 'Ends at' %>
The slide will not be shown after this date, even if published
- <%= f.date_field :ends_at, 'Ends at' %>
+ <%= f.date_field :ends_at, class: 'fullwidth form-control' %>
<% end %> diff --git a/spec/models/spree/slide_decorator_spec.rb b/spec/models/spree/slide_decorator_spec.rb new file mode 100644 index 0000000..7b6a7be --- /dev/null +++ b/spec/models/spree/slide_decorator_spec.rb @@ -0,0 +1,68 @@ +require 'spec_helper' + +RSpec.describe Spree::Slide do + describe '#in_time?' do + context 'when both starts_at and ends_at are nil' do + subject { Spree::Slide.new starts_at: nil, ends_at: nil } + + it { is_expected.to be_in_time } + end + + context 'when starts_at is in the past and ends_at is nil' do + subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: nil } + + it { is_expected.to be_in_time } + end + + context 'when starts_at is in the future and ends_at is nil' do + subject { Spree::Slide.new starts_at: 2.days.from_now, ends_at: nil } + + it { is_expected.not_to be_in_time } + end + + context 'when starts_at is nil and ends_at is in the future' do + subject { Spree::Slide.new starts_at: nil, ends_at: 2.days.from_now } + + it { is_expected.to be_in_time } + end + + context 'when starts_at is nil and ends_at is in the past' do + subject { Spree::Slide.new starts_at: nil, ends_at: 2.days.ago } + + it { is_expected.not_to be_in_time } + end + + context 'when both starts_at and end_at is in the past' do + subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: 1.day.ago } + + it { is_expected.not_to be_in_time } + end + + context 'when starts_at is in the past and end_at is in the future' do + subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: 2.days.from_now } + + it { is_expected.to be_in_time } + end + end + + describe '.in_time' do + it 'returns all the slides from the database that are in time' do + good_slides = [ + Spree::Slide.create(starts_at: nil, ends_at: nil), + Spree::Slide.create(starts_at: 2.days.ago, ends_at: nil), + Spree::Slide.create(starts_at: nil, ends_at: 2.days.from_now), + Spree::Slide.create(starts_at: 2.days.ago, ends_at: 2.days.from_now) + ].map(&:id) + + bad_slides = [ + Spree::Slide.create(starts_at: 2.days.from_now, ends_at: nil), + Spree::Slide.create(starts_at: nil, ends_at: 2.days.ago), + Spree::Slide.create(starts_at: 2.days.ago, ends_at: 1.day.ago) + ].map(&:id) + + slides = Spree::Slide.in_time.map(&:id) + expect(slides).to include *good_slides + expect(slides).not_to include *bad_slides + end + end +end From 4ced35bc24c6e9b2308c441e2ef2b1c5cdf9b55b Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Thu, 7 Sep 2017 11:40:40 -0500 Subject: [PATCH 04/11] refactor new methods in Slide model --- app/models/spree/slide.rb | 4 ++-- spec/models/spree/slide_decorator_spec.rb | 22 +++++++++++----------- 2 files changed, 13 insertions(+), 13 deletions(-) diff --git a/app/models/spree/slide.rb b/app/models/spree/slide.rb index b56f31f..675a4bd 100644 --- a/app/models/spree/slide.rb +++ b/app/models/spree/slide.rb @@ -14,7 +14,7 @@ class Spree::Slide < ActiveRecord::Base belongs_to :product, touch: true - def self.in_time + def self.active_for_current_time where '(starts_at is NULL AND ends_at is NULL) OR (starts_at <= ? AND ends_at is NULL) OR (starts_at is NULL AND ends_at >= ?) @@ -39,7 +39,7 @@ def slide_image !image.file? && product.present? && product.images.any? ? product.images.first.attachment : image end - def in_time? + def active_now? Time.now.between?(starts_at || 1.second.ago, ends_at || 1.second.from_now) end end diff --git a/spec/models/spree/slide_decorator_spec.rb b/spec/models/spree/slide_decorator_spec.rb index 7b6a7be..40ad1e5 100644 --- a/spec/models/spree/slide_decorator_spec.rb +++ b/spec/models/spree/slide_decorator_spec.rb @@ -1,52 +1,52 @@ require 'spec_helper' RSpec.describe Spree::Slide do - describe '#in_time?' do + describe '#active_now?' do context 'when both starts_at and ends_at are nil' do subject { Spree::Slide.new starts_at: nil, ends_at: nil } - it { is_expected.to be_in_time } + it { is_expected.to be_active_now } end context 'when starts_at is in the past and ends_at is nil' do subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: nil } - it { is_expected.to be_in_time } + it { is_expected.to be_active_now } end context 'when starts_at is in the future and ends_at is nil' do subject { Spree::Slide.new starts_at: 2.days.from_now, ends_at: nil } - it { is_expected.not_to be_in_time } + it { is_expected.not_to be_active_now } end context 'when starts_at is nil and ends_at is in the future' do subject { Spree::Slide.new starts_at: nil, ends_at: 2.days.from_now } - it { is_expected.to be_in_time } + it { is_expected.to be_active_now } end context 'when starts_at is nil and ends_at is in the past' do subject { Spree::Slide.new starts_at: nil, ends_at: 2.days.ago } - it { is_expected.not_to be_in_time } + it { is_expected.not_to be_active_now } end context 'when both starts_at and end_at is in the past' do subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: 1.day.ago } - it { is_expected.not_to be_in_time } + it { is_expected.not_to be_active_now } end context 'when starts_at is in the past and end_at is in the future' do subject { Spree::Slide.new starts_at: 2.days.ago, ends_at: 2.days.from_now } - it { is_expected.to be_in_time } + it { is_expected.to be_active_now } end end - describe '.in_time' do - it 'returns all the slides from the database that are in time' do + describe '.active_for_current_time' do + it 'returns all the slides from the database that are active for current time' do good_slides = [ Spree::Slide.create(starts_at: nil, ends_at: nil), Spree::Slide.create(starts_at: 2.days.ago, ends_at: nil), @@ -60,7 +60,7 @@ Spree::Slide.create(starts_at: 2.days.ago, ends_at: 1.day.ago) ].map(&:id) - slides = Spree::Slide.in_time.map(&:id) + slides = Spree::Slide.active_for_current_time.map(&:id) expect(slides).to include *good_slides expect(slides).not_to include *bad_slides end From e4d3d71754b32d38e7dffa89c4f51b315c32c366 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Thu, 7 Sep 2017 12:23:41 -0500 Subject: [PATCH 05/11] change Time.now for Time.current --- app/models/spree/slide.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/spree/slide.rb b/app/models/spree/slide.rb index 675a4bd..c5df8bf 100644 --- a/app/models/spree/slide.rb +++ b/app/models/spree/slide.rb @@ -19,7 +19,7 @@ def self.active_for_current_time OR (starts_at <= ? AND ends_at is NULL) OR (starts_at is NULL AND ends_at >= ?) OR (starts_at <= ? AND ends_at >= ?)', - *([Time.now] * 4) + *([Time.current] * 4) end def initialize(attrs = nil) @@ -40,6 +40,6 @@ def slide_image end def active_now? - Time.now.between?(starts_at || 1.second.ago, ends_at || 1.second.from_now) + Time.current.between?(starts_at || 1.second.ago, ends_at || 1.second.from_now) end end From 5ee54ee0848eb4f3a69b2daf523e4eb89b1ac37d Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Thu, 7 Sep 2017 13:50:58 -0500 Subject: [PATCH 06/11] rename spec file for Slide model --- spec/models/spree/{slide_decorator_spec.rb => slide_spec.rb} | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename spec/models/spree/{slide_decorator_spec.rb => slide_spec.rb} (100%) diff --git a/spec/models/spree/slide_decorator_spec.rb b/spec/models/spree/slide_spec.rb similarity index 100% rename from spec/models/spree/slide_decorator_spec.rb rename to spec/models/spree/slide_spec.rb From 1272cb10137735492053dfa3a71ae5a7ff315427 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Thu, 7 Sep 2017 18:52:53 -0500 Subject: [PATCH 07/11] add migration to add location fallback column --- app/models/spree/slide_location.rb | 1 + db/migrate/20170907185000_add_fallback_slide_column.rb | 5 +++++ 2 files changed, 6 insertions(+) create mode 100644 db/migrate/20170907185000_add_fallback_slide_column.rb diff --git a/app/models/spree/slide_location.rb b/app/models/spree/slide_location.rb index 6993052..36c5402 100644 --- a/app/models/spree/slide_location.rb +++ b/app/models/spree/slide_location.rb @@ -3,6 +3,7 @@ class Spree::SlideLocation < ActiveRecord::Base has_and_belongs_to_many :slides, class_name: 'Spree::Slide', join_table: 'spree_slide_slide_locations' + belongs_to :fallback_slide, class_name: 'Spree::Slide', foreign_key: 'fallback_slide_id' validates :name, presence: true diff --git a/db/migrate/20170907185000_add_fallback_slide_column.rb b/db/migrate/20170907185000_add_fallback_slide_column.rb new file mode 100644 index 0000000..312a6ac --- /dev/null +++ b/db/migrate/20170907185000_add_fallback_slide_column.rb @@ -0,0 +1,5 @@ +class AddFallbackSlideColumn < ActiveRecord::Migration + def change + add_column 'spree_slide_locations', :fallback_slide_id, :integer, default: nil + end +end From e48ffef70a19a34e055d5376c05e24d6d35b04d7 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Sun, 10 Sep 2017 19:59:43 -0500 Subject: [PATCH 08/11] edit fallback slide for each location in admin --- app/views/spree/admin/slide_locations/_form.html.erb | 6 ++++++ app/views/spree/admin/slide_locations/index.html.erb | 9 +++++++++ 2 files changed, 15 insertions(+) diff --git a/app/views/spree/admin/slide_locations/_form.html.erb b/app/views/spree/admin/slide_locations/_form.html.erb index 593aa1a..53980c1 100644 --- a/app/views/spree/admin/slide_locations/_form.html.erb +++ b/app/views/spree/admin/slide_locations/_form.html.erb @@ -5,6 +5,12 @@ <%= f.label :name, t(:name) %>
<%= f.text_field :name, class: "form-control fullwidth" %> <% end %> + + <%= f.field_container :fallback_slide_id do %> + <%= f.label :fallback_slide_id, 'Fallback slide' %>
+
This slide will be shown only when no other slide can be shown.
+ <%= f.collection_select(:fallback_slide_id, Spree::Slide.all, :id, :name, { include_blank: Spree.t('match_choices.none') }, { class: 'select2', disabled: (cannot? :edit, Spree::SlideLocation) }) %> + <% end %> diff --git a/app/views/spree/admin/slide_locations/index.html.erb b/app/views/spree/admin/slide_locations/index.html.erb index f45ee4c..e99b967 100644 --- a/app/views/spree/admin/slide_locations/index.html.erb +++ b/app/views/spree/admin/slide_locations/index.html.erb @@ -10,6 +10,7 @@ <%= Spree.t(:name) %> + Fallback slide @@ -17,6 +18,14 @@ <% @slide_locations.each do |location|%> <%= location.name %> + + <% if location.fallback_slide %> + <%= image_tag location.fallback_slide.try(:slide_image), style: 'width: 120px; height: auto;' %> + <%= link_to location.fallback_slide.name, admin_slide_path(location.fallback_slide) %> + <% else %> + None + <% end %> + <%= link_to_edit location, no_text: true, class: 'edit' %>   From ff150e2ce94c9164f7f9b1be58b74bd9acc91003 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Mon, 11 Sep 2017 12:54:04 -0500 Subject: [PATCH 09/11] exclude fallback slide from location slides The fallback slide should only be retrieved when there is no slides to be shown. --- app/models/spree/slide_location.rb | 4 ++-- spec/models/spree/slide_location_spec.rb | 19 +++++++++++++++++++ 2 files changed, 21 insertions(+), 2 deletions(-) create mode 100644 spec/models/spree/slide_location_spec.rb diff --git a/app/models/spree/slide_location.rb b/app/models/spree/slide_location.rb index 36c5402..04b5b3f 100644 --- a/app/models/spree/slide_location.rb +++ b/app/models/spree/slide_location.rb @@ -1,10 +1,10 @@ class Spree::SlideLocation < ActiveRecord::Base - has_and_belongs_to_many :slides, + ->(model) { where('spree_slides.id != ?', model.fallback_slide_id || 0) }, class_name: 'Spree::Slide', join_table: 'spree_slide_slide_locations' + belongs_to :fallback_slide, class_name: 'Spree::Slide', foreign_key: 'fallback_slide_id' validates :name, presence: true - end diff --git a/spec/models/spree/slide_location_spec.rb b/spec/models/spree/slide_location_spec.rb new file mode 100644 index 0000000..608434a --- /dev/null +++ b/spec/models/spree/slide_location_spec.rb @@ -0,0 +1,19 @@ +require 'spec_helper' + +RSpec.describe Spree::SlideLocation do + let(:slide1) { Spree::Slide.create } + let(:slide2) { Spree::Slide.create } + + describe '#slides' do + before do + Spree::SlideLocation.create name: 'Test location', + slides: [slide1, slide2], + fallback_slide: slide1 + end + + it 'doesnt include its fallback slide' do + location = Spree::SlideLocation.find_by! name: 'Test location' + expect(location.slides.map(&:id)).to contain_exactly slide2.id + end + end +end From cc78a0d17eddfb37d11c3936d61a1ebbfd7b4e94 Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Mon, 11 Sep 2017 14:32:23 -0500 Subject: [PATCH 10/11] order fallback slides by name --- app/views/spree/admin/slide_locations/_form.html.erb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/views/spree/admin/slide_locations/_form.html.erb b/app/views/spree/admin/slide_locations/_form.html.erb index 53980c1..b6daef6 100644 --- a/app/views/spree/admin/slide_locations/_form.html.erb +++ b/app/views/spree/admin/slide_locations/_form.html.erb @@ -9,7 +9,7 @@ <%= f.field_container :fallback_slide_id do %> <%= f.label :fallback_slide_id, 'Fallback slide' %>
This slide will be shown only when no other slide can be shown.
- <%= f.collection_select(:fallback_slide_id, Spree::Slide.all, :id, :name, { include_blank: Spree.t('match_choices.none') }, { class: 'select2', disabled: (cannot? :edit, Spree::SlideLocation) }) %> + <%= f.collection_select(:fallback_slide_id, Spree::Slide.all.order(:name), :id, :name, { include_blank: Spree.t('match_choices.none') }, { class: 'select2', disabled: (cannot? :edit, Spree::SlideLocation) }) %> <% end %> From fcd9e2b1def6da2d47a74427820d4dbe8f07ca7b Mon Sep 17 00:00:00 2001 From: Kevin Perez Date: Thu, 30 Nov 2017 13:57:52 -0600 Subject: [PATCH 11/11] create dummy app for testing in Travis environment --- .travis.yml | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/.travis.yml b/.travis.yml index b11346b..4a53bc3 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,6 +12,12 @@ gemfile: - gemfiles/spree_3_2.gemfile - gemfiles/spree_master.gemfile +before_script: + - mkdir ../temp; cd ../temp + - bundle exec rails _5.1.1_ plugin new spree_slider --mountable --dummy-path=spec/dummy --skip-test-unit + - mv spree_slider/spec/dummy ../spec/ + - cd ../build; rm -rf ../temp + script: - bundle exec rspec spec