From b8fbd6347b676965bf1e2e2df1e474fa81fb28bf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javi=20Mart=C3=ADn?= Date: Thu, 31 Oct 2019 20:27:20 +0100 Subject: [PATCH] Use acts_as_taggable for investment valuation tags We were manually doing the same thing, generating inconsistent results, since the method `valuation_tag_list` was using the `valuation` context, when actually the expected behavior would be to use the `valuation_tag` context. --- .../admin/budget_investments_controller.rb | 2 +- app/helpers/budgets_helper.rb | 2 +- app/models/budget/investment.rb | 9 +------- app/models/tagging.rb | 2 ++ .../admin/budget_investments/show.html.erb | 2 +- lib/tasks/consul.rake | 3 ++- lib/tasks/migrations.rake | 7 ++++++ spec/factories/classifications.rb | 2 +- .../features/admin/budget_investments_spec.rb | 14 ++++++------ spec/features/tags/budget_investments_spec.rb | 2 +- spec/lib/tasks/migrations_spec.rb | 22 +++++++++++++++++++ 11 files changed, 46 insertions(+), 21 deletions(-) create mode 100644 app/models/tagging.rb create mode 100644 lib/tasks/migrations.rake create mode 100644 spec/lib/tasks/migrations_spec.rb diff --git a/app/controllers/admin/budget_investments_controller.rb b/app/controllers/admin/budget_investments_controller.rb index 0d322920b..bb71ef946 100644 --- a/app/controllers/admin/budget_investments_controller.rb +++ b/app/controllers/admin/budget_investments_controller.rb @@ -110,7 +110,7 @@ class Admin::BudgetInvestmentsController < Admin::BaseController end def load_tags - @tags = Budget::Investment.tags_on(:valuation).order(:name).distinct + @tags = Budget::Investment.tags_on(:valuation_tags).order(:name).distinct end def load_ballot diff --git a/app/helpers/budgets_helper.rb b/app/helpers/budgets_helper.rb index 8f46b1225..b0fe98524 100644 --- a/app/helpers/budgets_helper.rb +++ b/app/helpers/budgets_helper.rb @@ -54,7 +54,7 @@ module BudgetsHelper end def investment_tags_select_options(budget) - tags = budget.investments.tags_on(:valuation).order(:name).pluck(:name) + tags = budget.investments.tags_on(:valuation_tags).order(:name).pluck(:name) tags = tags.concat budget.budget_valuation_tags.split(",") if budget.budget_valuation_tags.present? tags.uniq end diff --git a/app/models/budget/investment.rb b/app/models/budget/investment.rb index 2ee462a0f..c0beb4b68 100644 --- a/app/models/budget/investment.rb +++ b/app/models/budget/investment.rb @@ -16,6 +16,7 @@ class Budget include Mappable include Documentable + acts_as_taggable_on :valuation_tags acts_as_votable acts_as_paranoid column: :hidden_at include ActsAsParanoidAliases @@ -383,14 +384,6 @@ class Budget self.valuator_groups.map(&:name).compact.join(", ").presence end - def valuation_tag_list - tag_list_on(:valuation) - end - - def valuation_tag_list=(tags) - set_tag_list_on(:valuation, tags) - end - def self.with_milestone_status_id(status_id) includes(milestones: :translations).select do |investment| investment.milestone_status_id == status_id.to_i diff --git a/app/models/tagging.rb b/app/models/tagging.rb new file mode 100644 index 000000000..50e0fa9ad --- /dev/null +++ b/app/models/tagging.rb @@ -0,0 +1,2 @@ +class Tagging < ActsAsTaggableOn::Tagging +end diff --git a/app/views/admin/budget_investments/show.html.erb b/app/views/admin/budget_investments/show.html.erb index f9fa144e9..b2584c35a 100644 --- a/app/views/admin/budget_investments/show.html.erb +++ b/app/views/admin/budget_investments/show.html.erb @@ -29,7 +29,7 @@

<%= t("admin.budget_investments.show.tags") %>: - <%= @investment.tags_on(:valuation).pluck(:name).sort.join(", ") %> + <%= @investment.valuation_tags.pluck(:name).sort.join(", ") %>

