diff --git a/app/controllers/placements/schools/placements/build_controller.rb b/app/controllers/placements/schools/placements/build_controller.rb index dd6ec74bd..1cdb56628 100644 --- a/app/controllers/placements/schools/placements/build_controller.rb +++ b/app/controllers/placements/schools/placements/build_controller.rb @@ -37,15 +37,10 @@ def update @placement.subject_ids = subject_ids @placement.build_subjects(subject_ids) @placement.build_mentors(mentor_ids) + @placement.save! - if @placement.all_valid? - @placement.save! - - session.delete(:add_a_placement) - redirect_to placements_school_placements_path(school), flash: { success: t(".success") } and return - else - render :check_your_answers and return - end + session.delete(:add_a_placement) + redirect_to placements_school_placements_path(school), flash: { success: t(".success") } and return when :add_phase @placement = build_placement @placement.phase = phase_params @@ -77,8 +72,6 @@ def update else render :add_mentors and return end - else - raise ActionController::RoutingError, "Not Found" end redirect_to public_send(next_step(params[:id])) @@ -117,14 +110,19 @@ def build_or_retrieve_placement end def assign_subjects_based_on_phase - @phase = session.dig(:add_a_placement, "phase").presence || (school.primary_or_secondary_only? ? school.phase : "Primary") + phase = session.dig(:add_a_placement, "phase") + @phase = @placement.build_phase(phase) @subjects = @phase == "Primary" ? Subject.primary : Subject.secondary @placement.build_subjects(subject_ids) @selected_subjects = @placement.subjects end def find_mentors - session.dig(:add_a_placement, "mentor_ids").present? ? school.mentors.find(session.dig(:add_a_placement, "mentor_ids").compact_blank) : [] + if session.dig(:add_a_placement, "mentor_ids").present? + school.mentors.find(session.dig(:add_a_placement, "mentor_ids").compact_blank) + else + [] + end end def initialize_placement @@ -144,7 +142,8 @@ def school def build_placement if session.dig(:add_a_placement, "phase").present? - Placements::Schools::Placements::Build::Placement.new(school:, phase: session.dig(:add_a_placement, "phase")) + Placements::Schools::Placements::Build::Placement.new(school:, + phase: session.dig(:add_a_placement, "phase")) else Placements::Schools::Placements::Build::Placement.new(school:) end diff --git a/app/models/placements/mentor.rb b/app/models/placements/mentor.rb index df84aa3b3..859e010ef 100644 --- a/app/models/placements/mentor.rb +++ b/app/models/placements/mentor.rb @@ -21,8 +21,4 @@ class Placements::Mentor < Mentor has_many :placements, through: :placement_mentor_joins default_scope { joins(:mentor_memberships).distinct } - - def full_name - "#{first_name} #{last_name}" - end end diff --git a/app/models/placements/schools/placements/build/placement.rb b/app/models/placements/schools/placements/build/placement.rb index d67426159..00b3fdf2c 100644 --- a/app/models/placements/schools/placements/build/placement.rb +++ b/app/models/placements/schools/placements/build/placement.rb @@ -14,7 +14,7 @@ class Placements::Schools::Placements::Build::Placement < Placement def valid_phase? return true if phase.present? && [Placements::School::PRIMARY_PHASE, Placements::School::SECONDARY_PHASE].include?(phase) - errors.add(:phase, "Select a phase") + errors.add(:phase, I18n.t("activerecord.errors.models.placements/schools/placements/build/placement.attributes.phase.invalid")) false end @@ -24,7 +24,7 @@ def valid_mentor_ids? if mentor_ids.present? && mentor_ids.all? { |id| Placements::Mentor.exists?(id:) && school.mentors.exists?(id:) } true else - errors.add(:mentor_ids, "Select a mentor or not known") + errors.add(:mentor_ids, I18n.t("activerecord.errors.models.placements/schools/placements/build/placement.attributes.mentor_ids.invalid")) false end end @@ -34,7 +34,7 @@ def valid_subjects? if subject_ids.present? && converted_subject_ids.all? { |id| Subject.exists?(id:) && Subject.find(id).subject_area.downcase == phase.downcase } true else - errors.add(:subject_ids, "Select a subject") + errors.add(:subject_ids, I18n.t("activerecord.errors.models.placements/schools/placements/build/placement.attributes.subject_ids.invalid")) false end end @@ -44,10 +44,8 @@ def all_valid? end def build_subjects(subject_ids = nil) - if subject_ids.instance_of?(String) - subjects << Subject.find(subject_ids) - elsif subject_ids.present? - subject_ids.each { |subject_id| subjects << Subject.find(subject_id) } + if subject_ids.present? + subjects << Subject.where(id: subject_ids) else subjects.build end @@ -55,11 +53,14 @@ def build_subjects(subject_ids = nil) def build_mentors(mentor_ids = nil) if mentor_ids.present? - mentor_ids.compact_blank.each do |mentor_id| - mentors << Placements::Mentor.find(mentor_id) - end + mentors << Placements::Mentor.where(id: mentor_ids.compact_blank) else mentors.build end end + + def build_phase(phase) + phase.presence || + (school.primary_or_secondary_only? ? school.phase : "Primary") + end end diff --git a/config/locales/en/placements/schools/placements.yml b/config/locales/en/placements/schools/placements.yml index 733262b37..24c4ae191 100644 --- a/config/locales/en/placements/schools/placements.yml +++ b/config/locales/en/placements/schools/placements.yml @@ -1,5 +1,16 @@ en: + activerecord: + errors: + models: + placements/schools/placements/build/placement: + attributes: + phase: + invalid: Select a phase + mentor_ids: + invalid: Select a mentor or not known + subject_ids: + invalid: Select a subject placements: schools: placements: diff --git a/spec/models/placements/mentor_spec.rb b/spec/models/placements/mentor_spec.rb index 6440d8248..b7e651aee 100644 --- a/spec/models/placements/mentor_spec.rb +++ b/spec/models/placements/mentor_spec.rb @@ -44,11 +44,4 @@ expect(described_class.all).not_to include(claims_mentor) end end - - describe "#full_name" do - it "returns the mentor's full name" do - mentor = build(:placements_mentor, first_name: "Anne", last_name: "Smith") - expect(mentor.full_name).to eq("Anne Smith") - end - end end diff --git a/spec/models/placements/schools/placements/build/placement_spec.rb b/spec/models/placements/schools/placements/build/placement_spec.rb index 40614ab4c..9ab6dba2b 100644 --- a/spec/models/placements/schools/placements/build/placement_spec.rb +++ b/spec/models/placements/schools/placements/build/placement_spec.rb @@ -102,17 +102,6 @@ end describe "#build_subjects" do - context "when passes a string" do - it "builds subjects" do - placement = described_class.new(school:) - subject = create(:subject) - - placement.build_subjects(subject.id) - - expect(placement.subjects).to eq([subject]) - end - end - context "when passed an array" do it "builds subjects" do placement = described_class.new(school:) @@ -121,7 +110,10 @@ placement.build_subjects([subject_1.id, subject_2.id]) - expect(placement.subjects).to eq([subject_1, subject_2]) + expect(placement.subjects).to match_array([ + have_attributes(subject_1.attributes), + have_attributes(subject_2.attributes), + ]) end end @@ -131,7 +123,9 @@ placement.build_subjects - expect(placement.subjects.first).to have_attributes(Subject.new.attributes) + expect(placement.subjects).to match_array([ + have_attributes(Subject.new.attributes), + ]) end end end @@ -145,7 +139,10 @@ placement.build_mentors([mentor_1.id, mentor_2.id]) - expect(placement.mentors).to eq([mentor_1, mentor_2]) + expect(placement.mentors).to match_array([ + have_attributes(mentor_1.attributes), + have_attributes(mentor_2.attributes), + ]) end end @@ -155,7 +152,37 @@ placement.build_mentors - expect(placement.mentors.first).to have_attributes(Placements::Mentor.new.attributes) + expect(placement.mentors).to match_array([ + have_attributes(Placements::Mentor.new.attributes), + ]) + end + end + end + + describe "#build_phase" do + context "when passed a phase" do + it "returns the phase" do + placement = described_class.new + + expect(placement.build_phase("Primary")).to eq("Primary") + end + end + + context "when phase is blank and the school phase is set" do + it "returns the school's phase" do + allow(school).to receive(:phase).and_return("Primary") + placement = described_class.new(school:) + + expect(placement.build_phase(nil)).to eq(school.phase) + end + end + + context "when phase is blank and the school phase is not set" do + it "returns 'Primary'" do + allow(school).to receive(:phase).and_return(nil) + placement = described_class.new(school:) + + expect(placement.build_phase(nil)).to eq("Primary") end end end diff --git a/spec/system/placements/schools/placements/add_a_placement_spec.rb b/spec/system/placements/schools/placements/add_a_placement_spec.rb index 74f8077d9..95f0372b2 100644 --- a/spec/system/placements/schools/placements/add_a_placement_spec.rb +++ b/spec/system/placements/schools/placements/add_a_placement_spec.rb @@ -3,18 +3,13 @@ RSpec.describe "Placements / Schools / Placements / Add a placement", type: :system, service: :placements do let(:school) { build(:placements_school, name: "School 1", phase: "Primary") } - let(:subject_1) { create(:subject, name: "Primary subject", subject_area: :primary) } - let(:subject_2) { create(:subject, name: "Secondary subject", subject_area: :secondary) } - let(:subject_3) { create(:subject, name: "Secondary subject 2", subject_area: :secondary) } - let(:mentor_1) { create(:placements_mentor_membership, school:).mentor } - let(:mentor_2) { create(:placements_mentor_membership, school:).mentor } + let!(:subject_1) { create(:subject, name: "Primary subject", subject_area: :primary) } + let!(:subject_2) { create(:subject, name: "Secondary subject", subject_area: :secondary) } + let!(:subject_3) { create(:subject, name: "Secondary subject 2", subject_area: :secondary) } + let!(:mentor_1) { create(:placements_mentor_membership, school:).mentor } + let!(:mentor_2) { create(:placements_mentor_membership, school:).mentor } before do - mentor_1 - mentor_2 - subject_1 - subject_2 - subject_3 given_i_sign_in_as_anne end @@ -23,12 +18,26 @@ when_i_visit_the_placements_page and_i_click_on("Add placement") then_i_see_the_add_a_placement_subject_page(school.phase) - when_i_choose_a_subject("Primary subject") + when_i_choose_a_subject(subject_1.name) and_i_click_on("Continue") then_i_see_the_add_a_placement_mentor_page when_i_check_a_mentor(mentor_1.full_name) and_i_click_on("Continue") - then_i_see_the_check_your_answers_page(school.phase) + then_i_see_the_check_your_answers_page(school.phase, mentor_1) + and_i_click_on("Publish placement") + then_i_see_the_placements_page + and_i_see_my_placement(school.phase) + end + + scenario "when I select not known for the mentor" do + when_i_visit_the_placements_page + and_i_click_on("Add placement") + when_i_choose_a_subject(subject_1.name) + and_i_click_on("Continue") + then_i_see_the_add_a_placement_mentor_page + when_i_check_a_mentor("Not known") + and_i_click_on("Continue") + then_i_see_the_check_your_answers_page(school.phase, nil) and_i_click_on("Publish placement") then_i_see_the_placements_page and_i_see_my_placement(school.phase) @@ -74,11 +83,10 @@ scenario "my selected options are rendered when navigating using the back button" do when_i_visit_the_placements_page and_i_click_on("Add placement") - when_i_choose_a_subject("Primary subject") + when_i_choose_a_subject(subject_1.name) and_i_click_on("Continue") when_i_click_on("Back") - then_my_chosen_subject_is_selected("Primary subject") - + then_my_chosen_subject_is_selected(subject_1.name) when_i_visit_the_add_mentor_page when_i_check_a_mentor(mentor_1.full_name) and_i_click_on("Continue") @@ -93,7 +101,7 @@ then_i_see_the_add_a_placement_subject_page(school.phase) and_i_see_the_error_message("Select a subject") - when_i_choose_a_subject("Primary subject") + when_i_choose_a_subject(subject_1.name) and_i_click_on("Continue") and_i_click_on("Continue") then_i_see_the_add_a_placement_mentor_page @@ -108,12 +116,12 @@ when_i_visit_the_placements_page and_i_click_on("Add placement") then_i_see_the_add_a_placement_subject_page(school.phase) - when_i_check_the_subject("Secondary subject") + when_i_check_the_subject(subject_2.name) and_i_click_on("Continue") then_i_see_the_add_a_placement_mentor_page when_i_check_a_mentor(mentor_1.full_name) and_i_click_on("Continue") - then_i_see_the_check_your_answers_page(school.phase) + then_i_see_the_check_your_answers_page(school.phase, mentor_1) and_i_click_on("Publish placement") then_i_see_the_placements_page and_i_see_my_placement(school.phase) @@ -130,14 +138,14 @@ when_i_choose_a_phase("Primary") and_i_click_on("Continue") then_i_see_the_add_a_placement_subject_page("Primary") - when_i_choose_a_subject("Primary") + when_i_choose_a_subject(subject_1.name) then_i_see_the_add_a_placement_subject_page("Primary") - when_i_choose_a_subject("Primary subject") + when_i_choose_a_subject(subject_1.name) and_i_click_on("Continue") then_i_see_the_add_a_placement_mentor_page when_i_check_a_mentor(mentor_1.full_name) and_i_click_on("Continue") - then_i_see_the_check_your_answers_page("Primary") + then_i_see_the_check_your_answers_page("Primary", mentor_1) and_i_click_on("Publish placement") then_i_see_the_placements_page and_i_see_my_placement("Primary") @@ -153,13 +161,13 @@ when_i_choose_a_phase("Secondary") and_i_click_on("Continue") then_i_see_the_add_a_placement_subject_page("Secondary") - when_i_check_the_subject("Secondary subject") - and_i_check_the_subject("Secondary subject 2") + when_i_check_the_subject(subject_2.name) + and_i_check_the_subject(subject_3.name) and_i_click_on("Continue") then_i_see_the_add_a_placement_mentor_page when_i_check_a_mentor(mentor_1.full_name) and_i_click_on("Continue") - then_i_see_the_check_your_answers_page("Secondary") + then_i_see_the_check_your_answers_page("Secondary", mentor_1) and_i_click_on("Publish placement") then_i_see_the_placements_page and_i_see_my_placement("Secondary") @@ -272,11 +280,11 @@ def when_i_check_a_mentor(mentor_name) check mentor_name end - def then_i_see_the_check_your_answers_page(phase) + def then_i_see_the_check_your_answers_page(phase, mentor) expect(page).to have_content("Check your answers") expect(page).to have_content(phase) expect(page).to have_content("#{phase} subject") - expect(page).to have_content(mentor_1.full_name) + expect(page).to have_content(mentor.full_name) if mentor.present? end def and_my_chosen_mentor_is_checked(mentor_name)