Merge branch 'CLDC-3382-Support-user-functionality-merge-organisations' into CLDC-2093-Merge-organisations-page-view-all-merge-requests

app/components/bulk_upload_error_row_component.html.erb

@@ -1,13 +1,16 @@
 <div class="govuk-summary-card govuk-!-margin-bottom-6">
   <div class="govuk-summary-card__title-wrapper">
     <% if lettings? %>
-      <h3 class="govuk-summary-card__title"><strong>Row <%= row %></strong> <%= tenant_code_html %> <%= property_ref_html %></h3>
+      <h3 class="govuk-summary-card__title govuk-!-font-weight-regular"><strong>Row <%= row %></strong> <%= tenant_code_html %> <%= property_ref_html %></h3>
     <% else %>
-      <h3 class="govuk-summary-card__title"><strong>Row <%= row %></strong> <%= purchaser_code_html %></h3>
+      <h3 class="govuk-summary-card__title govuk-!-font-weight-regular"><strong>Row <%= row %></strong> <%= purchaser_code_html %></h3>
     <% end %>
   </div>
 
   <div class="govuk-summary-card__content">
+    <% potential_errors, critical_errors = bulk_upload_errors.partition { |error| error.category == "soft_validation" } %>
+    <h2 class="govuk-heading-m">Critical errors</h2>
+    <p class="govuk-body">These errors must be fixed to complete your logs.</p>
     <%= govuk_table do |table| %>
       <%= table.with_head do |head| %>
         <% head.with_row do |row| %>
@@ -18,16 +21,49 @@
         <% end %>
 
         <%= table.with_body do |body| %>
-          <% bulk_upload_errors.each do |error| %>
+          <% critical_errors.each do |error| %>
             <% body.with_row do |row| %>
-              <% row.with_cell(header: true, text: error.cell) %>
+                <% row.with_cell(text: error.cell) %>
-              <% row.with_cell(text: question_for_field(error.field)) %>
+                <% row.with_cell(text: question_for_field(error.field)) %>
-              <% row.with_cell(text: error.error) %>
+                <% row.with_cell(text: error.error, html_attributes: { class: "govuk-!-font-weight-bold" }) %>
-              <% row.with_cell(text: error.field.humanize) %>
+                <% row.with_cell(text: error.field.humanize) %>
             <% end %>
           <% end %>
         <% end %>
       <% end %>
     <% end %>
+
+    <% if potential_errors.any? %>
+      <h2 class="govuk-heading-m">Potential errors</h2>
+      <p class="govuk-body">The following groups of cells might have conflicting data. Check the answers and fix any incorrect data.<br><br>If the answers are correct, fix the critical errors and reupload the file. You'll need to confirm that the following data is correct when the file only contains potential errors.</p>
+        <%= govuk_table do |table| %>
+          <%= table.with_head do |head| %>
+            <% head.with_row do |row| %>
+              <% row.with_cell(header: true, text: "Cell") %>
+              <% row.with_cell(header: true, text: "Question") %>
+              <% row.with_cell(header: true, text: "Potential error") %>
+              <% row.with_cell(header: true, text: "Specification") %>
+            <% end %>
+          <% end %>
+
+          <%= table.with_body do |body| %>
+            <% potential_errors.group_by(&:error).each do |error_message, errors| %>
+              <% errors.each_with_index do |error, index| %>
+                <% row_class = "grouped-rows" %>
+                <% row_class += " first-row" if index.zero? %>
+                <% row_class += " last-row" if index == errors.size - 1 %>
+                <% body.with_row(html_attributes: { class: row_class }) do |row| %>
+                  <% row.with_cell(text: error.cell) %>
+                  <% row.with_cell(text: question_for_field(error.field)) %>
+                  <% if index == 0 %>
+                    <% row.with_cell(text: error_message, rowspan: errors.size, html_attributes: { class: "govuk-!-font-weight-bold grouped-multirow-cell" }) %>
+                  <% end %>
+                  <% row.with_cell(text: error.field.humanize) %>
+                <% end %>
+              <% end %>
+            <% end %>
+          <% end %>
+        <% end %>
+    <% end %>
   </div>
 </div>

