Merge branch 'main' into CLDC-2996-merged-scheme-activation-date-bug

.github/workflows/aws_deploy.yml

@@ -12,6 +12,10 @@ on:
       aws_task_prefix:
         required: true
         type: string
+      concurrency_tag:
+        required: false
+        type: string
+        default: ""
       environment:
         required: true
         type: string
@@ -20,7 +24,7 @@ on:
         type: string
 
 concurrency:
-  group: deploy-${{ inputs.environment }}
+  group: deploy-${{ inputs.environment }}${{ inputs.concurrency_tag }}
   cancel-in-progress: true
 
 env:

.github/workflows/review_pipeline.yml

@@ -33,6 +33,7 @@ jobs:
       aws_account_id: 837698168072
       aws_role_prefix: core-dev
       aws_task_prefix: core-review-${{ github.event.pull_request.number }}
+      concurrency_tag: ${{ github.event.pull_request.number }}
       environment: review
     permissions:
       id-token: write

app/frontend/controllers/conditional_question_controller.js

@@ -15,24 +15,27 @@ export default class extends Controller {
       Object.entries(conditionalFor).forEach(([targetQuestion, conditions]) => {
         if (!conditions.map(String).includes(String(selectedValue))) {
           const textNumericInput = document.getElementById(`${type}-${targetQuestion.replaceAll('_', '-')}-field`)
-          if (textNumericInput == null) {
+          const errorInput = document.getElementById(`${type}-${targetQuestion.replaceAll('_', '-')}-field-error`)
-            const dateInputs = [1, 2, 3].map((idx) => {
+          const dateInputs = [1, 2, 3].map((idx) => {
-              return document.getElementById(`${type.replaceAll('-', '_')}_${targetQuestion}_${idx}i`)
+            return document.getElementById(`lettings_log_mrcdate_${idx}i`)
-            })
+          })
-            this.clearDateInputs(dateInputs)
+          this.clearTextInput(textNumericInput)
-          } else {
+          this.clearTextInput(errorInput)
-            this.clearTextNumericInput(textNumericInput)
+          this.clearDateInputs(dateInputs)
-          }
         }
       })
     }
   }
 
-  clearTextNumericInput (input) {
+  clearTextInput (input) {
-    input.value = ''
+    if (input != null) {
+      input.value = ''
+    }
   }
 
   clearDateInputs (inputs) {
-    inputs.forEach((input) => { input.value = '' })
+    inputs.forEach((input) => {
+      this.clearTextInput(input)
+    })
   }
 }

app/mailers/bulk_upload_mailer.rb

@@ -50,9 +50,9 @@ class BulkUploadMailer < NotifyMailer
 
   def send_bulk_upload_complete_mail(user:, bulk_upload:)
     url = if bulk_upload.lettings?
-            lettings_logs_url("[years][]" => "", "[status][]" => "", user: :all, organisation_select: :all)
+            clear_filters_url(filter_type: "lettings_logs")
           else
-            sales_logs_url("[years][]" => "", "[status][]" => "", user: :all)
+            clear_filters_url(filter_type: "sales_logs")
           end
 
     n_logs = pluralize(bulk_upload.logs.count, "log")

app/views/form/page.html.erb

@@ -63,18 +63,20 @@
       <%= f.hidden_field :interruption_page_referrer_type, value: @interruption_page_referrer_type %>
 
       <div class="govuk-button-group">
-      <% if accessed_from_duplicate_logs?(request.query_parameters["referrer"]) %>
+      <% if !@page.interruption_screen? %>
-        <%= f.govuk_submit "Save changes" %>
+        <% if accessed_from_duplicate_logs?(request.query_parameters["referrer"]) %>
-        <%= govuk_link_to "Cancel", send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: request.query_parameters["original_log_id"]) %>
+          <%= f.govuk_submit "Save changes" %>
-      <% elsif returning_to_question_page?(@page, request.query_parameters["referrer"]) %>
+          <%= govuk_link_to "Cancel", send("#{@log.class.name.underscore}_duplicate_logs_path", @log, original_log_id: request.query_parameters["original_log_id"]) %>
-        <%= f.govuk_submit "Save changes" %>
+        <% elsif returning_to_question_page?(@page, request.query_parameters["referrer"]) %>
-        <%= govuk_link_to "Cancel", send(@log.form.cancel_path(@page, @log), @log) %>
+          <%= f.govuk_submit "Save changes" %>
-      <% else %>
+          <%= govuk_link_to "Cancel", send(@log.form.cancel_path(@page, @log), @log) %>
-        <%= f.govuk_submit "Save and continue" %>
+        <% else %>
-          <%= govuk_link_to(
+          <%= f.govuk_submit "Save and continue" %>
-            (@page.skip_text || "Skip for now"),
+            <%= govuk_link_to(
-            (@page.skip_href(@log) || send(@log.form.next_page_redirect_path(@page, @log, current_user), @log)),
+              (@page.skip_text || "Skip for now"),
-          ) %>
+              (@page.skip_href(@log) || send(@log.form.next_page_redirect_path(@page, @log, current_user), @log)),
+            ) %>
+        <% end %>
       <% end %>
       </div>
     </div>

