From 2eab6a476ea8ec9cedf199147f29b68308dc18eb Mon Sep 17 00:00:00 2001 From: Bertocq Date: Sun, 4 Feb 2018 21:41:41 +0100 Subject: [PATCH 1/5] Refactor sluggable concern spec --- spec/models/concerns/sluggable.rb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/spec/models/concerns/sluggable.rb b/spec/models/concerns/sluggable.rb index bfe76af1b..c319d55d8 100644 --- a/spec/models/concerns/sluggable.rb +++ b/spec/models/concerns/sluggable.rb @@ -3,13 +3,12 @@ require 'spec_helper' shared_examples_for 'sluggable' do describe 'generate_slug' do - before do - create(described_class.name.parameterize.tr('-', '_').to_sym, name: "Marlo Brañido Carlo") - end + let(:factory_name) { described_class.name.parameterize('_').to_sym } + let(:sluggable) { create(factory_name, name: "Marló Brañido Carlo") } context "when a new sluggable is created" do it "gets a slug string" do - expect(described_class.last.slug).to eq("marlo-branido-carlo") + expect(sluggable.slug).to eq("marlo-branido-carlo") end end end From 8f7297234490df70d15698764c0a52c08dca1a81 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Sun, 4 Feb 2018 22:37:45 +0100 Subject: [PATCH 2/5] Change budget factory name to avoid collisions --- spec/factories.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/factories.rb b/spec/factories.rb index 8d299bcd8..f6d121f50 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -220,7 +220,7 @@ FactoryBot.define do end factory :budget do - sequence(:name) { |n| "Budget #{n}" } + sequence(:name) { |n| "#{Faker::Lorem.word} #{n}" } currency_symbol "€" phase 'accepting' description_drafting "This budget is drafting" From 8e6e360fc8086f208cabe7e870608ee327e2ff64 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Sun, 4 Feb 2018 22:38:23 +0100 Subject: [PATCH 3/5] Create traits for budget group & heading with drafting budget --- spec/factories.rb | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/spec/factories.rb b/spec/factories.rb index f6d121f50..73512a689 100644 --- a/spec/factories.rb +++ b/spec/factories.rb @@ -278,6 +278,10 @@ FactoryBot.define do factory :budget_group, class: 'Budget::Group' do budget sequence(:name) { |n| "Group #{n}" } + + trait :drafting_budget do + association :budget, factory: [:budget, :drafting] + end end factory :budget_heading, class: 'Budget::Heading' do @@ -285,6 +289,10 @@ FactoryBot.define do sequence(:name) { |n| "Heading #{n}" } price 1000000 population 1234 + + trait :drafting_budget do + association :group, factory: [:budget_group, :drafting_budget] + end end factory :budget_investment, class: 'Budget::Investment' do From 198ff0cd1f7e2e5e68b7a810166b0b8fbdff5a71 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Sun, 4 Feb 2018 22:39:26 +0100 Subject: [PATCH 4/5] Use updatable slug factory trait to sluggable concern Slugs should only be updated on certain conditions, we need a trait that meets that conditions and the name of the trait passed as a mandatory & named argument on the sluggable concern --- spec/models/budget/group_spec.rb | 2 +- spec/models/budget/heading_spec.rb | 2 +- spec/models/budget_spec.rb | 2 +- spec/models/concerns/sluggable.rb | 17 ++++++++++++++++- 4 files changed, 19 insertions(+), 4 deletions(-) diff --git a/spec/models/budget/group_spec.rb b/spec/models/budget/group_spec.rb index 099700c4e..27392e81f 100644 --- a/spec/models/budget/group_spec.rb +++ b/spec/models/budget/group_spec.rb @@ -4,7 +4,7 @@ describe Budget::Group do let(:budget) { create(:budget) } - it_behaves_like "sluggable" + it_behaves_like "sluggable", updatable_slug_trait: :drafting_budget describe "name" do before do diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb index 655a77b4f..b2a09cb55 100644 --- a/spec/models/budget/heading_spec.rb +++ b/spec/models/budget/heading_spec.rb @@ -5,7 +5,7 @@ describe Budget::Heading do let(:budget) { create(:budget) } let(:group) { create(:budget_group, budget: budget) } - it_behaves_like "sluggable" + it_behaves_like "sluggable", updatable_slug_trait: :drafting_budget describe "name" do before do diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index c41182bdd..80f0a0c94 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -4,7 +4,7 @@ describe Budget do let(:budget) { create(:budget) } - it_behaves_like "sluggable" + it_behaves_like "sluggable", updatable_slug_trait: :drafting describe "name" do before do diff --git a/spec/models/concerns/sluggable.rb b/spec/models/concerns/sluggable.rb index c319d55d8..1b5038d71 100644 --- a/spec/models/concerns/sluggable.rb +++ b/spec/models/concerns/sluggable.rb @@ -1,6 +1,6 @@ require 'spec_helper' -shared_examples_for 'sluggable' do +shared_examples_for 'sluggable' do |updatable_slug_trait:| describe 'generate_slug' do let(:factory_name) { described_class.name.parameterize('_').to_sym } @@ -11,5 +11,20 @@ shared_examples_for 'sluggable' do expect(sluggable.slug).to eq("marlo-branido-carlo") end end + + context "slug updating condition is true" do + it "slug is updated" do + updatable = create(factory_name, updatable_slug_trait, name: 'Old Name') + expect{updatable.update_attributes(name: 'New Name')} + .to change{ updatable.slug }.from('old-name').to('new-name') + end + end + + context "slug updating condition is false" do + it "slug isn't updated" do + expect{sluggable.update_attributes(name: 'New Name')} + .not_to (change{ sluggable.slug }) + end + end end end From e0871e5dc6cbbe792b1ce29e381aceb6a3f5a47f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Sun, 4 Feb 2018 22:41:47 +0100 Subject: [PATCH 5/5] Only update slug if empty or model condition is met --- app/models/budget.rb | 4 ++++ app/models/budget/group.rb | 6 ++++++ app/models/budget/heading.rb | 6 ++++++ app/models/concerns/sluggable.rb | 2 +- 4 files changed, 17 insertions(+), 1 deletion(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index 28f37f4bc..9b55dc4a6 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -180,6 +180,10 @@ class Budget < ActiveRecord::Base ) end end + + def generate_slug? + slug.nil? || drafting? + end end diff --git a/app/models/budget/group.rb b/app/models/budget/group.rb index a2b3179a3..dd96cdb95 100644 --- a/app/models/budget/group.rb +++ b/app/models/budget/group.rb @@ -13,5 +13,11 @@ class Budget def single_heading_group? headings.count == 1 end + + private + + def generate_slug? + slug.nil? || budget.drafting? + end end end diff --git a/app/models/budget/heading.rb b/app/models/budget/heading.rb index 8816d1abf..4818eaeed 100644 --- a/app/models/budget/heading.rb +++ b/app/models/budget/heading.rb @@ -28,5 +28,11 @@ class Budget investments.empty? end + private + + def generate_slug? + slug.nil? || budget.drafting? + end + end end diff --git a/app/models/concerns/sluggable.rb b/app/models/concerns/sluggable.rb index 01e27489d..495ffaf77 100644 --- a/app/models/concerns/sluggable.rb +++ b/app/models/concerns/sluggable.rb @@ -2,7 +2,7 @@ module Sluggable extend ActiveSupport::Concern included do - before_validation :generate_slug + before_validation :generate_slug, if: :generate_slug? end def generate_slug