Merge pull request #3805 from DFE-Digital/6467-bug-party-6-changing-a…

…-placement-adds-a-new-one-instead

[6467] Changing a placement adds a new one instead

app/forms/placement_form.rb

@@ -92,6 +92,8 @@ def save!
       else
         placement.update(attributes)
       end
+    elsif placements_form.placements.any? { |form| form.placement.slug == placement.slug }
+      placement.update(attributes)
     else
       create_placement unless destroy?
     end
@@ -146,11 +148,7 @@ def existing_urns
   end
 
   def placement_number
-    if persisted?
-      trainee.placements.index(placement)
-    else
-      placements_form.placements.count
-    end + 1
+    (placements_form.placements.index { |form| form.placement.slug == placement.slug } || placements_form.placements.count) + 1
   end
 
   def create_placement

app/forms/placements_form.rb

@@ -53,7 +53,7 @@ def find_placement_from_param(slug)
   def placements
     slug_placements_forms_map = {}
 
-    trainee.placements.includes([:school]).order(created_at: :asc).each do |placement|
+    trainee.placements.includes([:school]).find_each do |placement|
       slug_placements_forms_map[placement.slug] = PlacementForm.new(
         placements_form: self,
         placement: placement,

app/models/placement.rb

@@ -24,6 +24,8 @@
 class Placement < ApplicationRecord
   include Sluggable
 
+  default_scope { order(created_at: :asc) }
+
   belongs_to :trainee
   belongs_to :school, optional: true
 

app/views/trainees/placements/_form.html.erb

@@ -2,6 +2,8 @@
   model: [@trainee, @placement_form],
   local: true,
   html: { data: { module: "app-schools-autocomplete" } },
+  url: url,
+  method: method,
 ) do |f| %>
 
   <% if f.govuk_error_summary.present? %>

app/views/trainees/placements/edit.html.erb

@@ -9,6 +9,6 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
-    <%= render "form" %>
+    <%= render("form", url: trainee_placement_path, method: :patch) %>
   </div>
 </div>

app/views/trainees/placements/new.html.erb

@@ -9,6 +9,6 @@
 
 <div class="govuk-grid-row">
   <div class="govuk-grid-column-two-thirds">
-    <%= render "form" %>
+    <%= render("form", url: trainee_placements_path, method: :post) %>
   </div>
 </div>

spec/features/form_sections/teacher_training_data/placements/editing_trainee_placements_spec.rb

@@ -35,7 +35,7 @@
       given_a_trainee_exists(:submitted_for_trn)
 
       given_i_have_one_placement
-      and_a_school_exists
+      and_two_schools_exist
 
       and_i_am_on_the_confirm_placement_details_page
       when_i_click_on_the_change_link
@@ -47,20 +47,43 @@
       then_i_see_the_confirmation_page
       and_i_see_the_updated_placement
     end
+
+    scenario "editing a stashed new placement" do
+      given_a_trainee_exists(:submitted_for_trn)
+      and_two_schools_exist
+      and_i_am_on_the_confirm_placement_details_page
+      and_i_click_on_add_a_placement_button
+      and_i_am_on_the_add_placement_page
+      and_i_select_an_existing_school
+      and_i_click_continue
+      and_i_see_the_confirmation_page
+      and_i_see_the_updated_placement
+      and_no_placements_are_created
+
+      when_i_click_on_the_change_link
+      and_it_is_prepopulated_with_existing_placement(school: school_one)
+      and_i_select_an_existing_school(school: school_two)
+      and_i_click_continue
+
+      then_i_see_the_confirmation_page
+      and_i_see_the_updated_placement(school: school_two)
+      and_i_see_only_one_placement
+      and_no_placements_are_created
+      and_i_click_update
+      and_one_placement_are_created
+    end
   end
 
 private
 
-  def and_it_is_prepopulated_with_existing_placement
-    expect(page.find_by_id("schools-autocomplete-element")["data-default-value"]).to have_text(trainee.placements.first.school.name)
+  def and_it_is_prepopulated_with_existing_placement(school: trainee.placements.first.school)
+    expect(page.find_by_id("schools-autocomplete-element")["data-default-value"]).to have_text(school.name)
   end
 
-  def given_i_am_on_the_confirm_placement_details_page
+  def and_i_am_on_the_confirm_placement_details_page
     visit "/trainees/#{trainee.slug}/placements/confirm"
   end
 
-  alias_method :and_i_am_on_the_confirm_placement_details_page, :given_i_am_on_the_confirm_placement_details_page
-
   def then_i_am_taken_to_the_placement_edit_page
     expect(page).to have_current_path("/trainees/#{trainee.slug}/placements/#{trainee.placements.first.slug}/edit")
   end
@@ -73,92 +96,63 @@ def given_i_have_one_placement
     create(:placement, trainee:)
   end
 