spec/features/form/conditional_questions_spec.rb

@@ -33,12 +33,15 @@ RSpec.describe "Form Conditional Questions" do
 
   before do
     sign_in user
-    allow(sales_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
-    allow(lettings_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
-    allow(FormHandler.instance).to receive(:current_lettings_form).and_return(fake_2021_2022_form)
   end
 
   context "with a page where some questions are only conditionally shown, depending on how you answer the first question" do
+    before do
+      allow(sales_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
+      allow(lettings_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
+      allow(FormHandler.instance).to receive(:current_lettings_form).and_return(fake_2021_2022_form)
+    end
+
     it "initially hides conditional questions" do
       visit("/lettings-logs/#{id}/armed-forces")
       expect(page).not_to have_selector("#armed_forces_injured_div")
@@ -57,6 +60,12 @@ RSpec.describe "Form Conditional Questions" do
   end
 
   context "when a conditional question has a saved answer", js: true do
+    before do
+      allow(sales_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
+      allow(lettings_log.form).to receive(:new_logs_end_date).and_return(Time.zone.today + 1.day)
+      allow(FormHandler.instance).to receive(:current_lettings_form).and_return(fake_2021_2022_form)
+    end
+
     it "is displayed correctly" do
       lettings_log.update!(postcode_known: 1, postcode_full: "NW1 6RT")
       visit("/lettings-logs/#{id}/property-postcode")
@@ -73,4 +82,33 @@ RSpec.describe "Form Conditional Questions" do
       expect(page).to have_field("sales-log-age1-field", with: "")
     end
   end
+
+  context "when a conditional question has an error" do
+    let(:lettings_log) do
+      FactoryBot.create(
+        :lettings_log,
+        :completed,
+        created_by: user,
+      )
+    end
+
+    before do
+      FormHandler.instance.use_real_forms!
+    end
+
+    it "shows conditional questions if the required answer is selected and hides it again when a different answer option is selected", js: true do
+      visit("/lettings-logs/#{id}/lead-tenant-age")
+      choose("lettings-log-age1-known-0-field", allow_label_click: true)
+      fill_in("lettings-log-age1-field", with: "200")
+      click_button("Save and continue")
+      expect(page).not_to have_field("lettings-log-age1-field")
+      expect(page).to have_field("lettings-log-age1-field-error")
+      choose("lettings-log-age1-known-1-field", allow_label_click: true)
+      expect(page).not_to have_field("lettings-log-age1-field")
+      expect(page).not_to have_field("lettings-log-age1-field-error")
+      choose("lettings-log-age1-known-0-field", allow_label_click: true)
+      expect(page).not_to have_field("lettings-log-age1-field")
+      expect(page).to have_field("lettings-log-age1-field-error", with: "")
+    end
+  end
 end

spec/features/form/validations_spec.rb

@@ -149,6 +149,7 @@ RSpec.describe "validations" do
         expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/net-income-value-check")
         expect(page).to have_content("You told us the lead tenant’s income is £750.00 weekly.")
         expect(page).to have_content("This is higher than we would expect for their working situation.")
+        expect(page).not_to have_button("Save changes")
         click_button("Confirm and continue")
         expect(page).to have_current_path("/lettings-logs/#{lettings_log.id}/net-income-uc-proportion")
       end

spec/mailers/bulk_upload_mailer_spec.rb

@@ -47,7 +47,7 @@ RSpec.describe BulkUploadMailer do
           log_type: "lettings",
           upload_timestamp: bulk_upload.created_at.to_fs(:govuk_date_and_time),
           success_description: "The lettings 2022/23 data you uploaded has been checked. The 0 logs you uploaded are now complete.",
-          logs_link: lettings_logs_url("[years][]" => "", "[status][]" => "", user: :all, organisation_select: :all),
+          logs_link: clear_filters_url(filter_type: "lettings_logs"),
         },
       )