app/frontend/styles/application.scss

@@ -81,3 +81,17 @@ $govuk-breakpoints: (
 .govuk-footer {
   border-top: govuk-spacing(2) solid $govuk-brand-colour;
 }
+
+.grouped-rows td {
+  border-top: none;
+  border-bottom: none;
+}
+
+.grouped-rows.first-row td {
+  border-top: 1px solid #b1b4b6;
+}
+
+.grouped-rows.last-row td,
+.grouped-rows .grouped-multirow-cell {
+  border-bottom: 1px solid #b1b4b6;
+}

app/models/form/question.rb

@@ -208,7 +208,8 @@ class Form::Question
   end
 
   def unanswered_error_message
-    I18n.t("validations.not_answered", question: error_display_label.downcase)
+    question_text = error_display_label.presence || "this question"
+    I18n.t("validations.not_answered", question: question_text.downcase)
   end
 
   def suffix_label(log)

app/models/lettings_log.rb

@@ -581,14 +581,22 @@ class LettingsLog < Log
   def non_location_setup_questions_completed?
     form.setup_sections.all? do |section|
       section.subsections.all? do |subsection|
-        relevant_qs = subsection.applicable_questions(self).reject { |q| optional_fields.include?(q.id) || %w[scheme_id location_id].include?(q.id) }
+        relevant_qs = subsection.questions.reject { |q| optional_fields.include?(q.id) || %w[scheme_id location_id].include?(q.id) }
-        relevant_qs.all? do |question|
+        relevant_applicable_qs = select_applicable_questions(self, relevant_qs)
+        relevant_applicable_qs.all? do |question|
           question.completed?(self)
         end
       end
     end
   end
 
+  # this is the same as the subsection method, but only for given questions
+  def select_applicable_questions(log, questions)
+    questions.select do |q|
+      (q.displayed_to_user?(log) && !q.derived?(log)) || q.is_derived_or_has_inferred_check_answers_value?(log)
+    end
+  end
+
   def resolve!
     update(unresolved: false)
   end

app/services/bulk_upload/lettings/year2024/row_parser.rb

@@ -44,7 +44,7 @@ class BulkUpload::Lettings::Year2024::RowParser
     field_39: "If 'Other', what is the type of tenancy?",
     field_40: "What is the length of the fixed-term tenancy to the nearest year?",
     field_41: "Is this letting sheltered accommodation?",
-    field_15: "Has tenant seen the DLUHC privacy notice?",
+    field_15: "Has tenant seen the MHCLG privacy notice?",
     field_42: "What is the lead tenant's age?",
     field_43: "Which of these best describes the lead tenant's gender identity?",
     field_44: "Which of these best describes the lead tenant's ethnic background?",
@@ -806,16 +806,14 @@ private
 
       if setup_question?(question)
         fields.each do |field|
-          if errors.select { |e| fields.include?(e.attribute) }.none?
+          if errors.select { |e| fields.include?(e.attribute) }.none? && field.present?
-            question_text = question.error_display_label.presence || "this question"
+            errors.add(field, question.unanswered_error_message, category: :setup)
-            errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase), category: :setup) if field.present?
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            question_text = question.error_display_label.presence || "this question"
+            errors.add(field, question.unanswered_error_message)
-            errors.add(field, I18n.t("validations.not_answered", question: question_text.downcase))
           end
         end
       end

app/services/bulk_upload/sales/year2024/row_parser.rb

@@ -1364,13 +1364,13 @@ private
       if setup_question?(question)
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase), category: :setup)
+            errors.add(field, question.unanswered_error_message, category: :setup)
           end
         end
       else
         fields.each do |field|
           unless errors.any? { |e| fields.include?(e.attribute) }
-            errors.add(field, I18n.t("validations.not_answered", question: question.error_display_label&.downcase))
+            errors.add(field, question.unanswered_error_message)
           end
         end
       end