diff --git a/lib/tasks/consul.rake b/lib/tasks/consul.rake index 1763575e5..03c510743 100644 --- a/lib/tasks/consul.rake +++ b/lib/tasks/consul.rake @@ -4,6 +4,7 @@ namespace :consul do desc "Runs tasks needed to upgrade from 1.0.0 to 1.1.0" task "execute_release_1.1.0_tasks": [ - "budgets:set_original_heading_id" + "budgets:set_original_heading_id", + "migrations:valuation_taggings" ] end diff --git a/lib/tasks/migrations.rake b/lib/tasks/migrations.rake new file mode 100644 index 000000000..d9b829e86 --- /dev/null +++ b/lib/tasks/migrations.rake @@ -0,0 +1,7 @@ +namespace :migrations do + desc "Migrates context of valuation taggings" + task valuation_taggings: :environment do + ApplicationLogger.new.info "Updating valuation taggings context" + Tagging.where(context: "valuation").update_all(context: "valuation_tags") + end +end diff --git a/spec/factories/classifications.rb b/spec/factories/classifications.rb index d75d90d3e..d1f107814 100644 --- a/spec/factories/classifications.rb +++ b/spec/factories/classifications.rb @@ -19,7 +19,7 @@ FactoryBot.define do end end - factory :tagging, class: "ActsAsTaggableOn::Tagging" do + factory :tagging do context { "tags" } association :taggable, factory: :proposal tag diff --git a/spec/features/admin/budget_investments_spec.rb b/spec/features/admin/budget_investments_spec.rb index 9345919a5..a2f2ad1ef 100644 --- a/spec/features/admin/budget_investments_spec.rb +++ b/spec/features/admin/budget_investments_spec.rb @@ -510,8 +510,8 @@ describe "Admin budget investments" do investment1 = create(:budget_investment, budget: budget, tag_list: "Education") investment2 = create(:budget_investment, budget: budget, tag_list: "Health") - investment1.set_tag_list_on(:valuation, "Teachers") - investment2.set_tag_list_on(:valuation, "Hospitals") + investment1.set_tag_list_on(:valuation_tags, "Teachers") + investment2.set_tag_list_on(:valuation_tags, "Hospitals") investment1.save! investment2.save! @@ -526,8 +526,8 @@ describe "Admin budget investments" do investment1 = create(:budget_investment, budget: budget, tag_list: "Roads") investment2 = create(:budget_investment, budget: new_budget, tag_list: "Accessibility") - investment1.set_tag_list_on(:valuation, "Roads") - investment2.set_tag_list_on(:valuation, "Accessibility") + investment1.set_tag_list_on(:valuation_tags, "Roads") + investment2.set_tag_list_on(:valuation_tags, "Accessibility") investment1.save! investment2.save! @@ -1192,7 +1192,7 @@ describe "Admin budget investments" do scenario "Adds existing valuation tags", :js do budget_investment1 = create(:budget_investment) - budget_investment1.set_tag_list_on(:valuation, "Education, Health") + budget_investment1.set_tag_list_on(:valuation_tags, "Education, Health") budget_investment1.save! budget_investment2 = create(:budget_investment) @@ -1230,7 +1230,7 @@ describe "Admin budget investments" do scenario "Changes valuation and user generated tags" do budget_investment = create(:budget_investment, tag_list: "Park") - budget_investment.set_tag_list_on(:valuation, "Education") + budget_investment.set_tag_list_on(:valuation_tags, "Education") budget_investment.save! visit admin_budget_budget_investment_path(budget_investment.budget, budget_investment) @@ -1671,7 +1671,7 @@ describe "Admin budget investments" do end scenario "Keeps the valuation tags", :js do - investment1.set_tag_list_on(:valuation, %w[Possimpible Truthiness]) + investment1.set_tag_list_on(:valuation_tags, %w[Possimpible Truthiness]) investment1.save! visit admin_budget_budget_investments_path(budget) diff --git a/spec/features/tags/budget_investments_spec.rb b/spec/features/tags/budget_investments_spec.rb index d880e2e70..4e165e8b8 100644 --- a/spec/features/tags/budget_investments_spec.rb +++ b/spec/features/tags/budget_investments_spec.rb @@ -334,7 +334,7 @@ describe "Tags" do scenario "Valuators do not see user tags" do investment = create(:budget_investment, heading: heading, tag_list: "Park") - investment.set_tag_list_on(:valuation, "Education") + investment.set_tag_list_on(:valuation_tags, "Education") investment.save! login_as(admin) diff --git a/spec/lib/tasks/migrations_spec.rb b/spec/lib/tasks/migrations_spec.rb new file mode 100644 index 000000000..ae6da3a54 --- /dev/null +++ b/spec/lib/tasks/migrations_spec.rb @@ -0,0 +1,22 @@ +require "rails_helper" + +describe "Migration tasks" do + let(:run_rake_task) do + Rake::Task["migrations:valuation_taggings"].reenable + Rake.application.invoke_task("migrations:valuation_taggings") + end + + it "updates taggings" do + valuation_tagging = create(:tagging, context: "valuation") + another_valuation_tagging = create(:tagging, context: "valuation") + valuation_tags_tagging = create(:tagging, context: "valuation_tags") + tags_tagging = create(:tagging) + + run_rake_task + + expect(valuation_tagging.reload.context).to eq "valuation_tags" + expect(another_valuation_tagging.reload.context).to eq "valuation_tags" + expect(valuation_tags_tagging.reload.context).to eq "valuation_tags" + expect(tags_tagging.reload.context).to eq "tags" + end +end