From f048df8dd824e65be140d18c0f5db4007cb36d58 Mon Sep 17 00:00:00 2001 From: taitus Date: Sat, 23 Jan 2021 13:45:36 +0100 Subject: [PATCH 1/2] Add relation between Goal and LocalTarget --- app/models/sdg/goal.rb | 1 + app/models/sdg/local_target.rb | 10 ++++++++-- db/dev_seeds/sdg.rb | 3 ++- .../20210123100638_add_goals_to_local_targets.rb | 5 +++++ db/schema.rb | 4 +++- spec/factories/sdg.rb | 1 + spec/models/sdg/local_target_spec.rb | 11 ++++++++++- 7 files changed, 30 insertions(+), 5 deletions(-) create mode 100644 db/migrate/20210123100638_add_goals_to_local_targets.rb diff --git a/app/models/sdg/goal.rb b/app/models/sdg/goal.rb index c342dde35204..167dc2bf4ef5 100644 --- a/app/models/sdg/goal.rb +++ b/app/models/sdg/goal.rb @@ -4,6 +4,7 @@ class SDG::Goal < ApplicationRecord validates :code, presence: true, uniqueness: true, inclusion: { in: 1..17 } has_many :targets, dependent: :destroy + has_many :local_targets, dependent: :destroy def title I18n.t("sdg.goals.goal_#{code}.title") diff --git a/app/models/sdg/local_target.rb b/app/models/sdg/local_target.rb index 6711519d1f99..1bc9a9a0e5a1 100644 --- a/app/models/sdg/local_target.rb +++ b/app/models/sdg/local_target.rb @@ -2,8 +2,6 @@ class SDG::LocalTarget < ApplicationRecord include Comparable include SDG::Related - delegate :goal, to: :target - translates :title, touch: true translates :description, touch: true include Globalizable @@ -14,8 +12,12 @@ class SDG::LocalTarget < ApplicationRecord validates :code, presence: true, uniqueness: true, format: ->(local_target) { /\A#{local_target.target&.code}\.\d+/ } validates :target, presence: true + validates :goal, presence: true belongs_to :target + belongs_to :goal + + before_validation :set_related_goal def self.[](code) find_by!(code: code) @@ -40,4 +42,8 @@ def numeric_subcode def subcode code.split(".").last end + + def set_related_goal + self.goal ||= target&.goal + end end diff --git a/db/dev_seeds/sdg.rb b/db/dev_seeds/sdg.rb index f7fe40a3a885..1dd5320de457 100644 --- a/db/dev_seeds/sdg.rb +++ b/db/dev_seeds/sdg.rb @@ -8,7 +8,8 @@ local_target = SDG::LocalTarget.create!(code: "#{target.code}.#{n + 1}", title: title, description: description, - target: target) + target: target, + goal: target.goal) random_locales.map do |locale| Globalize.with_locale(locale) do local_target.title = "Title for locale #{locale}" diff --git a/db/migrate/20210123100638_add_goals_to_local_targets.rb b/db/migrate/20210123100638_add_goals_to_local_targets.rb new file mode 100644 index 000000000000..b74846f3e685 --- /dev/null +++ b/db/migrate/20210123100638_add_goals_to_local_targets.rb @@ -0,0 +1,5 @@ +class AddGoalsToLocalTargets < ActiveRecord::Migration[5.2] + def change + add_reference :sdg_local_targets, :goal + end +end diff --git a/db/schema.rb b/db/schema.rb index c06db2e50367..25e23d4cbbd3 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 2021_01_07_125458) do +ActiveRecord::Schema.define(version: 2021_01_23_100638) do # These are extensions that must be enabled in order to support this database enable_extension "pg_trgm" @@ -1322,7 +1322,9 @@ t.string "code" t.datetime "created_at", null: false t.datetime "updated_at", null: false + t.bigint "goal_id" t.index ["code"], name: "index_sdg_local_targets_on_code", unique: true + t.index ["goal_id"], name: "index_sdg_local_targets_on_goal_id" t.index ["target_id"], name: "index_sdg_local_targets_on_target_id" end diff --git a/spec/factories/sdg.rb b/spec/factories/sdg.rb index 42073cf6782f..1f933eb0480b 100644 --- a/spec/factories/sdg.rb +++ b/spec/factories/sdg.rb @@ -13,6 +13,7 @@ sequence(:description) { |n| "Help for Local Target #{n}" } target { SDG::Target[code.rpartition(".").first] } + goal { SDG::Goal[code.split(".")[0]] } end factory :sdg_phase, class: "SDG::Phase" do diff --git a/spec/models/sdg/local_target_spec.rb b/spec/models/sdg/local_target_spec.rb index 6bea1113974d..bf2496728fbe 100644 --- a/spec/models/sdg/local_target_spec.rb +++ b/spec/models/sdg/local_target_spec.rb @@ -18,7 +18,7 @@ end it "is not valid without a code" do - expect(build(:sdg_local_target, code: nil, target: SDG::Target[1.1])).not_to be_valid + expect(build(:sdg_local_target, code: nil, target: SDG::Target[1.1], goal: SDG::Goal[1])).not_to be_valid end it "is not valid when code does not include associated target code" do @@ -47,6 +47,15 @@ expect(build(:sdg_local_target, target: nil)).not_to be_valid end + describe "#set_related_goal" do + it "before validation set related goal" do + local_target = build(:sdg_local_target, code: "1.1.1", goal: nil) + + expect(local_target).to be_valid + expect(local_target.goal).to eq(SDG::Goal[1]) + end + end + describe "#goal" do it "returns the target goal" do local_target = create(:sdg_local_target, code: "1.1.1") From bc3ce8d12dd86c1b5f74ff71396a7f40be99c68e Mon Sep 17 00:00:00 2001 From: taitus Date: Sat, 23 Jan 2021 14:01:20 +0100 Subject: [PATCH 2/2] Allow add local targets to RelatedListSelectorComponent --- .../sdg/related_list_selector_component.rb | 3 +- app/models/concerns/sdg/relatable.rb | 13 +++--- app/models/sdg/goal.rb | 9 ++++ app/models/sdg/local_target.rb | 10 ++--- app/models/sdg/target.rb | 12 +++--- .../related_list_selector_component_spec.rb | 16 ++++++- spec/models/sdg/goal_spec.rb | 28 +++++++++++++ spec/models/sdg/local_target_spec.rb | 16 +++++++ spec/models/sdg/relatable_spec.rb | 29 ++++++++++--- spec/models/sdg/target_spec.rb | 16 +++++++ spec/system/sdg_management/relations_spec.rb | 42 +++++++++++++------ 11 files changed, 157 insertions(+), 37 deletions(-) diff --git a/app/components/sdg/related_list_selector_component.rb b/app/components/sdg/related_list_selector_component.rb index b183765ded5f..4f32adf5a464 100644 --- a/app/components/sdg/related_list_selector_component.rb +++ b/app/components/sdg/related_list_selector_component.rb @@ -15,7 +15,8 @@ def sdg_related_suggestions def goals_and_targets goals.map do |goal| - [goal, *goal.targets.sort] + global_and_local_targets = goal.targets + goal.local_targets + [goal, global_and_local_targets.sort] end.flatten end diff --git a/app/models/concerns/sdg/relatable.rb b/app/models/concerns/sdg/relatable.rb index 636feca73958..1b1b8d4a4956 100644 --- a/app/models/concerns/sdg/relatable.rb +++ b/app/models/concerns/sdg/relatable.rb @@ -74,19 +74,20 @@ def sdg_target_list end def sdg_related_list - sdg_goals.order(:code).map do |goal| - [goal, sdg_global_targets.where(goal: goal).sort] - end.flatten.map(&:code).join(", ") + related_sdgs.sort.map(&:code).join(", ") end def sdg_related_list=(codes) target_codes, goal_codes = codes.tr(" ", "").split(",").partition { |code| code.include?(".") } - targets = target_codes.map { |code| SDG::Target[code] } + local_targets_codes, global_targets_codes = target_codes.partition { |code| code.split(".")[2] } + global_targets = global_targets_codes.map { |code| SDG::Target[code] } + local_targets = local_targets_codes.map { |code| SDG::LocalTarget[code] } goals = goal_codes.map { |code| SDG::Goal[code] } transaction do - self.sdg_global_targets = targets - self.sdg_goals = (targets.map(&:goal) + goals).uniq + self.sdg_local_targets = local_targets + self.sdg_global_targets = global_targets + self.sdg_goals = (global_targets.map(&:goal) + local_targets.map(&:goal) + goals).uniq end end end diff --git a/app/models/sdg/goal.rb b/app/models/sdg/goal.rb index 167dc2bf4ef5..3e0b8700751f 100644 --- a/app/models/sdg/goal.rb +++ b/app/models/sdg/goal.rb @@ -1,4 +1,5 @@ class SDG::Goal < ApplicationRecord + include Comparable include SDG::Related validates :code, presence: true, uniqueness: true, inclusion: { in: 1..17 } @@ -18,6 +19,14 @@ def description I18n.t("sdg.goals.goal_#{code}.description") end + def <=>(goal_or_target) + if goal_or_target.class == self.class + code <=> goal_or_target.code + elsif goal_or_target.respond_to?(:goal) + [self, -1] <=> [goal_or_target.goal, 1] + end + end + def self.[](code) find_by!(code: code) end diff --git a/app/models/sdg/local_target.rb b/app/models/sdg/local_target.rb index 1bc9a9a0e5a1..6e07d48950f0 100644 --- a/app/models/sdg/local_target.rb +++ b/app/models/sdg/local_target.rb @@ -23,11 +23,11 @@ def self.[](code) find_by!(code: code) end - def <=>(any_target) - if any_target.class == self.class - [target, numeric_subcode] <=> [any_target.target, any_target.numeric_subcode] - elsif any_target.class == target.class - -1 * (any_target <=> self) + def <=>(goal_or_target) + if goal_or_target.class == self.class + [target, numeric_subcode] <=> [goal_or_target.target, goal_or_target.numeric_subcode] + elsif [target.class, goal.class].include?(goal_or_target.class) + -1 * (goal_or_target <=> self) end end diff --git a/app/models/sdg/target.rb b/app/models/sdg/target.rb index ee1e96a45f42..e20536e1b969 100644 --- a/app/models/sdg/target.rb +++ b/app/models/sdg/target.rb @@ -12,11 +12,13 @@ def title I18n.t("sdg.goals.goal_#{goal.code}.targets.target_#{code_key}.title") end - def <=>(any_target) - if any_target.class == self.class - [goal.code, numeric_subcode] <=> [any_target.goal.code, any_target.numeric_subcode] - elsif any_target.class.name == "SDG::LocalTarget" - [self, -1] <=> [any_target.target, 1] + def <=>(goal_or_target) + if goal_or_target.class == self.class + [goal.code, numeric_subcode] <=> [goal_or_target.goal.code, goal_or_target.numeric_subcode] + elsif goal_or_target.class == goal.class + -1 * (goal_or_target <=> self) + elsif goal_or_target.class.name == "SDG::LocalTarget" + [self, -1] <=> [goal_or_target.target, 1] end end diff --git a/spec/components/sdg/related_list_selector_component_spec.rb b/spec/components/sdg/related_list_selector_component_spec.rb index 7326400e01f1..38a2fe74ea47 100644 --- a/spec/components/sdg/related_list_selector_component_spec.rb +++ b/spec/components/sdg/related_list_selector_component_spec.rb @@ -36,10 +36,12 @@ describe "#goals_and_targets" do it "return all goals and target with order" do + create(:sdg_local_target, code: "1.1.1") goals_and_targets = component.goals_and_targets expect(goals_and_targets.first).to eq SDG::Goal[1] expect(goals_and_targets.second).to eq SDG::Target[1.1] + expect(goals_and_targets.third).to eq SDG::LocalTarget["1.1.1"] expect(goals_and_targets.last).to eq SDG::Target[17.19] end end @@ -56,7 +58,7 @@ }) end - it "return suggestion tag for target" do + it "returns suggestion tag for global target" do suggestion = component.suggestion_tag_for(SDG::Target[1.1]) expect(suggestion).to eq({ @@ -66,5 +68,17 @@ value: "1.1" }) end + + it "returns suggestion tag for local target" do + create(:sdg_local_target, code: "1.1.1", title: "By 2030, eradicate extreme custom text") + suggestion = component.suggestion_tag_for(SDG::LocalTarget["1.1.1"]) + + expect(suggestion).to eq({ + tag: "1.1.1. By 2030 eradicate extreme custom text", + display_text: "1.1.1", + title: "By 2030, eradicate extreme custom text", + value: "1.1.1" + }) + end end end diff --git a/spec/models/sdg/goal_spec.rb b/spec/models/sdg/goal_spec.rb index 94d20710bf40..ac53fc049602 100644 --- a/spec/models/sdg/goal_spec.rb +++ b/spec/models/sdg/goal_spec.rb @@ -21,6 +21,34 @@ end end + describe "#<=>" do + let(:goal) { SDG::Goal[10] } + + it "can be compared against goals" do + lesser_goal = SDG::Goal[9] + greater_goal = SDG::Goal[11] + + expect(goal).to be > lesser_goal + expect(goal).to be < greater_goal + end + + it "can be compared against global targets" do + lesser_target = build(:sdg_target, code: "9.A", goal: SDG::Goal[9]) + greater_target = build(:sdg_target, code: "11.1", goal: SDG::Goal[11]) + + expect(goal).to be > lesser_target + expect(goal).to be < greater_target + end + + it "can be compared against local and global targets" do + lesser_local_target = build(:sdg_local_target, code: "9.B.12") + greater_target = build(:sdg_target, code: "11.1", goal: SDG::Goal[11]) + + expect(goal).to be > lesser_local_target + expect(goal).to be < greater_target + end + end + describe ".[]" do it "finds existing goals by code" do expect(SDG::Goal[1].code).to be 1 diff --git a/spec/models/sdg/local_target_spec.rb b/spec/models/sdg/local_target_spec.rb index bf2496728fbe..fc615ebb1164 100644 --- a/spec/models/sdg/local_target_spec.rb +++ b/spec/models/sdg/local_target_spec.rb @@ -90,6 +90,22 @@ expect(local_target).to be > lesser_target expect(local_target).to be < greater_target end + + it "can be compared against goals" do + lesser_goal = build(:sdg_goal, code: "10") + greater_goal = build(:sdg_goal, code: "11") + + expect(local_target).to be > lesser_goal + expect(local_target).to be < greater_goal + end + + it "can be compared against goals and global targets" do + lesser_goal = build(:sdg_goal, code: "10") + greater_target = build(:sdg_target, code: "11.1", goal: SDG::Goal[11]) + + expect(local_target).to be > lesser_goal + expect(local_target).to be < greater_target + end end describe ".[]" do diff --git a/spec/models/sdg/relatable_spec.rb b/spec/models/sdg/relatable_spec.rb index 4dd6aea511f4..4df51eadce99 100644 --- a/spec/models/sdg/relatable_spec.rb +++ b/spec/models/sdg/relatable_spec.rb @@ -102,9 +102,10 @@ describe "#sdg_related_list" do it "orders related list by code" do relatable.sdg_goals = [SDG::Goal[1], SDG::Goal[3], SDG::Goal[2]] - relatable.sdg_targets = [SDG::Target[2.2], SDG::Target[1.2], SDG::Target[2.1]] + local_targets = %w[2.2.2 2.2.1 3.1.1].map { |code| create(:sdg_local_target, code: code) } + relatable.sdg_targets = [SDG::Target[2.2], SDG::Target[1.2], SDG::Target[2.1]] + local_targets - expect(relatable.sdg_related_list).to eq "1, 1.2, 2, 2.1, 2.2, 3" + expect(relatable.sdg_related_list).to eq "1, 1.2, 2, 2.1, 2.2, 2.2.1, 2.2.2, 3, 3.1.1" end end @@ -133,6 +134,13 @@ expect(relatable.reload.sdg_targets).to match_array [SDG::Target[1.1]] end + it "assigns a single local target" do + relatable.sdg_related_list = local_target.code + + expect(relatable.reload.sdg_goals).to match_array [local_target.goal] + expect(relatable.reload.sdg_local_targets).to match_array [local_target] + end + it "assigns multiple targets" do relatable.sdg_related_list = "1.1,2.3" @@ -146,6 +154,14 @@ expect(relatable.reload.sdg_goals).to match_array [SDG::Goal[1], SDG::Goal[2], SDG::Goal[3]] end + it "assigns a multiple local targets" do + relatable.sdg_related_list = "#{local_target.code}, #{another_local_target.code}" + + expect(relatable.reload.sdg_goals).to match_array [local_target.goal, another_local_target.goal] + expect(relatable.reload.sdg_local_targets).to match_array [local_target, another_local_target] + end + + it "ignores trailing spaces and spaces between commas" do relatable.sdg_related_list = " 1.1, 2.3 " @@ -153,11 +169,12 @@ expect(relatable.reload.sdg_targets).to match_array [SDG::Target[1.1], SDG::Target[2.3]] end - it "assigns goals and targets" do - relatable.sdg_related_list = "1.1,3,4,4.1" + it "assigns goals, targets and local_targets" do + relatable.sdg_related_list = "1.1,3,4,4.1,#{another_local_target.code}" - expect(relatable.reload.sdg_goals).to match_array [SDG::Goal[1], SDG::Goal[3], SDG::Goal[4]] - expect(relatable.reload.sdg_targets).to match_array [SDG::Target[1.1], SDG::Target[4.1]] + expect(relatable.reload.sdg_goals).to match_array [SDG::Goal[1], another_local_target.goal, SDG::Goal[3], SDG::Goal[4]] + expect(relatable.reload.sdg_global_targets).to match_array [SDG::Target[1.1], SDG::Target[4.1]] + expect(relatable.reload.sdg_local_targets).to match_array [another_local_target] end end diff --git a/spec/models/sdg/target_spec.rb b/spec/models/sdg/target_spec.rb index d227d9954582..acff451de07f 100644 --- a/spec/models/sdg/target_spec.rb +++ b/spec/models/sdg/target_spec.rb @@ -87,6 +87,22 @@ expect(target).to be < local_target end end + + it "can be compared against goals" do + lesser_goal = build(:sdg_goal, code: "9") + greater_goal = build(:sdg_goal, code: "11") + + expect(target).to be > lesser_goal + expect(target).to be < greater_goal + end + + it "can be compared against goals and global targets" do + lesser_goal = build(:sdg_goal, code: "9") + greater_target = build(:sdg_target, code: "11.1", goal: SDG::Goal[11]) + + expect(target).to be > lesser_goal + expect(target).to be < greater_target + end end describe ".[]" do diff --git a/spec/system/sdg_management/relations_spec.rb b/spec/system/sdg_management/relations_spec.rb index 6469df2770a3..8178c2157631 100644 --- a/spec/system/sdg_management/relations_spec.rb +++ b/spec/system/sdg_management/relations_spec.rb @@ -204,14 +204,17 @@ end describe "Edit" do - scenario "allows adding the goals and targets and marks the resource as reviewed" do + + scenario "allows adding the goals, global targets and local targets and marks the resource as reviewed" do process = create(:legislation_process, title: "SDG process") - process.sdg_goals = [SDG::Goal[3]] - process.sdg_targets = [SDG::Target[3.3]] + create(:sdg_local_target, code: "1.1.1") + create(:sdg_local_target, code: "3.3.3") + process.sdg_goals = [SDG::Goal[3], SDG::Goal[4]] + process.sdg_targets = [SDG::Target[3.3], SDG::LocalTarget["3.3.3"]] visit sdg_management_edit_legislation_process_path(process) - find(:css, ".sdg-related-list-selector-input").set("1.2, 2,") + find(:css, ".sdg-related-list-selector-input").set("1.2, 2, 1.1.1, ") click_button "Update Process" @@ -220,20 +223,22 @@ click_link "Marked as reviewed" within("tr", text: "SDG process") do - expect(page).to have_css "td", exact_text: "1.2, 3.3" - expect(page).to have_css "td", exact_text: "1, 2, 3" + expect(page).to have_css "td", exact_text: "1.1.1, 1.2, 3.3, 3.3.3" + expect(page).to have_css "td", exact_text: "1, 2, 3, 4" end end - scenario "allows removing the goals and targets" do + scenario "allows removing the goals, global target and local_targets" do process = create(:legislation_process, title: "SDG process") - process.sdg_goals = [SDG::Goal[2], SDG::Goal[3]] - process.sdg_targets = [SDG::Target[2.1], SDG::Target[3.3]] + create(:sdg_local_target, code: "1.1.1") + process.sdg_goals = [SDG::Goal[1], SDG::Goal[2], SDG::Goal[3]] + process.sdg_targets = [SDG::Target[2.1], SDG::Target[3.3], SDG::LocalTarget["1.1.1"]] visit sdg_management_edit_legislation_process_path(process) remove_sdg_goal_or_target_tag(2) remove_sdg_goal_or_target_tag(3.3) + remove_sdg_goal_or_target_tag("1.1.1") click_button "Update Process" @@ -242,7 +247,7 @@ click_link "Marked as reviewed" within("tr", text: "SDG process") do - expect(page).to have_css "td", exact_text: "2, 3" + expect(page).to have_css "td", exact_text: "1, 2, 3" expect(page).to have_css "td", exact_text: "2.1" end end @@ -265,8 +270,9 @@ end end - scenario "allows adding the goals and targets with autocomplete" do + scenario "allows adding the goals, global targets and local targets with autocomplete" do process = create(:legislation_process, title: "SDG process") + create(:sdg_local_target, code: "1.1.1") visit sdg_management_edit_legislation_process_path(process) fill_in "Sustainable Development Goals and Targets", with: "3" @@ -279,12 +285,17 @@ within(".amsify-suggestags-input-area") { expect(page).to have_content "1.1" } + fill_in "Sustainable Development Goals and Targets", with: "1.1.1" + within(".amsify-list") { find(:css, "[data-val='1.1.1']").click } + + within(".amsify-suggestags-input-area") { expect(page).to have_content "1.1.1" } + click_button "Update Process" click_link "Marked as reviewed" within("tr", text: "SDG process") do expect(page).to have_css "td", exact_text: "1, 3" - expect(page).to have_css "td", exact_text: "1.1" + expect(page).to have_css "td", exact_text: "1.1, 1.1.1" end end @@ -338,8 +349,9 @@ scenario "when remove a last tag related to a Goal, the icon will not be checked" do process = create(:legislation_process, title: "SDG process") + create(:sdg_local_target, code: "1.1.1") process.sdg_goals = [SDG::Goal[1]] - process.sdg_targets = [SDG::Target[1.1]] + process.sdg_targets = [SDG::Target[1.1], SDG::LocalTarget["1.1.1"]] visit sdg_management_edit_legislation_process_path(process) remove_sdg_goal_or_target_tag(1) @@ -348,6 +360,10 @@ remove_sdg_goal_or_target_tag(1.1) + expect(find("li[data-code='1']")["aria-checked"]).to eq "true" + + remove_sdg_goal_or_target_tag("1.1.1") + expect(find("li[data-code='1']")["aria-checked"]).to eq "false" end end