app/views/bulk_upload_lettings_results/summary.html.erb

@@ -4,7 +4,7 @@
     <h1 class="govuk-heading-l">Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again</h1>
 
     <p class="govuk-body-l">
-      We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+      We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadLettings::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
     </p>
 
     <p class="govuk-body-l">

app/views/bulk_upload_sales_results/summary.html.erb

@@ -4,7 +4,7 @@
     <h1 class="govuk-heading-l">Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again</h1>
 
     <p class="govuk-body-l">
-      We could not create logs from your bulk upload. Below is a list of everything that you need to fix your spreadsheet. You can download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
+      We could not create logs from your bulk upload because of the following errors. Download the <%= govuk_link_to "specification", Forms::BulkUploadSales::PrepareYourFile.new(year: @bulk_upload.year).specification_path, target: "_blank" %> to help you fix the cells in your CSV file.
     </p>
 
     <p class="govuk-body-l">

config/locales/en.yml

@@ -589,13 +589,13 @@ en:
 
     declaration:
       missing:
-        pre_2024: "You must show the MHCLG privacy notice to the tenant before you can submit this log."
+        pre_2024: "You must show the MHCLG privacy notice to the tenant before you can submit this log"
-        post_2024: "You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log."
+        post_2024: "You must show or give the tenant access to the MHCLG privacy notice before you can submit this log"
 
     privacynotice:
       missing:
-        pre_2024: "You must show the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log."
+        pre_2024: "You must show the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log"
-        post_2024: "You must show or give access to the MHCLG privacy notice to the %{buyer_or_buyers} before you can submit this log."
+        post_2024: "You must show or give the %{buyer_or_buyers} access to the MHCLG privacy notice before you can submit this log"
 
     scheme:
       toggle_date:

spec/components/bulk_upload_error_row_component_spec.rb

@@ -108,4 +108,53 @@ RSpec.describe BulkUploadErrorRowComponent, type: :component do
       expect(result).to have_content(expected)
     end
   end
+
+  context "when there are potential errors" do
+    let(:row) { rand(9_999) }
+    let(:tenant_code) { SecureRandom.hex(4) }
+    let(:property_ref) { SecureRandom.hex(4) }
+    let(:purchaser_code) { nil }
+    let(:category) { "soft_validation" }
+    let(:field_46) { 40 }
+    let(:field_50) { 5 }
+    let(:error) { "You told us this person is aged 40 years and retired." }
+    let(:bulk_upload) { create(:bulk_upload, :lettings) }
+    let(:bulk_upload_errors) do
+      [
+        FactoryBot.build(
+          :bulk_upload_error,
+          bulk_upload:,
+          row:,
+          tenant_code:,
+          property_ref:,
+          purchaser_code:,
+          field: :field_46,
+          error:,
+          category:,
+        ),
+        FactoryBot.build(
+          :bulk_upload_error,
+          bulk_upload:,
+          row:,
+          tenant_code:,
+          property_ref:,
+          purchaser_code:,
+          field: :field_50,
+          error:,
+          category:,
+        ),
+      ]
+    end
+
+    it "renders the potential errors section" do
+      result = render_inline(described_class.new(bulk_upload_errors:))
+      expect(result).to have_content("Potential errors")
+    end
+
+    it "renders the potential error message" do
+      expected = error
+      result = render_inline(described_class.new(bulk_upload_errors:))
+      expect(result).to have_content(expected, count: 1)
+    end
+  end
 end

spec/models/form/lettings/questions/declaration_spec.rb

@@ -63,7 +63,7 @@ RSpec.describe Form::Lettings::Questions::Declaration, type: :model do
     end
 
     it "returns correct unanswered_error_message" do
-      expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the tenant before you can submit this log.")
+      expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the tenant before you can submit this log")
     end
   end
 
@@ -87,7 +87,7 @@ RSpec.describe Form::Lettings::Questions::Declaration, type: :model do
     end
 
     it "returns correct unanswered_error_message" do