-  def and_a_trainee_exists_with_trn_received
-    @trainee ||= given_a_trainee_exists(:trn_received)
-    FormStore.clear_all(@trainee.id)
+  def school_one
+    @school_one ||= create(:school)
   end
 
-  def and_a_school_exists
-    @school ||= create(:school)
+  def school_two
+    @school_two ||= create(:school)
   end
+  alias_method :and_a_school_exists, :school_one
 
-  def when_i_select_an_existing_school
-    find(:xpath, "//input[@name='placement[school_id]']", visible: false).set(@school.id)
+  def and_two_schools_exist
+    school_one
+    school_two
   end
 
-  def when_i_navigate_to_the_new_placement_form
-    visit new_trainee_placement_path(trainee_id: @trainee.slug)
+  def and_i_am_on_the_add_placement_page
+    expect(page).to have_current_path("/trainees/#{trainee.slug}/placements/new")
   end
-  alias_method :and_i_navigate_to_the_new_placement_form, :when_i_navigate_to_the_new_placement_form
 
-  def then_i_see_the_not_found_page
-    expect(page).to have_current_path(not_found_path)
-  end
-
-  def then_i_see_the_new_placement_form
-    expect(page).to have_content("First placement")
-  end
-
-  def then_i_see_the_second_new_placement_form
-    expect(page).to have_content("Second placement")
-  end
-
-  def when_i_select_an_existing_school
-    find(:xpath, "//input[@name='placement[school_id]']", visible: false).set(@school.id)
+  def when_i_select_an_existing_school(school: school_one)
+    find(:xpath, "//input[@name='placement[school_id]']", visible: false).set(school.id)
   end
+  alias_method :and_i_select_an_existing_school, :when_i_select_an_existing_school
 
   def and_i_click_continue
     click_button "Continue"
   end
 
-  def when_i_click_add_a_placement
-    click_link "Add a placement"
+  def and_i_click_on_add_a_placement_button
+    page.click_link("Add a placement", class: "govuk-button--secondary govuk-button")
   end
 
   def then_i_see_the_confirmation_page
-    expect(page).to have_current_path(trainee_placements_confirm_path(trainee_id: @trainee.slug))
+    expect(page).to have_current_path(trainee_placements_confirm_path(trainee_id: trainee.slug))
     expect(page).to have_content("Confirm placement details")
   end
+  alias_method :and_i_see_the_confirmation_page, :then_i_see_the_confirmation_page
 
-  def and_i_see_the_updated_placement
+  def and_i_see_the_updated_placement(school: school_one)
     expect(page).to have_content("First placement")
-    expect(page).to have_content(@school.name)
-    expect(page).to have_content(@school.postcode)
-    expect(page).to have_content("URN #{@school.urn}")
-  end
-
-  def when_i_enter_details_for_a_new_school
-    fill_in("School or setting name", with: "St. Alice's Primary School", visible: false)
-    fill_in("School unique reference number (URN) - if it has one", with: "654321", visible: false)
-    fill_in("Postcode", with: "OX1 1AA", visible: false)
-  end
-
-  def and_i_see_the_second_new_placement_ready_for_confirmation
-    expect(page).to have_content("Second placement")
-    expect(page).to have_content("St. Alice's Primary School")
-    expect(page).to have_content("OX1 1AA")
-    expect(page).to have_content("URN 654321")
+    expect(page).to have_content(school.name)
+    expect(page).to have_content(school.postcode)
+    expect(page).to have_content("URN #{school.urn}")
   end
 
-  def when_i_click_update
-    click_button "Update record"
-  end
-  alias_method :and_i_click_update, :when_i_click_update
-
-  def then_i_see_a_flash_message
-    expect(page).to have_content("Trainee placement details updated")
+  def and_i_see_only_one_placement
+    expect(page).to have_css(".app-summary-card", count: 1)
   end
 
   def and_no_placements_are_created
-    expect(@trainee.reload.placements.count).to eq(0)
+    expect(trainee.reload.placements.count).to eq(0)
   end
 
-  def and_two_new_placements_are_created
-    expect(@trainee.reload.placements.count).to eq(2)
+  def and_i_click_update
+    click_button "Update record"
   end
-  alias_method :and_no_new_placements_are_created, :and_two_new_placements_are_created
 
-  def when_i_revisit_the_placements_confirmation_page
-    visit trainee_placements_confirm_path(trainee_id: @trainee.slug)
+  def and_one_placement_are_created
+    expect(trainee.reload.placements.count).to eq(1)
   end
 end

spec/models/placement_spec.rb

@@ -5,6 +5,10 @@
 RSpec.describe Placement do
   let(:trainee) { create(:trainee) }
 
+  describe "default scope" do
+    it { expect(described_class.all.to_sql).to eq(described_class.order(created_at: :asc).to_sql) }
+  end
+
   context "validations" do
     context "when a school exists" do
       subject { described_class.new(trainee: trainee, school: create(:school)) }