From 418cb369b27e8bd4a3fa2d43fe75789e74cd61f3 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Wed, 10 Apr 2024 09:17:09 +0100 Subject: [PATCH 1/5] Refactor document upload form This makes a few small refactors to the document upload functionality to make the code easier to read. --- .../teacher_interface/documents_controller.rb | 14 +++++++------ .../add_another_upload_form.rb | 10 ++++----- .../documents/edit_available.html.erb | 2 +- .../documents/edit_uploads.html.erb | 21 ++++++++----------- .../teacher_interface/uploads/delete.html.erb | 11 ++++------ config/locales/helpers.en.yml | 12 +++++++++++ 6 files changed, 38 insertions(+), 32 deletions(-) diff --git a/app/controllers/teacher_interface/documents_controller.rb b/app/controllers/teacher_interface/documents_controller.rb index f2753fc9bb..99d7dbbb29 100644 --- a/app/controllers/teacher_interface/documents_controller.rb +++ b/app/controllers/teacher_interface/documents_controller.rb @@ -13,13 +13,15 @@ class DocumentsController < BaseController def show if document.uploads.empty? && !document.optional? - redirect_to new_teacher_interface_application_form_document_upload_path( + redirect_to [ + :new, + :teacher_interface, + :application_form, document, - ) + :upload, + ] else - redirect_to edit_teacher_interface_application_form_document_path( - document, - ) + redirect_to [:edit, :teacher_interface, :application_form, document] end end @@ -46,7 +48,7 @@ def update ), ) else - TeacherInterface::AddAnotherUploadForm.new( + AddAnotherUploadForm.new( add_another: params.dig( :teacher_interface_add_another_upload_form, diff --git a/app/forms/teacher_interface/add_another_upload_form.rb b/app/forms/teacher_interface/add_another_upload_form.rb index 810a560f6f..360dab0baf 100644 --- a/app/forms/teacher_interface/add_another_upload_form.rb +++ b/app/forms/teacher_interface/add_another_upload_form.rb @@ -1,11 +1,9 @@ # frozen_string_literal: true -module TeacherInterface - class AddAnotherUploadForm < BaseForm - attribute :add_another, :boolean - validates :add_another, inclusion: { in: [true, false] } +class TeacherInterface::AddAnotherUploadForm < TeacherInterface::BaseForm + attribute :add_another, :boolean + validates :add_another, inclusion: { in: [true, false] } - def update_model - end + def update_model end end diff --git a/app/views/teacher_interface/documents/edit_available.html.erb b/app/views/teacher_interface/documents/edit_available.html.erb index 912524f570..adb287d5dc 100644 --- a/app/views/teacher_interface/documents/edit_available.html.erb +++ b/app/views/teacher_interface/documents/edit_available.html.erb @@ -1,4 +1,4 @@ -<% content_for :page_title, title_with_error_prefix("Upload a document", error: @form.errors.any?) %> +<% content_for :page_title, title_with_error_prefix(t("helpers.legend.teacher_interface_document_available_form.available"), error: @form.errors.any?) %> <% content_for :back_link_url, back_history_path(default: teacher_interface_application_form_path) %> <%= form_with model: @form, url: [:teacher_interface, :application_form, @document], method: :patch do |f| %> diff --git a/app/views/teacher_interface/documents/edit_uploads.html.erb b/app/views/teacher_interface/documents/edit_uploads.html.erb index 9abce4050f..5f2c06f42d 100644 --- a/app/views/teacher_interface/documents/edit_uploads.html.erb +++ b/app/views/teacher_interface/documents/edit_uploads.html.erb @@ -1,14 +1,16 @@ -<% content_for :page_title, title_with_error_prefix("Check your uploaded files", error: @form.errors.any?) %> +<% title = if @document.allow_multiple_uploads? + "Check your uploaded files – #{t("document.document_type.#{@document.document_type}")} document" + else + "Check your uploaded file" + end %> + +<% content_for :page_title, title_with_error_prefix(title, error: @form.errors.any?) %> <% content_for :back_link_url, back_history_path(default: teacher_interface_application_form_path) %> <%= form_with model: @form, url: [:teacher_interface, :application_form, @document], method: :patch do |f| %> <%= f.govuk_error_summary %> - <% if @document.allow_multiple_uploads? %> -

Check your uploaded files – <%= t("document.document_type.#{@document.document_type}") %> document

- <% else %> -

Check your uploaded file

- <% end %> +

<%= title %>

<% if @document.for_consent_request? %>

<%= qualification_title(@document.documentable.qualification) %>

@@ -45,12 +47,7 @@ end %> <% if @document.allow_multiple_uploads? %> - <%= f.govuk_radio_buttons_fieldset :add_another, - legend: { text: "Do you need to upload another page?" }, - hint: { text: "If your document has several pages or you’re providing an image of the other side of your identity card or driving license, you can upload them here. Otherwise, select ‘No’ and then ‘Continue’." } do %> - <%= f.govuk_radio_button :add_another, "true", label: { text: "Yes" } %> - <%= f.govuk_radio_button :add_another, "false", label: { text: "No" } %> - <% end %> + <%= f.govuk_collection_radio_buttons :add_another, %i[true false], :itself %> <% else %> <%= f.hidden_field :add_another, value: "false" %> <% end %> diff --git a/app/views/teacher_interface/uploads/delete.html.erb b/app/views/teacher_interface/uploads/delete.html.erb index c366612943..f5798da0f3 100644 --- a/app/views/teacher_interface/uploads/delete.html.erb +++ b/app/views/teacher_interface/uploads/delete.html.erb @@ -1,15 +1,12 @@ -<% content_for :page_title, title_with_error_prefix("Delete #{@upload.filename}", error: @form.errors.any?) %> +<% title = "Are you sure you want to delete #{@upload.filename}?" %> + +<% content_for :page_title, title_with_error_prefix(title, error: @form.errors.any?) %> <% content_for :back_link_url, back_history_path(default: edit_teacher_interface_application_form_document_path(@document)) %> <%= form_with model: @form, url: [:teacher_interface, :application_form, @document, @upload], method: :delete do |f| %> <%= f.govuk_error_summary %> - <%= f.govuk_collection_radio_buttons :confirm, - [OpenStruct.new(id: "true", name: "Yes"), OpenStruct.new(id: "false", name: "No")], - :id, - :name, - inline: true, - legend: { text: "Are you sure you want to delete #{@upload.filename}?", size: "l", tag: "h1" } %> + <%= f.govuk_collection_radio_buttons :confirm, %i[true false], :itself, legend: { text: title, size: "l", tag: "h1" } %> <%= f.govuk_submit %> <% end %> diff --git a/config/locales/helpers.en.yml b/config/locales/helpers.en.yml index 09164bcf37..1946a89390 100644 --- a/config/locales/helpers.en.yml +++ b/config/locales/helpers.en.yml @@ -29,6 +29,8 @@ en: email: We’ll use this to send you an email with a link to continue with your QTS application. Do not use a work or university email that you might lose access to. teacher_interface_add_another_qualification_form: add_another: Add another degree or qualification if they are related to your teaching career. For example, to show you are qualified to teach a particular subject. + teacher_interface_add_another_upload_form: + add_another: If your document has several pages or you’re providing an image of the other side of your identity card or driving license, you can upload them here. Otherwise, select ‘No’ and then ‘Continue’. teacher_interface_alternative_name_form: has_alternative_name: If your name appears differently on your ID documents or qualifications you need to upload proof of your name change, for example, your marriage or civil partnership certificate. alternative_given_names: Enter your full name apart from your family name @@ -168,6 +170,10 @@ en: add_another_options: true: "Yes" false: "No" + teacher_interface_add_another_upload_form: + add_another_options: + true: "Yes" + false: "No" teacher_interface_alternative_name_form: has_alternative_name_options: true: Yes – I’ll upload another document to prove this @@ -177,6 +183,10 @@ en: teacher_interface_age_range_form: minimum: Age From maximum: Age To + teacher_interface_delete_upload_form: + confirm_options: + true: "Yes" + false: "No" teacher_interface_delete_work_history_form: confirm_options: true: "Yes" @@ -293,6 +303,8 @@ en: work_experience: How long have you worked as a recognised teacher? teacher_interface_add_another_qualification_form: add_another: Do you want to add another qualification? + teacher_interface_add_another_upload_form: + add_another: Do you need to upload another page? teacher_interface_add_another_work_history_form: add_another: Add another role? teacher_interface_alternative_name_form: From c992e3c32ced60382de58d4e35250a9b9f29559f Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Wed, 10 Apr 2024 11:18:26 +0100 Subject: [PATCH 2/5] Show upload scan status This ensures that users can see the status of their uploads making it very clear if one of them has been scanned as having malware, and telling them what they need to do next. --- .../assessor_interface/uploads_controller.rb | 2 +- .../teacher_interface/documents_controller.rb | 26 ++++++++++++++----- .../teacher_interface/uploads_controller.rb | 2 +- app/helpers/upload_helper.rb | 16 +++++++----- app/models/document.rb | 6 ++++- app/models/upload.rb | 10 ++++--- .../documents/edit_uploads.html.erb | 2 +- .../teacher_interface/uploads/delete.html.erb | 17 +++++++++--- config/locales/teacher_interface.en.yml | 2 ++ spec/helpers/upload_helper_spec.rb | 17 +++--------- .../malware_scanning_spec.rb | 15 ++--------- 11 files changed, 65 insertions(+), 50 deletions(-) diff --git a/app/controllers/assessor_interface/uploads_controller.rb b/app/controllers/assessor_interface/uploads_controller.rb index d804020c4e..615f26d049 100644 --- a/app/controllers/assessor_interface/uploads_controller.rb +++ b/app/controllers/assessor_interface/uploads_controller.rb @@ -13,7 +13,7 @@ class UploadsController < BaseController def show authorize %i[assessor_interface application_form] - if upload.downloadable? + if upload.safe_to_link? send_blob_stream(upload.attachment, disposition: :inline) else render "shared/malware_scan" diff --git a/app/controllers/teacher_interface/documents_controller.rb b/app/controllers/teacher_interface/documents_controller.rb index 99d7dbbb29..60c5ce5e8e 100644 --- a/app/controllers/teacher_interface/documents_controller.rb +++ b/app/controllers/teacher_interface/documents_controller.rb @@ -26,14 +26,21 @@ def show end def edit - if show_available_form? - @form = + @form = + if show_available_form? DocumentAvailableForm.new(document:, available: document.available) - render :edit_available - else - @form = AddAnotherUploadForm.new - render :edit_uploads + else + AddAnotherUploadForm.new + end + + if document.any_unsafe_to_link? + @form.errors.add( + :uploads, + I18n.t("teacher_interface.documents.unsafe_to_link"), + ) end + + render show_available_form? ? :edit_available : :edit_uploads end def update @@ -57,6 +64,13 @@ def update ) end + if document.any_unsafe_to_link? + @form.errors.add( + :uploads, + I18n.t("teacher_interface.documents.unsafe_to_link"), + ) + end + handle_application_form_section( form: @form, if_success_then_redirect: ->(check_path) do diff --git a/app/controllers/teacher_interface/uploads_controller.rb b/app/controllers/teacher_interface/uploads_controller.rb index 0adac336f7..043ecf53f0 100644 --- a/app/controllers/teacher_interface/uploads_controller.rb +++ b/app/controllers/teacher_interface/uploads_controller.rb @@ -19,7 +19,7 @@ class UploadsController < BaseController before_action :load_upload, only: %i[delete destroy show] def show - if @upload.downloadable? + if @upload.safe_to_link? send_blob_stream(@upload.attachment, disposition: :inline) else render "shared/malware_scan" diff --git a/app/helpers/upload_helper.rb b/app/helpers/upload_helper.rb index 43b99c9e2b..73f8988c76 100644 --- a/app/helpers/upload_helper.rb +++ b/app/helpers/upload_helper.rb @@ -2,19 +2,23 @@ module UploadHelper def upload_link_to(upload) - href = + if upload.unsafe_to_link? + tag.div(class: "govuk-form-group--error") do + tag.p(upload.filename, class: "govuk-!-font-weight-bold") + + tag.p( + I18n.t("teacher_interface.documents.unsafe_to_link"), + class: "govuk-error-message", + ) + end + elsif upload.safe_to_link? govuk_link_to( "#{upload.filename} (opens in a new tab)", upload_path(upload), target: :_blank, rel: :noopener, ) - - if upload.malware_scan_error? || upload.malware_scan_suspect? - href + - tag.p("There’s a problem with this file", class: "govuk-error-message") else - href + upload.filename end end diff --git a/app/models/document.rb b/app/models/document.rb index de7fd9e930..0b7e097771 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -72,7 +72,11 @@ def completed? end def downloadable? - uploads.all?(&:downloadable?) + uploads.all?(&:safe_to_link?) + end + + def any_unsafe_to_link? + uploads.any?(&:unsafe_to_link?) end def for_further_information_request? diff --git a/app/models/upload.rb b/app/models/upload.rb index e858ef8c5b..ef38f2aa03 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -46,8 +46,12 @@ def is_pdf? attachment.blob.content_type == "application/pdf" end - def downloadable? - !FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) || - malware_scan_clean? + def safe_to_link? + malware_scan_clean? || + !FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) + end + + def unsafe_to_link? + malware_scan_error? || malware_scan_suspect? end end diff --git a/app/views/teacher_interface/documents/edit_uploads.html.erb b/app/views/teacher_interface/documents/edit_uploads.html.erb index 5f2c06f42d..e344ab17f4 100644 --- a/app/views/teacher_interface/documents/edit_uploads.html.erb +++ b/app/views/teacher_interface/documents/edit_uploads.html.erb @@ -20,7 +20,7 @@

Files added

<% end %> - <%= govuk_summary_list do |summary_list| + <%= govuk_summary_list(html_attributes: { id: "teacher-interface-add-another-upload-form-uploads-field-error" }) do |summary_list| @document.uploads.order(:created_at).each_with_index do |upload, index| summary_list.with_row do |row| if @document.allow_multiple_uploads? diff --git a/app/views/teacher_interface/uploads/delete.html.erb b/app/views/teacher_interface/uploads/delete.html.erb index f5798da0f3..e76162ce25 100644 --- a/app/views/teacher_interface/uploads/delete.html.erb +++ b/app/views/teacher_interface/uploads/delete.html.erb @@ -1,4 +1,8 @@ -<% title = "Are you sure you want to delete #{@upload.filename}?" %> +<% title = if @upload.unsafe_to_link? + "A suspected virus has been detected in #{@upload.filename}. Delete it and upload a new file." + else + "Are you sure you want to delete #{@upload.filename}?" + end %> <% content_for :page_title, title_with_error_prefix(title, error: @form.errors.any?) %> <% content_for :back_link_url, back_history_path(default: edit_teacher_interface_application_form_document_path(@document)) %> @@ -6,7 +10,12 @@ <%= form_with model: @form, url: [:teacher_interface, :application_form, @document, @upload], method: :delete do |f| %> <%= f.govuk_error_summary %> - <%= f.govuk_collection_radio_buttons :confirm, %i[true false], :itself, legend: { text: title, size: "l", tag: "h1" } %> - - <%= f.govuk_submit %> + <% if @upload.unsafe_to_link? %> +

<%= title %>

+ <%= f.hidden_field :confirm, value: "true" %> + <%= f.govuk_submit("Delete", warning: true) %> + <% else %> + <%= f.govuk_collection_radio_buttons :confirm, %i[true false], :itself, legend: { text: title, size: "l", tag: "h1" } %> + <%= f.govuk_submit %> + <% end %> <% end %> diff --git a/config/locales/teacher_interface.en.yml b/config/locales/teacher_interface.en.yml index eb7ec4df72..5fb2d5b887 100644 --- a/config/locales/teacher_interface.en.yml +++ b/config/locales/teacher_interface.en.yml @@ -58,6 +58,8 @@ en: draft: check: Check your application save: Save and sign out + documents: + unsafe_to_link: The selected file contains a suspected virus. Delete it and upload a new file. english_language: check: heading: Check your answers diff --git a/spec/helpers/upload_helper_spec.rb b/spec/helpers/upload_helper_spec.rb index 9dbca92466..10d458a652 100644 --- a/spec/helpers/upload_helper_spec.rb +++ b/spec/helpers/upload_helper_spec.rb @@ -35,30 +35,19 @@ context "when the upload malware scan is pending" do let(:upload) { build_stubbed(:upload, :pending) } - before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } - it "returns text for a pending scan" do - expect(upload_link_to).to have_link( - upload.filename, - href: - "/teacher/application/documents/#{upload.document.id}/uploads/#{upload.id}", - ) + expect(upload_link_to).to have_content(upload.filename) end end context "when the upload malware scan is suspect" do let(:upload) { build_stubbed(:upload, :suspect) } - before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } - it "appends an error for a suspect scan" do expect(upload_link_to).to have_content( - "There’s a problem with this file", - ) - expect(upload_link_to).to have_css( - "p.govuk-error-message", - text: "There’s a problem with this file", + "The selected file contains a suspected virus", ) + expect(upload_link_to).to have_css(".govuk-form-group--error") end end end diff --git a/spec/system/teacher_interface/malware_scanning_spec.rb b/spec/system/teacher_interface/malware_scanning_spec.rb index ea39e9889f..8fa59bfcf4 100644 --- a/spec/system/teacher_interface/malware_scanning_spec.rb +++ b/spec/system/teacher_interface/malware_scanning_spec.rb @@ -18,9 +18,6 @@ then_i_see_the_check_your_uploaded_files_page and_the_uploaded_files_have_been_marked_as_malware and_the_uploaded_files_have_been_removed - - when_i_click_on_a_file_marked_as_malware - then_i_see_theres_a_problem_with_the_file end def given_there_is_an_application_form @@ -59,10 +56,10 @@ def then_i_see_the_check_your_uploaded_files_page def and_the_uploaded_files_have_been_marked_as_malware expect(teacher_check_uploaded_files_page.files).to have_content( - "File 1\tupload.pdf (opens in a new tab)\nThere’s a problem with this file", + "File 1\t\nupload.pdf\nThe selected file contains a suspected virus.", ) expect(teacher_check_uploaded_files_page.files).to have_content( - "File 2\tupload.pdf (opens in a new tab)\nThere’s a problem with this file", + "File 2\t\nupload.pdf\nThe selected file contains a suspected virus.", ) end @@ -75,14 +72,6 @@ def and_the_uploaded_files_have_been_removed expect(written_statement.uploads.last.attachment.filename).to be_nil end - def when_i_click_on_a_file_marked_as_malware - first("a", text: "upload.pdf").click - end - - def then_i_see_theres_a_problem_with_the_file - expect(page).to have_content("There’s a problem with this file") - end - def teacher @teacher ||= create(:teacher) end From f0278e7416cba08eaa553971758d2cc321c40810 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Wed, 10 Apr 2024 11:31:14 +0100 Subject: [PATCH 3/5] Track document malware status This updates how we track the status of documents to include the malware status, preventing users from being able to submit their application if the scanning hasn't completed. --- app/components/status_tag/component.rb | 1 + app/components/task_list/component.html.erb | 2 +- .../consent_requests_controller.rb | 3 +- .../documents_controller.rb | 2 +- app/helpers/document_helper.rb | 38 +++++++--------- ...application_form_section_status_updater.rb | 42 ++++++++--------- app/models/application_form.rb | 5 ++- app/models/document.rb | 23 +++++++--- .../further_information_request_item.rb | 10 ++++- app/models/qualification.rb | 5 +++ .../qualification_requests_view_object.rb | 16 +------ .../application_form_view_object.rb | 6 +++ ...further_information_request_view_object.rb | 2 +- .../application_forms/show/_draft.html.erb | 19 ++++++++ .../uploads/_upload_timeout_error.html.erb | 30 ++++++------- config/locales/components.en.yml | 1 + ...eck_your_answers_summary_component_spec.rb | 19 -------- spec/models/application_form_spec.rb | 45 +++++++++++-------- .../further_information_request_item_spec.rb | 12 ++--- spec/support/system_helpers.rb | 4 ++ .../teacher_interface/qualifications_spec.rb | 4 +- ...er_information_request_view_object_spec.rb | 4 +- 22 files changed, 159 insertions(+), 134 deletions(-) diff --git a/app/components/status_tag/component.rb b/app/components/status_tag/component.rb index 38b8fa83c1..7991096856 100644 --- a/app/components/status_tag/component.rb +++ b/app/components/status_tag/component.rb @@ -32,6 +32,7 @@ def tags cannot_start: "grey", declined: "red", draft: "grey", + error: "red", expired: "pink", in_progress: "blue", invalid: "red", diff --git a/app/components/task_list/component.html.erb b/app/components/task_list/component.html.erb index a5b35fe21b..6820a071ec 100644 --- a/app/components/task_list/component.html.erb +++ b/app/components/task_list/component.html.erb @@ -1,6 +1,6 @@
    <% sections.each do |section| %> -
  1. +
  2. <% if (title = section[:title]).present? %>

    <% if sections.count > 1 %> diff --git a/app/controllers/assessor_interface/consent_requests_controller.rb b/app/controllers/assessor_interface/consent_requests_controller.rb index 94cc94b597..8211b8ebed 100644 --- a/app/controllers/assessor_interface/consent_requests_controller.rb +++ b/app/controllers/assessor_interface/consent_requests_controller.rb @@ -80,8 +80,7 @@ def update_upload end def check_upload - unless consent_request.unsigned_consent_document.completed? && - consent_request.unsigned_consent_document.downloadable? + unless consent_request.unsigned_consent_document.completed? history_stack.pop redirect_to [ diff --git a/app/controllers/assessor_interface/documents_controller.rb b/app/controllers/assessor_interface/documents_controller.rb index 7018540224..0d894be0cd 100644 --- a/app/controllers/assessor_interface/documents_controller.rb +++ b/app/controllers/assessor_interface/documents_controller.rb @@ -13,7 +13,7 @@ class DocumentsController < BaseController def show_pdf authorize %i[assessor_interface application_form] - unless document.downloadable? + unless document.completed? render "shared/malware_scan" return end diff --git a/app/helpers/document_helper.rb b/app/helpers/document_helper.rb index ffef55fc8a..d7a6187c52 100644 --- a/app/helpers/document_helper.rb +++ b/app/helpers/document_helper.rb @@ -4,34 +4,26 @@ module DocumentHelper include UploadHelper def document_link_to(document, translated: false) - scope = - ( - if translated - document.translated_uploads - else - document.original_uploads - end - ) - if document.optional? && !document.available - "The applicant has indicated that they haven't done an induction period " \ - "and don't have this document.".html_safe + tag.em( + "The applicant has indicated that they haven't " \ + "done an induction period and don't have this document.", + ) else uploads = - scope.order(:created_at).select { |upload| upload.attachment.present? } - - malware_scan_active = - FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) - - items = uploads.map { |upload| upload_link_to(upload) } - - if malware_scan_active && scope.malware_scan_suspect.exists? - items << tag.em( - "#{scope.count} #{"file upload".pluralize(scope.count)} has been scanned as malware and deleted.", + ( + if translated + document.translated_uploads + else + document.original_uploads + end ) - end - tag.ul(class: "govuk-list") { items.map { |item| concat(tag.li(item)) } } + tag.ul(class: "govuk-list") do + uploads + .order(:created_at) + .each { |upload| concat(tag.li(upload_link_to(upload))) } + end end end end diff --git a/app/lib/application_form_section_status_updater.rb b/app/lib/application_form_section_status_updater.rb index a3c905504a..6e63b2e308 100644 --- a/app/lib/application_form_section_status_updater.rb +++ b/app/lib/application_form_section_status_updater.rb @@ -73,7 +73,7 @@ def personal_information_status end def identification_document_status - identification_document.completed? ? :completed : :not_started + identification_document.status end def qualifications_status @@ -84,7 +84,13 @@ def qualifications_status return :in_progress end - qualifications.all?(&:complete?) ? :completed : :in_progress + if qualifications.any?(&:any_documents_unsafe_to_link?) + :error + elsif qualifications.all?(&:complete?) + :completed + else + :in_progress + end end def age_range_status @@ -105,16 +111,20 @@ def english_language_status case english_language_proof_method when "medium_of_instruction" - status_for_document( - english_language_medium_of_instruction_document, - not_started: :in_progress, - ) + if (status = english_language_medium_of_instruction_document.status) != + "not_started" + status + else + "in_progress" + end when "provider" if english_language_provider_other - status_for_document( - english_language_proficiency_document, - not_started: :in_progress, - ) + if (status = english_language_proficiency_document.status) != + "not_started" + status + else + "in_progress" + end else status_for_values( english_language_proof_method, @@ -167,7 +177,7 @@ def written_statement_status if teaching_authority_provides_written_statement written_statement_confirmation ? :completed : :not_started else - status_for_document(written_statement_document) + written_statement_document.status end end @@ -176,14 +186,4 @@ def status_for_values(*values) return :completed if values.all?(&:present?) :in_progress end - - def status_for_document(document, not_started: :not_started) - if document.completed? - :completed - elsif !document.available.nil? - :in_progress - else - not_started - end - end end diff --git a/app/models/application_form.rb b/app/models/application_form.rb index c1bc10a0a4..5a78ac19fd 100644 --- a/app/models/application_form.rb +++ b/app/models/application_form.rb @@ -155,9 +155,10 @@ class ApplicationForm < ApplicationRecord ].freeze STATUS_VALUES = { - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", }.freeze STATUS_COLUMNS.each { |column| enum column, STATUS_VALUES, prefix: column } diff --git a/app/models/document.rb b/app/models/document.rb index 0b7e097771..acf3a532af 100644 --- a/app/models/document.rb +++ b/app/models/document.rb @@ -68,17 +68,30 @@ def optional? end def completed? - uploads.present? || (optional? && available == false) - end - - def downloadable? - uploads.all?(&:safe_to_link?) + (uploads.present? && uploads.all?(&:safe_to_link?)) || + (optional? && available == false) end def any_unsafe_to_link? uploads.any?(&:unsafe_to_link?) end + def empty? + uploads.empty? && available.nil? + end + + def status + if any_unsafe_to_link? + "error" + elsif completed? + "completed" + elsif empty? + "not_started" + else + "in_progress" + end + end + def for_further_information_request? documentable.is_a?(FurtherInformationRequestItem) end diff --git a/app/models/further_information_request_item.rb b/app/models/further_information_request_item.rb index 365ffabedb..b82eba32df 100644 --- a/app/models/further_information_request_item.rb +++ b/app/models/further_information_request_item.rb @@ -40,8 +40,14 @@ class FurtherInformationRequestItem < ApplicationRecord work_history_contact: "work_history_contact", } - def state - completed? ? :completed : :not_started + def status + if completed? + "completed" + elsif document? && document.any_unsafe_to_link? + "error" + else + "not_started" + end end def completed? diff --git a/app/models/qualification.rb b/app/models/qualification.rb index 23ebfcddb3..d879822a4d 100644 --- a/app/models/qualification.rb +++ b/app/models/qualification.rb @@ -90,6 +90,11 @@ def incomplete? !complete? end + def any_documents_unsafe_to_link? + certificate_document.any_unsafe_to_link? || + transcript_document.any_unsafe_to_link? + end + def institution_country_name CountryName.from_code(institution_country_code) end diff --git a/app/view_objects/assessor_interface/qualification_requests_view_object.rb b/app/view_objects/assessor_interface/qualification_requests_view_object.rb index 29a4d01160..918632c5d7 100644 --- a/app/view_objects/assessor_interface/qualification_requests_view_object.rb +++ b/app/view_objects/assessor_interface/qualification_requests_view_object.rb @@ -128,9 +128,7 @@ def send_consent_document_task_item all_documents_completed = qualification_requests.consent_method_signed.count == consent_requests.count && - consent_requests - .map(&:unsigned_consent_document) - .all? { |document| document.completed? && document.downloadable? } + consent_requests.map(&:unsigned_consent_document).all?(&:completed?) all_consents_requested = consent_requests.all?(&:requested?) @@ -186,17 +184,7 @@ def signed_consent_method_task_items(qualification_request) ] end, status: - ( - if consent_request&.unsigned_consent_document&.completed? - if consent_request&.unsigned_consent_document&.downloadable? - "completed" - else - "in_progress" - end - else - "not_started" - end - ), + consent_request&.unsigned_consent_document&.status || "not_started", }, unless send_consent_document_in_all_qualifications? send_consent_document_task_item diff --git a/app/view_objects/teacher_interface/application_form_view_object.rb b/app/view_objects/teacher_interface/application_form_view_object.rb index bb554e4e35..b3100db511 100644 --- a/app/view_objects/teacher_interface/application_form_view_object.rb +++ b/app/view_objects/teacher_interface/application_form_view_object.rb @@ -55,6 +55,12 @@ def completed_task_list_sections end end + def errored_task_list_sections + task_list_sections.filter do |section| + section[:items].any? { |item| item[:status] == "error" } + end + end + def can_submit? completed_task_list_sections.count == task_list_sections.count end diff --git a/app/view_objects/teacher_interface/further_information_request_view_object.rb b/app/view_objects/teacher_interface/further_information_request_view_object.rb index 2708f1be76..f80d4655a5 100644 --- a/app/view_objects/teacher_interface/further_information_request_view_object.rb +++ b/app/view_objects/teacher_interface/further_information_request_view_object.rb @@ -30,7 +30,7 @@ def task_list_sections further_information_request, item, ], - status: item.state, + status: item.status, } end diff --git a/app/views/teacher_interface/application_forms/show/_draft.html.erb b/app/views/teacher_interface/application_forms/show/_draft.html.erb index 5c510ac00c..48729b4fba 100644 --- a/app/views/teacher_interface/application_forms/show/_draft.html.erb +++ b/app/views/teacher_interface/application_forms/show/_draft.html.erb @@ -1,3 +1,18 @@ +<% if (errored_task_list_sections = view_object.errored_task_list_sections).present? %> + +<% end %> + <% if view_object.show_work_history_under_submission_banner? || view_object.show_work_history_under_induction_banner? %> <%= govuk_notification_banner(title_text: "Important") do %>

    You have added at least one teaching role that started before you were qualified as a teacher.

    @@ -36,6 +51,10 @@ <%= render TaskList::Component.new(view_object.task_list_sections) %> +<% if errored_task_list_sections.present? %> +

    You cannot submit your application until all errors have been fixed.

    +<% end %> +
    <% if view_object.can_submit? %> <%= govuk_button_link_to t("teacher_interface.application_forms.show.draft.check"), edit_teacher_interface_application_form_path %> diff --git a/app/views/teacher_interface/uploads/_upload_timeout_error.html.erb b/app/views/teacher_interface/uploads/_upload_timeout_error.html.erb index 613a35cd5a..345b7abcfd 100644 --- a/app/views/teacher_interface/uploads/_upload_timeout_error.html.erb +++ b/app/views/teacher_interface/uploads/_upload_timeout_error.html.erb @@ -1,18 +1,16 @@ -
    -
    -

    - There was a problem uploading your document -

    -
    -
      -
    • - Please try uploading it again. -
      • -
    • - If you still experience this problem, email us at - <%= govuk_link_to t("service.email.enquiries"), email_path("enquiries") %> -
      • -
    -
    + diff --git a/config/locales/components.en.yml b/config/locales/components.en.yml index a9ce94d079..e043efdaa4 100644 --- a/config/locales/components.en.yml +++ b/config/locales/components.en.yml @@ -11,6 +11,7 @@ en: completed: Completed declined: Declined draft: Draft + error: Error expired: Overdue in_progress: In progress invalid: Invalid diff --git a/spec/components/check_your_answers_summary_component_spec.rb b/spec/components/check_your_answers_summary_component_spec.rb index f2559be104..c529279427 100644 --- a/spec/components/check_your_answers_summary_component_spec.rb +++ b/spec/components/check_your_answers_summary_component_spec.rb @@ -272,25 +272,6 @@ ) end - context "with a suspect malware scan" do - before do - FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) - create( - :upload, - malware_scan_result: "suspect", - document: model.document, - ) - end - - after { FeatureFlags::FeatureFlag.deactivate(:fetch_malware_scan_result) } - - it "includes a warning" do - expect(row.at_css(".govuk-summary-list__value").text).to include( - "2 file uploads has been scanned as malware and deleted.", - ) - end - end - it "renders the change link" do a = row.at_css(".govuk-summary-list__actions .govuk-link") diff --git a/spec/models/application_form_spec.rb b/spec/models/application_form_spec.rb index b1e2c38331..ff66525d02 100644 --- a/spec/models/application_form_spec.rb +++ b/spec/models/application_form_spec.rb @@ -133,9 +133,10 @@ it do is_expected.to define_enum_for(:personal_information_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:personal_information_status) .backed_by_column_of_type(:string) @@ -144,9 +145,10 @@ it do is_expected.to define_enum_for(:identification_document_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:identification_document_status) .backed_by_column_of_type(:string) @@ -155,9 +157,10 @@ it do is_expected.to define_enum_for(:qualifications_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:qualifications_status) .backed_by_column_of_type(:string) @@ -166,9 +169,10 @@ it do is_expected.to define_enum_for(:age_range_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:age_range_status) .backed_by_column_of_type(:string) @@ -177,9 +181,10 @@ it do is_expected.to define_enum_for(:subjects_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:subjects_status) .backed_by_column_of_type(:string) @@ -188,9 +193,10 @@ it do is_expected.to define_enum_for(:english_language_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:english_language_status) .backed_by_column_of_type(:string) @@ -199,9 +205,10 @@ it do is_expected.to define_enum_for(:work_history_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:work_history_status) .backed_by_column_of_type(:string) @@ -210,9 +217,10 @@ it do is_expected.to define_enum_for(:registration_number_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:registration_number_status) .backed_by_column_of_type(:string) @@ -221,9 +229,10 @@ it do is_expected.to define_enum_for(:written_statement_status) .with_values( - not_started: "not_started", - in_progress: "in_progress", completed: "completed", + error: "error", + in_progress: "in_progress", + not_started: "not_started", ) .with_prefix(:written_statement_status) .backed_by_column_of_type(:string) diff --git a/spec/models/further_information_request_item_spec.rb b/spec/models/further_information_request_item_spec.rb index 28dfac97c4..c5ad94e83f 100644 --- a/spec/models/further_information_request_item_spec.rb +++ b/spec/models/further_information_request_item_spec.rb @@ -45,8 +45,8 @@ create(:further_information_request_item) end - describe "#state" do - subject(:state) { further_information_request_item.state } + describe "#status" do + subject(:status) { further_information_request_item.status } context "with text information" do before do @@ -54,7 +54,7 @@ end context "without a response" do - it { is_expected.to eq(:not_started) } + it { is_expected.to eq("not_started") } end context "with a response" do @@ -62,7 +62,7 @@ further_information_request_item.update!(response: "response") end - it { is_expected.to eq(:completed) } + it { is_expected.to eq("completed") } end end @@ -73,7 +73,7 @@ end context "without an upload" do - it { is_expected.to eq(:not_started) } + it { is_expected.to eq("not_started") } end context "with an upload" do @@ -81,7 +81,7 @@ create(:upload, document: further_information_request_item.document) end - it { is_expected.to eq(:completed) } + it { is_expected.to eq("completed") } end end end diff --git a/spec/support/system_helpers.rb b/spec/support/system_helpers.rb index 5760ab7556..29a52b360c 100644 --- a/spec/support/system_helpers.rb +++ b/spec/support/system_helpers.rb @@ -95,6 +95,10 @@ def given_malware_scanning_is_enabled(scan_result: "No threats found") allow(stubbed_service).to receive(:call).and_return(stubbed_response) end + def given_malware_scanning_is_disabled + FeatureFlags::FeatureFlag.deactivate(:fetch_malware_scan_result) + end + def when_i_sign_out sign_out @user end diff --git a/spec/system/teacher_interface/qualifications_spec.rb b/spec/system/teacher_interface/qualifications_spec.rb index 41c4237f06..e9ec3735e3 100644 --- a/spec/system/teacher_interface/qualifications_spec.rb +++ b/spec/system/teacher_interface/qualifications_spec.rb @@ -6,10 +6,11 @@ before do given_i_am_authorized_as_a_user(teacher) given_an_application_form_exists - given_malware_scanning_is_enabled end it "records qualifications" do + given_malware_scanning_is_enabled + when_i_visit_the(:teacher_application_page) then_i_see_the(:teacher_application_page) and_i_see_the_qualifications_task @@ -44,6 +45,7 @@ end it "deletes qualifications" do + given_malware_scanning_is_disabled given_some_qualifications_exist when_i_visit_the(:teacher_application_page) diff --git a/spec/view_objects/teacher_interface/further_information_request_view_object_spec.rb b/spec/view_objects/teacher_interface/further_information_request_view_object_spec.rb index 9abb7322ff..58beecebb9 100644 --- a/spec/view_objects/teacher_interface/further_information_request_view_object_spec.rb +++ b/spec/view_objects/teacher_interface/further_information_request_view_object_spec.rb @@ -56,7 +56,7 @@ further_information_request, text_item, ], - status: :not_started, + status: "not_started", ) end @@ -71,7 +71,7 @@ further_information_request, document_item, ], - status: :not_started, + status: "not_started", ) end end From 9186a9def356672427b5c6dc974086294be83658 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Wed, 10 Apr 2024 11:36:24 +0100 Subject: [PATCH 4/5] Remove shared malware scan view We don't need to render this view any more as we won't be linking to the documents unless they're clean. --- .../assessor_interface/documents_controller.rb | 2 +- .../assessor_interface/uploads_controller.rb | 2 +- .../teacher_interface/uploads_controller.rb | 2 +- app/views/shared/malware_scan.html.erb | 9 --------- config/locales/en.yml | 10 ---------- .../assessor_interface/uploads_controller_spec.rb | 3 +-- .../teacher_interface/uploads_controller_spec.rb | 3 +-- 7 files changed, 5 insertions(+), 26 deletions(-) delete mode 100644 app/views/shared/malware_scan.html.erb diff --git a/app/controllers/assessor_interface/documents_controller.rb b/app/controllers/assessor_interface/documents_controller.rb index 0d894be0cd..b27fc6b905 100644 --- a/app/controllers/assessor_interface/documents_controller.rb +++ b/app/controllers/assessor_interface/documents_controller.rb @@ -14,7 +14,7 @@ def show_pdf authorize %i[assessor_interface application_form] unless document.completed? - render "shared/malware_scan" + render "errors/forbidden", status: :forbidden return end diff --git a/app/controllers/assessor_interface/uploads_controller.rb b/app/controllers/assessor_interface/uploads_controller.rb index 615f26d049..8a144b87e1 100644 --- a/app/controllers/assessor_interface/uploads_controller.rb +++ b/app/controllers/assessor_interface/uploads_controller.rb @@ -16,7 +16,7 @@ def show if upload.safe_to_link? send_blob_stream(upload.attachment, disposition: :inline) else - render "shared/malware_scan" + render "errors/forbidden", status: :forbidden end end diff --git a/app/controllers/teacher_interface/uploads_controller.rb b/app/controllers/teacher_interface/uploads_controller.rb index 043ecf53f0..613b30775b 100644 --- a/app/controllers/teacher_interface/uploads_controller.rb +++ b/app/controllers/teacher_interface/uploads_controller.rb @@ -22,7 +22,7 @@ def show if @upload.safe_to_link? send_blob_stream(@upload.attachment, disposition: :inline) else - render "shared/malware_scan" + render "errors/forbidden", status: :forbidden end end diff --git a/app/views/shared/malware_scan.html.erb b/app/views/shared/malware_scan.html.erb deleted file mode 100644 index bb2f55a35b..0000000000 --- a/app/views/shared/malware_scan.html.erb +++ /dev/null @@ -1,9 +0,0 @@ -<% content_for :page_title, t(@upload.malware_scan_result, scope: %i[malware_scan title]) %> - -

    - <%= t(@upload.malware_scan_result, scope: %i[malware_scan title]) %> -

    - -

    - <%= t(@upload.malware_scan_result, scope: %i[malware_scan body]) %> -

    diff --git a/config/locales/en.yml b/config/locales/en.yml index 2796ed92f1..c3bf2c5758 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -41,16 +41,6 @@ en: unsigned_consent: unsigned consent written_statement: written statement - malware_scan: - title: - error: There’s a problem with your file - pending: We’re checking your file - suspect: There’s a problem with your file - body: - error: There’s a problem with this file. You’ll need to take another image of the document you want to upload, then try again. - pending: We’re carrying out some checks on the file you’ve uploaded. Once we’ve successfully completed these checks, you’ll be able to preview your file. - suspect: There’s a problem with this file. You’ll need to take another image of the document you want to upload, then try again. - staff: omniauth_callbacks: failure: There was a problem signing you in. Please try again. diff --git a/spec/controllers/assessor_interface/uploads_controller_spec.rb b/spec/controllers/assessor_interface/uploads_controller_spec.rb index 8dbec6d18b..5a1504fe5e 100644 --- a/spec/controllers/assessor_interface/uploads_controller_spec.rb +++ b/spec/controllers/assessor_interface/uploads_controller_spec.rb @@ -64,14 +64,13 @@ end context "when the upload malware scan is not clean" do - render_views true let(:upload) { create(:upload, :suspect, document:) } before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } it "responds with information about the malware scan" do perform - expect(response.body).to include("There’s a problem with your file") + expect(response.status).to eq(403) end end end diff --git a/spec/controllers/teacher_interface/uploads_controller_spec.rb b/spec/controllers/teacher_interface/uploads_controller_spec.rb index ea41b9279f..ee82fea8a2 100644 --- a/spec/controllers/teacher_interface/uploads_controller_spec.rb +++ b/spec/controllers/teacher_interface/uploads_controller_spec.rb @@ -104,14 +104,13 @@ end context "when the upload malware scan is not clean" do - render_views true let(:upload) { create(:upload, :suspect, document:) } before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } it "renders information about the scan result" do perform - expect(response.body).to include("There’s a problem with your file") + expect(response.status).to eq(403) end end end From a24cbde20dc61bb127b500721a87f1bad9ecf4c4 Mon Sep 17 00:00:00 2001 From: Thomas Leese Date: Thu, 11 Apr 2024 10:42:27 +0100 Subject: [PATCH 5/5] Improve testing document uploads This marks the documents as clean in environments where we don't perform virus scanning, unless the document filename is virus.pdf in which case it's marked as a virus. --- app/forms/concerns/uploadable_form.rb | 32 ++++++++++++++----- app/models/upload.rb | 5 +-- .../uploads_controller_spec.rb | 2 -- .../uploads_controller_spec.rb | 2 -- spec/factories/application_forms.rb | 17 ++++++++-- spec/factories/qualifications.rb | 4 +-- .../delete_upload_form_spec.rb | 4 +-- .../further_information_request_item_spec.rb | 6 +++- spec/models/qualification_spec.rb | 4 +-- .../consent_requests_view_object_spec.rb | 6 +++- 10 files changed, 55 insertions(+), 27 deletions(-) diff --git a/app/forms/concerns/uploadable_form.rb b/app/forms/concerns/uploadable_form.rb index 3e8140ce37..83564a3868 100644 --- a/app/forms/concerns/uploadable_form.rb +++ b/app/forms/concerns/uploadable_form.rb @@ -24,25 +24,27 @@ module UploadableForm def create_uploads! document.uploads.each(&:destroy!) unless document.allow_multiple_uploads? + uploads = [] + if original_attachment.present? - document.uploads.create!( + uploads << document.uploads.create!( attachment: original_attachment, filename: original_attachment.original_filename, + malware_scan_result: malware_scan_result(original_attachment), translation: false, ) end if translated_attachment.present? - document.uploads.create!( + uploads << document.uploads.create!( attachment: translated_attachment, filename: translated_attachment.original_filename, + malware_scan_result: malware_scan_result(translated_attachment), translation: true, ) end - if FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) - fetch_and_update_malware_scan_results - end + fetch_and_update_malware_scan_results(uploads) end private @@ -61,9 +63,23 @@ def attachments_present end end - def fetch_and_update_malware_scan_results - document.uploads.each do |upload| - UpdateMalwareScanResultJob.set(wait: 2.seconds).perform_later(upload) + def fetch_and_update_malware_scan_results(uploads) + if FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) + uploads + .select(&:malware_scan_pending?) + .each do |upload| + UpdateMalwareScanResultJob.set(wait: 2.seconds).perform_later(upload) + end + end + end + + def malware_scan_result(attachment) + if FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) + "pending" + elsif attachment.original_filename == "virus.pdf" + "suspect" + else + "clean" end end end diff --git a/app/models/upload.rb b/app/models/upload.rb index ef38f2aa03..834f84c7c2 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -46,10 +46,7 @@ def is_pdf? attachment.blob.content_type == "application/pdf" end - def safe_to_link? - malware_scan_clean? || - !FeatureFlags::FeatureFlag.active?(:fetch_malware_scan_result) - end + alias_method :safe_to_link?, :malware_scan_clean? def unsafe_to_link? malware_scan_error? || malware_scan_suspect? diff --git a/spec/controllers/assessor_interface/uploads_controller_spec.rb b/spec/controllers/assessor_interface/uploads_controller_spec.rb index 5a1504fe5e..baccc1f8f3 100644 --- a/spec/controllers/assessor_interface/uploads_controller_spec.rb +++ b/spec/controllers/assessor_interface/uploads_controller_spec.rb @@ -66,8 +66,6 @@ context "when the upload malware scan is not clean" do let(:upload) { create(:upload, :suspect, document:) } - before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } - it "responds with information about the malware scan" do perform expect(response.status).to eq(403) diff --git a/spec/controllers/teacher_interface/uploads_controller_spec.rb b/spec/controllers/teacher_interface/uploads_controller_spec.rb index ee82fea8a2..b54e7e833f 100644 --- a/spec/controllers/teacher_interface/uploads_controller_spec.rb +++ b/spec/controllers/teacher_interface/uploads_controller_spec.rb @@ -106,8 +106,6 @@ context "when the upload malware scan is not clean" do let(:upload) { create(:upload, :suspect, document:) } - before { FeatureFlags::FeatureFlag.activate(:fetch_malware_scan_result) } - it "renders information about the scan result" do perform expect(response.status).to eq(403) diff --git a/spec/factories/application_forms.rb b/spec/factories/application_forms.rb index 3824faccb2..0e4e29ab96 100644 --- a/spec/factories/application_forms.rb +++ b/spec/factories/application_forms.rb @@ -238,7 +238,11 @@ trait :with_identification_document do identification_document_status { "completed" } after(:create) do |application_form, _evaluator| - create(:upload, document: application_form.identification_document) + create( + :upload, + :clean, + document: application_form.identification_document, + ) end end @@ -247,7 +251,7 @@ alternative_given_names { Faker::Name.name } alternative_family_name { Faker::Name.last_name } after(:create) do |application_form, _evaluator| - create(:upload, document: application_form.name_change_document) + create(:upload, :clean, document: application_form.name_change_document) end end @@ -297,6 +301,7 @@ after(:create) do |application_form, _evaluator| create( :upload, + :clean, document: application_form.english_language_medium_of_instruction_document, ) @@ -309,6 +314,7 @@ after(:create) do |application_form, _evaluator| create( :upload, + :clean, document: application_form.english_language_proficiency_document, ) end @@ -335,6 +341,7 @@ after(:create) do |application_form, _evaluator| create( :upload, + :clean, document: application_form.english_language_proficiency_document, ) end @@ -375,7 +382,11 @@ if application_form.teaching_authority_provides_written_statement application_form.update!(written_statement_confirmation: true) else - create(:upload, document: application_form.written_statement_document) + create( + :upload, + :clean, + document: application_form.written_statement_document, + ) end end end diff --git a/spec/factories/qualifications.rb b/spec/factories/qualifications.rb index 68a2a08acb..43b8570723 100644 --- a/spec/factories/qualifications.rb +++ b/spec/factories/qualifications.rb @@ -34,8 +34,8 @@ certificate_date { complete_date + 1.year } after(:create) do |qualification, _evaluator| - create(:upload, document: qualification.certificate_document) - create(:upload, document: qualification.transcript_document) + create(:upload, :clean, document: qualification.certificate_document) + create(:upload, :clean, document: qualification.transcript_document) end end end diff --git a/spec/forms/teacher_interface/delete_upload_form_spec.rb b/spec/forms/teacher_interface/delete_upload_form_spec.rb index ca88d127f9..0b8bb8d17e 100644 --- a/spec/forms/teacher_interface/delete_upload_form_spec.rb +++ b/spec/forms/teacher_interface/delete_upload_form_spec.rb @@ -17,7 +17,7 @@ subject(:save) { form.save(validate: true) } let(:document) { create(:document) } - let!(:upload) { create(:upload, document:) } + let!(:upload) { create(:upload, :clean, document:) } context "when confirm is true" do let(:confirm) { "true" } @@ -33,7 +33,7 @@ end context "with another upload" do - before { create(:upload, document:) } + before { create(:upload, :clean, document:) } it "doesn't mark the document as incomplete" do expect { save }.to_not change(document, :completed?).from(true) diff --git a/spec/models/further_information_request_item_spec.rb b/spec/models/further_information_request_item_spec.rb index c5ad94e83f..b58327c4fd 100644 --- a/spec/models/further_information_request_item_spec.rb +++ b/spec/models/further_information_request_item_spec.rb @@ -78,7 +78,11 @@ context "with an upload" do before do - create(:upload, document: further_information_request_item.document) + create( + :upload, + :clean, + document: further_information_request_item.document, + ) end it { is_expected.to eq("completed") } diff --git a/spec/models/qualification_spec.rb b/spec/models/qualification_spec.rb index b5d38fadb9..75320644d5 100644 --- a/spec/models/qualification_spec.rb +++ b/spec/models/qualification_spec.rb @@ -126,8 +126,8 @@ certificate_date: Date.new(2021, 1, 1), ) - create(:upload, document: qualification.certificate_document) - create(:upload, document: qualification.transcript_document) + create(:upload, :clean, document: qualification.certificate_document) + create(:upload, :clean, document: qualification.transcript_document) end it { is_expected.to be true } diff --git a/spec/view_objects/teacher_interface/consent_requests_view_object_spec.rb b/spec/view_objects/teacher_interface/consent_requests_view_object_spec.rb index 50863e8345..a2d6658000 100644 --- a/spec/view_objects/teacher_interface/consent_requests_view_object_spec.rb +++ b/spec/view_objects/teacher_interface/consent_requests_view_object_spec.rb @@ -97,7 +97,11 @@ context "when the signed consent document is uploaded" do before do - create(:upload, document: consent_request.signed_consent_document) + create( + :upload, + :clean, + document: consent_request.signed_consent_document, + ) end it do