-      expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the tenant before you can submit this log.")
+      expect(question.unanswered_error_message).to eq("You must show or give the tenant access to the MHCLG privacy notice before you can submit this log")
     end
   end
 end

spec/models/form/sales/questions/privacy_notice_spec.rb

@@ -60,7 +60,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
       end
 
       it "returns correct unanswered_error_message" do
-        expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyer before you can submit this log.")
+        expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyer before you can submit this log")
       end
     end
 
@@ -78,7 +78,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
       end
 
       it "returns correct unanswered_error_message" do
-        expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyers before you can submit this log.")
+        expect(question.unanswered_error_message).to eq("You must show the MHCLG privacy notice to the buyers before you can submit this log")
       end
     end
   end
@@ -100,7 +100,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
       end
 
       it "returns correct unanswered_error_message" do
-        expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyer before you can submit this log.")
+        expect(question.unanswered_error_message).to eq("You must show or give the buyer access to the MHCLG privacy notice before you can submit this log")
       end
     end
 
@@ -118,7 +118,7 @@ RSpec.describe Form::Sales::Questions::PrivacyNotice, type: :model do
       end
 
       it "returns correct unanswered_error_message" do
-        expect(question.unanswered_error_message).to eq("You must show or give access to the MHCLG privacy notice to the buyers before you can submit this log.")
+        expect(question.unanswered_error_message).to eq("You must show or give the buyers access to the MHCLG privacy notice before you can submit this log")
       end
     end
   end

spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb

@@ -686,7 +686,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do
 
           it "cannot be nulled" do
             parser.valid?
-            expect(parser.errors[:field_45]).to eq(["You must show the MHCLG privacy notice to the tenant before you can submit this log."])
+            expect(parser.errors[:field_45]).to eq(["You must show the MHCLG privacy notice to the tenant before you can submit this log"])
           end
         end
       end

spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb

@@ -747,7 +747,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do
 
           it "cannot be nulled" do
             parser.valid?
-            expect(parser.errors[:field_15]).to eq(["You must answer tenant has seen the privacy notice"])
+            expect(parser.errors[:field_15]).to eq(["You must show or give the tenant access to the MHCLG privacy notice before you can submit this log"])
           end
         end
 

spec/services/bulk_upload/sales/year2024/row_parser_spec.rb

@@ -1042,12 +1042,21 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do
     describe "#field_18" do # data protection
       let(:attributes) { setup_section_params.merge({ field_18: nil }) }
 
+      before do
+        parser.valid?
+      end
+
       context "when not answered" do
         it "returns a setup error" do
-          parser.valid?
           expect(parser.errors.where(:field_18, category: :setup)).to be_present
         end
       end
+
+      context "when the privacy notice is not accepted" do
+        it "cannot be nulled" do
+          expect(parser.errors[:field_18]).to eq(["You must show or give the buyer access to the MHCLG privacy notice before you can submit this log"])
+        end
+      end
     end
 
     [

spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/show.html.erb" do
 
       fragment = Capybara::Node::Simple.new(rendered)
 
-      expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+      expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
     end
   end
 end

spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_lettings_results/summary.html.erb" do
 
       fragment = Capybara::Node::Simple.new(rendered)
 
-      expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+      expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
     end
   end
 end

spec/views/bulk_upload_sales_results/show.html.erb_spec.rb

@@ -32,8 +32,7 @@ RSpec.describe "bulk_upload_sales_results/show.html.erb" do
       render
 
       fragment = Capybara::Node::Simple.new(rendered)
-
+      expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
-      expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
     end
   end
 end

spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb

@@ -33,7 +33,7 @@ RSpec.describe "bulk_upload_sales_results/summary.html.erb" do
 
       fragment = Capybara::Node::Simple.new(rendered)
 
-      expect(fragment.find_css("table tbody th").map(&:inner_text)).to eql(%w[Z100 AA100])
+      expect(fragment.find_css("table tbody td").map(&:inner_text).values_at(0, 4)).to eql(%w[Z100 AA100])
     end
   end
 end