From eb9d5b9b84bc106cc2a5eac24b5daca4ddc06ef1 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 12:55:36 +0200 Subject: [PATCH 1/7] Add slug string column to Budget, Budget::Heading and Budget::Group Why: * We're still using id's on routes and other parts of the app, using slugs are more friendly and nice How: * Just a migration with a slug column with string type --- .../20170704105112_add_slugs_to_budget_heading_group.rb | 7 +++++++ db/schema.rb | 5 ++++- 2 files changed, 11 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20170704105112_add_slugs_to_budget_heading_group.rb diff --git a/db/migrate/20170704105112_add_slugs_to_budget_heading_group.rb b/db/migrate/20170704105112_add_slugs_to_budget_heading_group.rb new file mode 100644 index 000000000..85c94e66f --- /dev/null +++ b/db/migrate/20170704105112_add_slugs_to_budget_heading_group.rb @@ -0,0 +1,7 @@ +class AddSlugsToBudgetHeadingGroup < ActiveRecord::Migration + def change + add_column :budgets, :slug, :string + add_column :budget_groups, :slug, :string + add_column :budget_headings, :slug, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index cb9a015e9..0787b5ed1 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: 20170703120055) do +ActiveRecord::Schema.define(version: 20170704105112) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -103,6 +103,7 @@ ActiveRecord::Schema.define(version: 20170703120055) do create_table "budget_groups", force: :cascade do |t| t.integer "budget_id" t.string "name", limit: 50 + t.string "slug" end add_index "budget_groups", ["budget_id"], name: "index_budget_groups_on_budget_id", using: :btree @@ -112,6 +113,7 @@ ActiveRecord::Schema.define(version: 20170703120055) do t.string "name", limit: 50 t.integer "price", limit: 8 t.integer "population" + t.string "slug" end add_index "budget_headings", ["group_id"], name: "index_budget_headings_on_group_id", using: :btree @@ -196,6 +198,7 @@ ActiveRecord::Schema.define(version: 20170703120055) do t.text "description_balloting" t.text "description_reviewing_ballots" t.text "description_finished" + t.string "slug" end create_table "campaigns", force: :cascade do |t| From 148b58f71d92730a89f2323c7346449403e8872f Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 14:13:55 +0200 Subject: [PATCH 2/7] Add sluggable shared example for Sluggable concern Why: * We need certain models that use a Sluggable to generate a slug based on name attribute on save How: * Creating a shared example that knows how to create a factory of the described class * Checking in it that new objects of the described class get the correct slug --- spec/models/concerns/sluggable.rb | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) create mode 100644 spec/models/concerns/sluggable.rb diff --git a/spec/models/concerns/sluggable.rb b/spec/models/concerns/sluggable.rb new file mode 100644 index 000000000..bfe76af1b --- /dev/null +++ b/spec/models/concerns/sluggable.rb @@ -0,0 +1,16 @@ +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 + + context "when a new sluggable is created" do + it "gets a slug string" do + expect(described_class.last.slug).to eq("marlo-branido-carlo") + end + end + end +end From 73e0a5a88ddd77f72a858bfe2069a33c20176fe6 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 14:16:53 +0200 Subject: [PATCH 3/7] Add to Budget Budget::Heading Budget::Group specs the sluggable shared example Why: * Those classes have a slug attribute that needs to be filled How: * Just adding the shared example to Budget model spec, and creating the model spec file for Heading and Group to include it as well --- spec/models/budget/group_spec.rb | 7 +++++++ spec/models/budget/heading_spec.rb | 7 +++++++ spec/models/budget_spec.rb | 2 ++ 3 files changed, 16 insertions(+) create mode 100644 spec/models/budget/group_spec.rb create mode 100644 spec/models/budget/heading_spec.rb diff --git a/spec/models/budget/group_spec.rb b/spec/models/budget/group_spec.rb new file mode 100644 index 000000000..073d05b4d --- /dev/null +++ b/spec/models/budget/group_spec.rb @@ -0,0 +1,7 @@ +require 'rails_helper' + +describe Budget::Group do + + it_behaves_like "sluggable" + +end diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb new file mode 100644 index 000000000..f62bad570 --- /dev/null +++ b/spec/models/budget/heading_spec.rb @@ -0,0 +1,7 @@ +require 'rails_helper' + +describe Budget::Heading do + + it_behaves_like "sluggable" + +end diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index a1d1a1f42..b91fcc6e0 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -2,6 +2,8 @@ require 'rails_helper' describe Budget do + it_behaves_like "sluggable" + describe "description" do it "changes depending on the phase" do budget = create(:budget) From 4535fc9345d0aa568421d98d4f1c46bde9a92ad7 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 14:18:54 +0200 Subject: [PATCH 4/7] Create Sluggable concern, generates slug using name attribute before validation Why: * We need a way to generate a slug for a object given his name attribute value How: * A concern that generates the slug before validation --- app/models/concerns/sluggable.rb | 11 +++++++++++ 1 file changed, 11 insertions(+) create mode 100644 app/models/concerns/sluggable.rb diff --git a/app/models/concerns/sluggable.rb b/app/models/concerns/sluggable.rb new file mode 100644 index 000000000..01e27489d --- /dev/null +++ b/app/models/concerns/sluggable.rb @@ -0,0 +1,11 @@ +module Sluggable + extend ActiveSupport::Concern + + included do + before_validation :generate_slug + end + + def generate_slug + self.slug = name.to_s.parameterize + end +end From e3d89261a6fd3efe9d9d2a1ab94e19a252ace92c Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 14:21:24 +0200 Subject: [PATCH 5/7] Add Sluggable concern and unique validation to Budget, Group and Heading * What: We need to generate slug on Budget, Group and Heading classes, validating its unique for its scope * How: Adding a presence and unique validation using Budget always as scope. --- app/models/budget.rb | 4 +++- app/models/budget/group.rb | 8 ++++++-- app/models/budget/heading.rb | 9 ++++++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/app/models/budget.rb b/app/models/budget.rb index bc7231f29..efb88e62a 100644 --- a/app/models/budget.rb +++ b/app/models/budget.rb @@ -1,13 +1,15 @@ class Budget < ActiveRecord::Base include Measurable + include Sluggable PHASES = %w(accepting reviewing selecting valuating balloting reviewing_ballots finished).freeze CURRENCY_SYMBOLS = %w(€ $ £ ¥).freeze - validates :name, presence: true + validates :name, presence: true, uniqueness: true validates :phase, inclusion: { in: PHASES } validates :currency_symbol, presence: true + validates :slug, presence: true, format: /\A[a-z0-9\-_]+\z/ has_many :investments, dependent: :destroy has_many :ballots, dependent: :destroy diff --git a/app/models/budget/group.rb b/app/models/budget/group.rb index dd7910950..93d7bba63 100644 --- a/app/models/budget/group.rb +++ b/app/models/budget/group.rb @@ -1,10 +1,14 @@ class Budget class Group < ActiveRecord::Base + include Sluggable + belongs_to :budget has_many :headings, dependent: :destroy validates :budget_id, presence: true - validates :name, presence: true + validates :name, presence: true, uniqueness: { scope: :budget } + validates :slug, presence: true, format: /\A[a-z0-9\-_]+\z/ + end -end \ No newline at end of file +end diff --git a/app/models/budget/heading.rb b/app/models/budget/heading.rb index a81308947..1a232c75e 100644 --- a/app/models/budget/heading.rb +++ b/app/models/budget/heading.rb @@ -1,12 +1,15 @@ class Budget class Heading < ActiveRecord::Base + include Sluggable + belongs_to :group has_many :investments validates :group_id, presence: true - validates :name, presence: true + validates :name, presence: true, uniqueness: { if: :name_exists_in_budget_headings } validates :price, presence: true + validates :slug, presence: true, format: /\A[a-z0-9\-_]+\z/ delegate :budget, :budget_id, to: :group, allow_nil: true @@ -16,5 +19,9 @@ class Budget "#{group.name}: #{name}" end + def name_exists_in_budget_headings + group.budget.headings.where(name: name).any? + end + end end From 4068e50b3325001811ac2c1838211aee605d5168 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 14:37:56 +0200 Subject: [PATCH 6/7] Create rake task to generate slug for objects that use Sluggable concern Why: * Once slug presence validation and usage is merged on to a code base, existing objects without it will become invalid and unusable How: * Running `bundle exec slugs:gnerate` will look for all models that include the concern, and generate a slug for each object --- lib/tasks/slugs.rake | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 lib/tasks/slugs.rake diff --git a/lib/tasks/slugs.rake b/lib/tasks/slugs.rake new file mode 100644 index 000000000..2615c5b4c --- /dev/null +++ b/lib/tasks/slugs.rake @@ -0,0 +1,8 @@ +namespace :slugs do + desc "Generate slug attribute for objects from classes that use Sluggable concern" + task generate: :environment do + %w(Budget Budget::Heading Budget::Group).each do |class_name| + class_name.constantize.all.each(&:generate_slug) + end + end +end From 512059e021fad3ffb581009d62ca2fd8651b6518 Mon Sep 17 00:00:00 2001 From: Bertocq Date: Tue, 4 Jul 2017 19:22:15 +0200 Subject: [PATCH 7/7] Increase Budget, Heading and Group model specs to cover slug uniqueness Why: * Slug must be unique among: 1. Budget slug: among other budgets 2. Group slug: among other groups from its budget 3. Heading slug: among other headings from all the groups from its budget How: * Adding checks for all possible scenarios on each model specs --- spec/models/budget/group_spec.rb | 16 ++++++++++++++++ spec/models/budget/heading_spec.rb | 21 +++++++++++++++++++++ spec/models/budget_spec.rb | 10 ++++++++++ 3 files changed, 47 insertions(+) diff --git a/spec/models/budget/group_spec.rb b/spec/models/budget/group_spec.rb index 073d05b4d..4d2570b59 100644 --- a/spec/models/budget/group_spec.rb +++ b/spec/models/budget/group_spec.rb @@ -4,4 +4,20 @@ describe Budget::Group do it_behaves_like "sluggable" + let(:budget) { create(:budget) } + + describe "name" do + before do + create(:budget_group, budget: budget, name: 'object name') + end + + it "can be repeatead in other budget's groups" do + expect(build(:budget_group, budget: create(:budget), name: 'object name')).to be_valid + end + + it "must be unique among all budget's groups" do + expect(build(:budget_group, budget: budget, name: 'object name')).not_to be_valid + end + end + end diff --git a/spec/models/budget/heading_spec.rb b/spec/models/budget/heading_spec.rb index f62bad570..ee2e4c142 100644 --- a/spec/models/budget/heading_spec.rb +++ b/spec/models/budget/heading_spec.rb @@ -4,4 +4,25 @@ describe Budget::Heading do it_behaves_like "sluggable" + let(:budget) { create(:budget) } + let(:group) { create(:budget_group, budget: budget) } + + describe "name" do + before do + create(:budget_heading, group: group, name: 'object name') + end + + it "can be repeatead in other budget's groups" do + expect(build(:budget_heading, group: create(:budget_group), name: 'object name')).to be_valid + end + + it "must be unique among all budget's groups" do + expect(build(:budget_heading, group: create(:budget_group, budget: budget), name: 'object name')).not_to be_valid + end + + it "must be unique among all it's group" do + expect(build(:budget_heading, group: group, name: 'object name')).not_to be_valid + end + end + end diff --git a/spec/models/budget_spec.rb b/spec/models/budget_spec.rb index b91fcc6e0..d3a7574e3 100644 --- a/spec/models/budget_spec.rb +++ b/spec/models/budget_spec.rb @@ -4,6 +4,16 @@ describe Budget do it_behaves_like "sluggable" + describe "name" do + before do + create(:budget, name: 'object name') + end + + it "is validated for uniqueness" do + expect(build(:budget, name: 'object name')).not_to be_valid + end + end + describe "description" do it "changes depending on the phase" do budget = create(:budget)