diff --git a/app/components/bulk_upload_summary_component.html.erb b/app/components/bulk_upload_summary_component.html.erb new file mode 100644 index 000000000..abbff4493 --- /dev/null +++ b/app/components/bulk_upload_summary_component.html.erb @@ -0,0 +1,37 @@ +
+
+
+
+

+ <%= bulk_upload.filename %> + +

+
+
+

Uploaded by: <%= bulk_upload.user.name %> (<%= bulk_upload.user.email %>)

+

Uploading organisation: <%= bulk_upload.organisation.name %>

+

Time of upload: <%= bulk_upload.created_at.to_formatted_s(:govuk_date_and_time) %>

+
+

+ <%= download_file_link(bulk_upload) %> + <%= view_error_report_link(bulk_upload) %> + <%= view_logs_link(bulk_upload) %> +

+
+ +
+
diff --git a/app/components/bulk_upload_summary_component.rb b/app/components/bulk_upload_summary_component.rb new file mode 100644 index 000000000..fa4cad414 --- /dev/null +++ b/app/components/bulk_upload_summary_component.rb @@ -0,0 +1,72 @@ +class BulkUploadSummaryComponent < ViewComponent::Base + attr_reader :bulk_upload + + def initialize(bulk_upload:) + @bulk_upload = bulk_upload + @bulk_upload_errors = bulk_upload.bulk_upload_errors + super + end + + def upload_status + helpers.status_tag(bulk_upload.status, ["app-tag--small govuk-!-font-weight-regular no-max-width"]) + end + + def setup_errors_count + @bulk_upload_errors.where(category: "setup").count + end + + def critical_errors_count + @bulk_upload_errors.where(category: [nil, "", "not_answered"]).count + end + + def potential_errors_count + @bulk_upload_errors.where(category: "soft_validation").count + end + + def formatted_count_text(count, singular_text, plural_text = nil) + return if count.nil? || count <= 0 + + text = count > 1 ? (plural_text || singular_text.pluralize(count)) : singular_text + content_tag(:p, class: "govuk-!-font-size-16 govuk-!-margin-bottom-1") do + concat(content_tag(:strong, count)) + concat(" #{text}") + end + end + + def counts(*counts_with_texts) + counts_with_texts.map { |count, singular_text, plural_text| + formatted_count_text(count, singular_text, plural_text) if count.present? + }.compact.join("").html_safe + end + + def download_file_link(bulk_upload) + send("download_#{bulk_upload.log_type}_file_link", bulk_upload) + end + + def download_lettings_file_link(bulk_upload) + govuk_link_to "Download file", download_lettings_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" + end + + def download_sales_file_link(bulk_upload) + govuk_link_to "Download file", download_sales_bulk_upload_path(bulk_upload), class: "govuk-link govuk-!-margin-right-2" + end + + def view_error_report_link(bulk_upload) + status = bulk_upload.status.to_s + return unless %w[important_errors critical_errors potential_errors].include?(status) + + path = if status == "important_errors" + "summary_bulk_upload_#{bulk_upload.log_type}_result_url" + else + "bulk_upload_#{bulk_upload.log_type}_result_path" + end + + govuk_link_to "View error report", send(path, bulk_upload), class: "govuk-link" + end + + def view_logs_link(bulk_upload) + return unless bulk_upload.status.to_s == "logs_uploaded_with_errors" + + govuk_link_to "View logs with errors", send("#{bulk_upload.log_type}_logs_path", bulk_upload_id: [bulk_upload.id]), class: "govuk-link" + end +end diff --git a/app/components/create_log_actions_component.html.erb b/app/components/create_log_actions_component.html.erb index 5a90587ed..8af78b169 100644 --- a/app/components/create_log_actions_component.html.erb +++ b/app/components/create_log_actions_component.html.erb @@ -1,10 +1,11 @@ <% if display_actions? %>
- <% if create_button_href.present? %> - <%= govuk_button_to create_button_copy, create_button_href, class: "govuk-!-margin-right-6" %> + <%= govuk_button_to create_button_copy, create_button_href, class: "govuk-!-margin-right-6" %> + <% unless user.support? %> + <%= govuk_button_link_to upload_button_copy, upload_button_href, secondary: true %> <% end %> - <% if upload_button_href.present? && !user.support? %> - <%= govuk_button_link_to upload_button_copy, upload_button_href, secondary: true %> + <% if user.support? %> + <%= govuk_button_link_to view_uploads_button_copy, view_uploads_button_href, secondary: true %> <% end %>
<% end %> diff --git a/app/components/create_log_actions_component.rb b/app/components/create_log_actions_component.rb index 4b451c2cc..4395c48a9 100644 --- a/app/components/create_log_actions_component.rb +++ b/app/components/create_log_actions_component.rb @@ -18,39 +18,27 @@ class CreateLogActionsComponent < ViewComponent::Base user.organisation.data_protection_confirmed? && user.organisation.organisation_or_stock_owner_signed_dsa_and_holds_own_stock? end - def create_button_href - case log_type - when "lettings" - lettings_logs_path - when "sales" - sales_logs_path - end + def create_button_copy + "Create a new #{log_type} log" end - def create_button_copy - case log_type - when "lettings" - "Create a new lettings log" - when "sales" - "Create a new sales log" - end + def create_button_href + send("#{log_type}_logs_path") end def upload_button_copy - case log_type - when "lettings" - "Upload lettings logs in bulk" - when "sales" - "Upload sales logs in bulk" - end + "Upload #{log_type} logs in bulk" end def upload_button_href - case log_type - when "lettings" - bulk_upload_lettings_log_path(id: "start") - when "sales" - bulk_upload_sales_log_path(id: "start") - end + send("bulk_upload_#{log_type}_log_path", id: "start") + end + + def view_uploads_button_copy + "View #{log_type} bulk uploads" + end + + def view_uploads_button_href + send("bulk_uploads_#{log_type}_logs_path") end end diff --git a/app/components/search_component.rb b/app/components/search_component.rb index 36d621240..6c77eb3c4 100644 --- a/app/components/search_component.rb +++ b/app/components/search_component.rb @@ -9,17 +9,9 @@ class SearchComponent < ViewComponent::Base end def path(current_user) - if request.path.include?("organisations") && request.path.include?("users") - request.path - elsif request.path.include?("organisations") && request.path.include?("logs") - request.path - elsif request.path.include?("organisations") && request.path.include?("schemes") - request.path - elsif request.path.include?("organisations") && request.path.include?("stock-owners") - request.path - elsif request.path.include?("organisations") && request.path.include?("managing-agents") - request.path - elsif request.path.include?("users") + return request.path if matching_path_conditions? + + if request.path.include?("users") user_path(current_user) elsif request.path.include?("organisations") organisations_path @@ -35,4 +27,17 @@ private def user_path(current_user) current_user.support? ? users_path : users_organisation_path(current_user.organisation) end + + def matching_path_conditions? + [ + %r{organisations/\d+/users}, + %r{organisations/\d+/lettings-logs}, + %r{organisations/\d+/sales-logs}, + %r{organisations/\d+/schemes}, + %r{organisations/\d+/stock-owners}, + %r{organisations/\d+/managing-agents}, + %r{sales-logs/bulk-uploads}, + %r{lettings-logs/bulk-uploads}, + ].any? { |pattern| request.path.match?(pattern) } + end end diff --git a/app/controllers/form_controller.rb b/app/controllers/form_controller.rb index 8a95464a7..70b6f892b 100644 --- a/app/controllers/form_controller.rb +++ b/app/controllers/form_controller.rb @@ -409,6 +409,7 @@ private next if question.subsection.id == "setup" question.page.questions.map(&:id).each { |id| @log[id] = nil } + @log.previous_la_known = nil if question.id == "ppostcode_full" end @log.save! @questions = params[@log.model_name.param_key].keys.reject { |id| %w[clear_question_ids page].include?(id) }.map { |id| @log.form.get_question(id, @log) } diff --git a/app/controllers/lettings_logs_controller.rb b/app/controllers/lettings_logs_controller.rb index 491d92372..0194946d4 100644 --- a/app/controllers/lettings_logs_controller.rb +++ b/app/controllers/lettings_logs_controller.rb @@ -5,8 +5,8 @@ class LettingsLogsController < LogsController before_action :find_resource, only: %i[update show] - before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv] - before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index email_csv download_csv] + before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv bulk_uploads] + before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index email_csv download_csv bulk_uploads] before_action :authenticate_scope!, only: %i[download_csv email_csv] before_action :extract_bulk_upload_from_session_filters, only: [:index] @@ -115,6 +115,40 @@ class LettingsLogsController < LogsController end end + def bulk_uploads + return render_not_authorized unless current_user.support? + + @filter_type = "lettings_bulk_uploads" + + if params[:organisation_id].present? && params[:clear_old_filters].present? + redirect_to clear_filters_path(filter_type: @filter_type, organisation_id: params[:organisation_id]) and return + end + + uploads = BulkUpload.lettings.where("created_at >= ?", 30.days.ago) + unpaginated_filtered_uploads = filter_manager.filtered_uploads(uploads, search_term, filter_manager.session_filters) + + @pagy, @bulk_uploads = pagy(unpaginated_filtered_uploads) + @search_term = search_term + @total_count = uploads.size + @searched = search_term.presence + render "bulk_upload_shared/uploads" + end + + def download_bulk_upload + return render_not_authorized unless current_user.support? + + bulk_upload = BulkUpload.find(params[:id]) + downloader = BulkUpload::Downloader.new(bulk_upload:) + + if Rails.env.development? + downloader.call + send_file downloader.path, filename: bulk_upload.filename, type: "text/csv" + else + presigned_url = downloader.presigned_url + redirect_to presigned_url, allow_other_host: true + end + end + private def session_filters @@ -122,7 +156,11 @@ private end def filter_manager - FilterManager.new(current_user:, session:, params:, filter_type: "lettings_logs") + if request.path.include?("bulk-uploads") + FilterManager.new(current_user:, session:, params:, filter_type: "lettings_bulk_uploads") + else + FilterManager.new(current_user:, session:, params:, filter_type: "lettings_logs") + end end def authenticate_scope! diff --git a/app/controllers/sales_logs_controller.rb b/app/controllers/sales_logs_controller.rb index 7c4d8aa60..d1bbe3bc2 100644 --- a/app/controllers/sales_logs_controller.rb +++ b/app/controllers/sales_logs_controller.rb @@ -3,8 +3,8 @@ class SalesLogsController < LogsController rescue_from ActiveRecord::RecordNotFound, with: :render_not_found - before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv] - before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index email_csv download_csv] + before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv bulk_uploads] + before_action -> { filter_manager.serialize_filters_to_session }, if: :current_user, only: %i[index email_csv download_csv bulk_uploads] before_action :authenticate_scope!, only: %i[download_csv email_csv] before_action :extract_bulk_upload_from_session_filters, only: [:index] @@ -85,6 +85,40 @@ class SalesLogsController < LogsController params.require(:sales_log).permit(SalesLog.editable_fields) end + def bulk_uploads + return render_not_authorized unless current_user.support? + + @filter_type = "sales_bulk_uploads" + + if params[:organisation_id].present? && params[:clear_old_filters].present? + redirect_to clear_filters_path(filter_type: @filter_type, organisation_id: params[:organisation_id]) and return + end + + uploads = BulkUpload.sales.where("created_at >= ?", 30.days.ago) + unpaginated_filtered_uploads = filter_manager.filtered_uploads(uploads, search_term, session_filters) + + @pagy, @bulk_uploads = pagy(unpaginated_filtered_uploads) + @search_term = search_term + @total_count = uploads.size + @searched = search_term.presence + render "bulk_upload_shared/uploads" + end + + def download_bulk_upload + return render_not_authorized unless current_user.support? + + bulk_upload = BulkUpload.find(params[:id]) + downloader = BulkUpload::Downloader.new(bulk_upload:) + + if Rails.env.development? + downloader.call + send_file downloader.path, filename: bulk_upload.filename, type: "text/csv" + else + presigned_url = downloader.presigned_url + redirect_to presigned_url, allow_other_host: true + end + end + private def session_filters @@ -92,7 +126,11 @@ private end def filter_manager - FilterManager.new(current_user:, session:, params:, filter_type: "sales_logs") + if request.path.include?("bulk-uploads") + FilterManager.new(current_user:, session:, params:, filter_type: "sales_bulk_uploads") + else + FilterManager.new(current_user:, session:, params:, filter_type: "sales_logs") + end end def extract_bulk_upload_from_session_filters diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index 868b4936d..70a538900 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -5,6 +5,14 @@ class SessionsController < ApplicationController if path_params[:organisation_id].present? redirect_to send("#{params[:filter_type]}_organisation_path", id: path_params[:organisation_id], scheme_id: path_params[:scheme_id], search: path_params[:search]) + elsif params[:filter_type].include?("bulk_uploads") + bulk_upload_type = params[:filter_type].split("_").first + uploading_organisation = params[:organisation_id].presence + if uploading_organisation.present? + redirect_to send("bulk_uploads_#{bulk_upload_type}_logs_path", search: path_params[:search], uploading_organisation:) + else + redirect_to send("bulk_uploads_#{bulk_upload_type}_logs_path", search: path_params[:search]) + end else redirect_to send("#{params[:filter_type]}_path", scheme_id: path_params[:scheme_id], search: path_params[:search]) end diff --git a/app/frontend/styles/_bulk-uploads.scss b/app/frontend/styles/_bulk-uploads.scss new file mode 100644 index 000000000..eceae6565 --- /dev/null +++ b/app/frontend/styles/_bulk-uploads.scss @@ -0,0 +1,18 @@ +.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; +} + +.text-normal-break { + white-space: normal; + word-break: break-all; +} diff --git a/app/frontend/styles/_tag.scss b/app/frontend/styles/_tag.scss index dc6c366fa..20a5d51c5 100644 --- a/app/frontend/styles/_tag.scss +++ b/app/frontend/styles/_tag.scss @@ -5,3 +5,7 @@ padding-bottom: 2px; padding-left: 6px; } + +.no-max-width { + max-width: none; +} diff --git a/app/frontend/styles/application.scss b/app/frontend/styles/application.scss index f37d9eb05..837b0db6d 100644 --- a/app/frontend/styles/application.scss +++ b/app/frontend/styles/application.scss @@ -23,6 +23,7 @@ $govuk-breakpoints: ( @import "govuk-prototype-styles"; @import "accessible-autocomplete"; +@import "bulk-uploads"; @import "button"; @import "card"; @import "data_box"; @@ -82,20 +83,6 @@ $govuk-breakpoints: ( 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; -} - .govuk-notification-banner__content > * { max-width: fit-content; } diff --git a/app/helpers/bulk_upload_helper.rb b/app/helpers/bulk_upload_helper.rb new file mode 100644 index 000000000..7c2fe6972 --- /dev/null +++ b/app/helpers/bulk_upload_helper.rb @@ -0,0 +1,12 @@ +module BulkUploadHelper + def bulk_upload_title(controller_name) + case controller_name + when "lettings_logs" + "Lettings bulk uploads" + when "sales_logs" + "Sales bulk uploads" + else + "Bulk uploads" + end + end +end diff --git a/app/helpers/filters_helper.rb b/app/helpers/filters_helper.rb index c090a4a41..e43614d45 100644 --- a/app/helpers/filters_helper.rb +++ b/app/helpers/filters_helper.rb @@ -5,18 +5,21 @@ module FiltersHelper return false unless session[session_name_for(filter_type)] selected_filters = JSON.parse(session[session_name_for(filter_type)]) - return true if !selected_filters.key?("user") && filter == "assigned_to" && value == :all - return true if selected_filters["assigned_to"] == "specific_user" && filter == "assigned_to" && value == :specific_user - return true if !selected_filters.key?("owning_organisation") && filter == "owning_organisation_select" && value == :all - return true if !selected_filters.key?("managing_organisation") && filter == "managing_organisation_select" && value == :all - - return true if (selected_filters["owning_organisation"].present? || selected_filters["owning_organisation_text_search"].present?) && filter == "owning_organisation_select" && value == :specific_org - return true if (selected_filters["managing_organisation"].present? || selected_filters["managing_organisation_text_search"].present?) && filter == "managing_organisation_select" && value == :specific_org - - return false if selected_filters[filter].blank? - - selected_filters[filter].include?(value.to_s) + case filter + when "assigned_to" + assigned_to_filter_selected?(selected_filters, value) + when "owning_organisation_select" + owning_organisation_filter_selected?(selected_filters, value) + when "managing_organisation_select" + managing_organisation_filter_selected?(selected_filters, value) + when "uploaded_by" + uploaded_by_filter_selected?(selected_filters, value) + when "uploading_organisation_select" + uploading_organisation_filter_selected?(selected_filters, value) + else + selected_filters[filter]&.include?(value.to_s) || false + end end def any_filter_selected?(filter_type) @@ -119,6 +122,11 @@ module FiltersHelper [OpenStruct.new(id: "", name: "Select an option", hint: "")] end + def uploaded_by_filter_options + user_options = User.all + [OpenStruct.new(id: "", name: "Select an option", hint: "")] + user_options.map { |user_option| OpenStruct.new(id: user_option.id, name: user_option.name, hint: user_option.email) } + end + def filter_search_url(category) case category when :user @@ -270,7 +278,7 @@ private filters.each.sum do |category, category_filters| if %w[years status needstypes bulk_upload_id].include?(category) category_filters.count(&:present?) - elsif %w[user owning_organisation managing_organisation user_text_search owning_organisation_text_search managing_organisation_text_search].include?(category) + elsif %w[user owning_organisation managing_organisation user_text_search owning_organisation_text_search managing_organisation_text_search uploading_organisation].include?(category) 1 else 0 @@ -335,4 +343,34 @@ private def unanswered_filter_value "You didn’t answer this filter".html_safe end + + def assigned_to_filter_selected?(selected_filters, value) + return true if !selected_filters.key?("user") && value == :all + + selected_filters["assigned_to"] == value.to_s + end + + def owning_organisation_filter_selected?(selected_filters, value) + return true if !selected_filters.key?("owning_organisation") && value == :all + + (selected_filters["owning_organisation"].present? || selected_filters["owning_organisation_text_search"].present?) && value == :specific_org + end + + def managing_organisation_filter_selected?(selected_filters, value) + return true if !selected_filters.key?("managing_organisation") && value == :all + + (selected_filters["managing_organisation"].present? || selected_filters["managing_organisation_text_search"].present?) && value == :specific_org + end + + def uploaded_by_filter_selected?(selected_filters, value) + return true if !selected_filters.key?("user") && value == :all + + selected_filters["uploaded_by"] == value.to_s + end + + def uploading_organisation_filter_selected?(selected_filters, value) + return true if !selected_filters.key?("uploading_organisation") && value == :all + + (selected_filters["uploading_organisation"].present? || selected_filters["uploading_organisation_text_search"].present?) && value == :specific_org + end end diff --git a/app/helpers/tag_helper.rb b/app/helpers/tag_helper.rb index 3c2e332f6..bc0d8e06b 100644 --- a/app/helpers/tag_helper.rb +++ b/app/helpers/tag_helper.rb @@ -19,6 +19,14 @@ module TagHelper request_merged: "Merged", ready_to_merge: "Ready to merge", processing: "Processing", + blank_template: "Blank template", + wrong_template: "Wrong template used", + important_errors: "Errors on important questions in CSV", + critical_errors: "Critical errors in CSV", + potential_errors: "Potential errors in CSV", + logs_uploaded_with_errors: "Logs uploaded with errors", + errors_fixed_in_service: "Errors fixed on site", + logs_uploaded_no_errors: "Logs uploaded with no errors", }.freeze COLOUR = { @@ -39,6 +47,14 @@ module TagHelper request_merged: "green", ready_to_merge: "blue", processing: "yellow", + blank_template: "red", + wrong_template: "red", + important_errors: "red", + critical_errors: "red", + potential_errors: "red", + logs_uploaded_with_errors: "blue", + errors_fixed_in_service: "green", + logs_uploaded_no_errors: "green", }.freeze def status_tag(status, classes = []) diff --git a/app/models/bulk_upload.rb b/app/models/bulk_upload.rb index 69ab42871..ad76c2192 100644 --- a/app/models/bulk_upload.rb +++ b/app/models/bulk_upload.rb @@ -1,6 +1,7 @@ class BulkUpload < ApplicationRecord enum log_type: { lettings: "lettings", sales: "sales" } enum rent_type_fix_status: { not_applied: "not_applied", applied: "applied", not_needed: "not_needed" } + enum failure_reason: { blank_template: "blank_template", wrong_template: "wrong_template" } belongs_to :user @@ -10,12 +11,53 @@ class BulkUpload < ApplicationRecord has_many :sales_logs after_initialize :generate_identifier, unless: :identifier + after_initialize :initialize_processing, if: :new_record? + + scope :search_by_filename, ->(filename) { where("filename ILIKE ?", "%#{filename}%") } + scope :search_by_user_name, ->(name) { where(user_id: User.where("name ILIKE ?", "%#{name}%").select(:id)) } + scope :search_by_user_email, ->(email) { where(user_id: User.where("email ILIKE ?", "%#{email}%").select(:id)) } + scope :search_by_organisation_name, ->(name) { where(organisation_id: Organisation.where("name ILIKE ?", "%#{name}%").select(:id)) } + + scope :search_by, lambda { |param| + search_by_filename(param) + .or(search_by_user_name(param)) + .or(search_by_user_email(param)) + .or(search_by_organisation_name(param)) + } + + scope :filter_by_id, ->(id) { where(id:) } + scope :filter_by_years, ->(years, _user = nil) { where(year: years) } + scope :filter_by_uploaded_by, ->(user_id, _user = nil) { where(user_id:) } + scope :filter_by_user_text_search, ->(param, _user = nil) { where(user_id: User.search_by(param).select(:id)) } + scope :filter_by_user, ->(user_id, _user = nil) { user_id.present? ? where(user_id:) : all } + scope :filter_by_uploading_organisation, ->(organisation_id, _user = nil) { where(organisation_id:) } def completed? incomplete_logs = logs.where.not(status: "completed") !incomplete_logs.exists? end + def status + return :processing if processing + return :blank_template if failure_reason == "blank_template" + return :wrong_template if failure_reason == "wrong_template" + + if logs.visible.exists? + return :errors_fixed_in_service if completed? && bulk_upload_errors.any? + return :logs_uploaded_with_errors if bulk_upload_errors.any? + end + + if bulk_upload_errors.important.any? + :important_errors + elsif bulk_upload_errors.critical.any? + :critical_errors + elsif bulk_upload_errors.potential.any? + :potential_errors + else + :logs_uploaded_no_errors + end + end + def year_combo "#{year}/#{year - 2000 + 1}" end @@ -105,9 +147,17 @@ class BulkUpload < ApplicationRecord User.find_by(id: moved_user_id)&.name end + def organisation + Organisation.find_by(id: organisation_id) + end + private def generate_identifier self.identifier ||= SecureRandom.uuid end + + def initialize_processing + self.processing = true if processing.nil? + end end diff --git a/app/models/bulk_upload_error.rb b/app/models/bulk_upload_error.rb index c9ca14b0f..154fca2fc 100644 --- a/app/models/bulk_upload_error.rb +++ b/app/models/bulk_upload_error.rb @@ -4,4 +4,8 @@ class BulkUploadError < ApplicationRecord scope :order_by_row, -> { order("row::integer ASC") } scope :order_by_cell, -> { order(Arel.sql("LPAD(cell, 10, '0')")) } scope :order_by_col, -> { order(Arel.sql("LPAD(col, 10, '0')")) } + scope :important, -> { where(category: "setup") } + scope :potential, -> { where(category: "soft_validation") } + scope :critical, -> { where(category: nil).or(where.not(category: %w[setup soft_validation])) } + scope :critical_or_important, -> { critical.or(important) } end diff --git a/app/models/user.rb b/app/models/user.rb index a32748167..0d3bc4846 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -224,12 +224,26 @@ class User < ApplicationRecord def logs_filters(specific_org: false) if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners? - %w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id user_text_search owning_organisation_text_search managing_organisation_text_search] + %w[years status needstypes assigned_to user owning_organisation managing_organisation bulk_upload_id user_text_search owning_organisation_text_search managing_organisation_text_search] else %w[years status needstypes assigned_to user bulk_upload_id user_text_search] end end + def scheme_filters(specific_org: false) + if (support? && !specific_org) || organisation.has_managing_agents? || organisation.has_stock_owners? + %w[status owning_organisation owning_organisation_text_search] + else + %w[status] + end + end + + def bulk_uploads_filters(specific_org: false) + return [] unless support? && !specific_org + + %w[user years uploaded_by uploading_organisation user_text_search uploading_organisation_text_search] + end + delegate :name, to: :organisation, prefix: true def self.download_attributes diff --git a/app/models/validations/financial_validations.rb b/app/models/validations/financial_validations.rb index 53e50a92f..724fa9b6e 100644 --- a/app/models/validations/financial_validations.rb +++ b/app/models/validations/financial_validations.rb @@ -121,6 +121,11 @@ module Validations::FinancialValidations def validate_rent_amount(record) if record.wtshortfall + if record.is_supported_housing? && record.wchchrg && (record.wtshortfall > record.wchchrg) + record.errors.add :tshortfall, message: I18n.t("validations.financial.tshortfall.more_than_carehome_charge") + record.errors.add :chcharge, I18n.t("validations.financial.carehome.less_than_shortfall") + end + if record.wtcharge && (record.wtshortfall > record.wtcharge) record.errors.add :tshortfall, :more_than_rent, message: I18n.t("validations.financial.tshortfall.more_than_total_charge") record.errors.add :tcharge, I18n.t("validations.financial.tcharge.less_than_shortfall") diff --git a/app/services/bulk_upload/downloader.rb b/app/services/bulk_upload/downloader.rb index a7dc9aad0..303b14070 100644 --- a/app/services/bulk_upload/downloader.rb +++ b/app/services/bulk_upload/downloader.rb @@ -15,6 +15,10 @@ class BulkUpload::Downloader file.unlink end + def presigned_url + s3_storage_service.get_presigned_url(bulk_upload.identifier, 60, response_content_disposition: "attachment; filename=#{bulk_upload.filename}") + end + private def download diff --git a/app/services/bulk_upload/lettings/year2024/row_parser.rb b/app/services/bulk_upload/lettings/year2024/row_parser.rb index 3a18ac633..5002deac1 100644 --- a/app/services/bulk_upload/lettings/year2024/row_parser.rb +++ b/app/services/bulk_upload/lettings/year2024/row_parser.rb @@ -456,12 +456,12 @@ class BulkUpload::Lettings::Year2024::RowParser return @valid = true if blank_row? super(:before_log) - before_errors = errors.dup + @before_errors = errors.dup log.valid? super(:after_log) - errors.merge!(before_errors) + errors.merge!(@before_errors) log.errors.each do |error| fields = field_mapping_for_errors[error.attribute] || [] @@ -815,13 +815,13 @@ private if setup_question?(question) fields.each do |field| - if errors.select { |e| fields.include?(e.attribute) }.none? && field.present? + if field.present? && errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) } errors.add(field, question.unanswered_error_message, category: :setup) end end else fields.each do |field| - unless errors.any? { |e| fields.include?(e.attribute) } + if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) } errors.add(field, question.unanswered_error_message) end end diff --git a/app/services/bulk_upload/processor.rb b/app/services/bulk_upload/processor.rb index 3fbb1e2d4..83a8d1ca0 100644 --- a/app/services/bulk_upload/processor.rb +++ b/app/services/bulk_upload/processor.rb @@ -1,6 +1,16 @@ class BulkUpload::Processor attr_reader :bulk_upload + BLANK_TEMPLATE_ERRORS = [ + I18n.t("activemodel.errors.models.bulk_upload/lettings/validator.attributes.base.blank_file"), + I18n.t("activemodel.errors.models.bulk_upload/sales/validator.attributes.base.blank_file"), + ].freeze + + WRONG_TEMPLATE_ERRORS = [ + *I18n.t("activemodel.errors.models.bulk_upload/lettings/validator.attributes.base", default: {}).values, + *I18n.t("activemodel.errors.models.bulk_upload/sales/validator.attributes.base", default: {}).values, + ].freeze + def initialize(bulk_upload:) @bulk_upload = bulk_upload end @@ -11,7 +21,7 @@ class BulkUpload::Processor download @bulk_upload.update!(total_logs_count: validator.total_logs_count) - return send_failure_mail(errors: validator.errors.full_messages) if validator.invalid? + return handle_invalid_validator if validator.invalid? validator.call @@ -37,6 +47,7 @@ class BulkUpload::Processor send_failure_mail ensure downloader.delete_local_file! + bulk_upload.update!(processing: false) end def approve @@ -144,4 +155,14 @@ private raise "Validator not found for #{bulk_upload.log_type}" end end + + def handle_invalid_validator + if BLANK_TEMPLATE_ERRORS.any? { |error| validator.errors.full_messages.include?(error) } + @bulk_upload.update!(failure_reason: "blank_template") + elsif WRONG_TEMPLATE_ERRORS.any? { |error| validator.errors.full_messages.include?(error) } + @bulk_upload.update!(failure_reason: "wrong_template") + end + + send_failure_mail(errors: validator.errors.full_messages) + end end diff --git a/app/services/bulk_upload/sales/year2024/row_parser.rb b/app/services/bulk_upload/sales/year2024/row_parser.rb index df1a06528..d024e9c86 100644 --- a/app/services/bulk_upload/sales/year2024/row_parser.rb +++ b/app/services/bulk_upload/sales/year2024/row_parser.rb @@ -504,12 +504,12 @@ class BulkUpload::Sales::Year2024::RowParser return true if blank_row? super(:before_log) - before_errors = errors.dup + @before_errors = errors.dup log.valid? super(:after_log) - errors.merge!(before_errors) + errors.merge!(@before_errors) log.errors.each do |error| fields = field_mapping_for_errors[error.attribute] || [] @@ -1377,13 +1377,13 @@ private if setup_question?(question) fields.each do |field| - unless errors.any? { |e| fields.include?(e.attribute) } + if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) } errors.add(field, question.unanswered_error_message, category: :setup) end end else fields.each do |field| - unless errors.any? { |e| fields.include?(e.attribute) } + if errors.none? { |e| fields.include?(e.attribute) } && @before_errors.none? { |e| fields.include?(e.attribute) } errors.add(field, question.unanswered_error_message) end end diff --git a/app/services/filter_manager.rb b/app/services/filter_manager.rb index 9f68a097c..7757ad39e 100644 --- a/app/services/filter_manager.rb +++ b/app/services/filter_manager.rb @@ -76,6 +76,21 @@ class FilterManager locations.order(created_at: :desc) end + def self.filter_uploads(uploads, search_term, filters, all_orgs, user) + uploads = filter_by_search(uploads, search_term) + + filters.each do |category, values| + next if Array(values).reject(&:empty?).blank? + next if category == "uploading_organisation" && all_orgs + next if category == "uploading_organisation_text_search" && all_orgs + next if category == "uploaded_by" + next if category == "uploaded_by_text_search" && filters["uploaded_by"] != "specific_user" + + uploads = uploads.public_send("filter_by_#{category}", values, user) + end + uploads.order(created_at: :desc) + end + def serialize_filters_to_session(specific_org: false) session[session_name_for(filter_type)] = session_filters(specific_org:).to_json end @@ -91,7 +106,6 @@ class FilterManager else {} end - if filter_type.include?("logs") current_user.logs_filters(specific_org:).each do |filter| new_filters[filter] = params[filter] if params[filter].present? @@ -117,13 +131,21 @@ class FilterManager end if filter_type.include?("schemes") - current_user.logs_filters(specific_org:).each do |filter| + current_user.scheme_filters(specific_org:).each do |filter| new_filters[filter] = params[filter] if params[filter].present? end new_filters = new_filters.except("owning_organisation") if params["owning_organisation_select"] == "all" end + if filter_type.include?("bulk_uploads") + current_user.bulk_uploads_filters(specific_org:).each do |filter| + new_filters[filter] = params[filter] if params[filter].present? + end + new_filters = new_filters.except("uploading_organisation") if params["uploading_organisation_select"] == "all" + new_filters = new_filters.except("user") if params["uploaded_by"] == "all" + new_filters["user"] = current_user.id.to_s if params["uploaded_by"] == "you" + end new_filters end @@ -152,6 +174,12 @@ class FilterManager @bulk_upload ||= current_user.bulk_uploads.find_by(id:) end + def filtered_uploads(uploads, search_term, filters) + all_orgs = params["uploading_organisation_select"] == "all" + + FilterManager.filter_uploads(uploads, search_term, filters, all_orgs, current_user) + end + private def logs_filters diff --git a/app/services/storage/s3_service.rb b/app/services/storage/s3_service.rb index 8f7803346..88199c0a0 100644 --- a/app/services/storage/s3_service.rb +++ b/app/services/storage/s3_service.rb @@ -20,10 +20,10 @@ module Storage response.key_count == 1 end - def get_presigned_url(file_name, duration) + def get_presigned_url(file_name, duration, response_content_disposition: nil) Aws::S3::Presigner .new({ client: @client }) - .presigned_url(:get_object, bucket: @configuration.bucket_name, key: file_name, expires_in: duration) + .presigned_url(:get_object, bucket: @configuration.bucket_name, key: file_name, expires_in: duration, response_content_disposition:) end def get_file_io(file_name) diff --git a/app/views/bulk_upload_lettings_results/show.html.erb b/app/views/bulk_upload_lettings_results/show.html.erb index 15c486b91..fd49c8f3e 100644 --- a/app/views/bulk_upload_lettings_results/show.html.erb +++ b/app/views/bulk_upload_lettings_results/show.html.erb @@ -13,7 +13,14 @@ Here’s 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. -

File: <%= @bulk_upload.filename %>

+

File name: <%= @bulk_upload.filename %>

+ + <% if current_user.support? %> +
+ <%= govuk_link_to "Download file", download_lettings_bulk_upload_path(@bulk_upload) %> +
+ <% end %> + diff --git a/app/views/bulk_upload_lettings_results/summary.html.erb b/app/views/bulk_upload_lettings_results/summary.html.erb index 8a59e8999..2e4fa91fc 100644 --- a/app/views/bulk_upload_lettings_results/summary.html.erb +++ b/app/views/bulk_upload_lettings_results/summary.html.erb @@ -5,13 +5,18 @@ Bulk upload for lettings (<%= @bulk_upload.year_combo %>)

Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again

-

+

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.

-

- File: <%= @bulk_upload.filename %> -

+

File name: <%= @bulk_upload.filename %>

+ + <% if current_user.support? %> +
+ <%= govuk_link_to "Download file", download_lettings_bulk_upload_path(@bulk_upload) %> +
+ <% end %> + diff --git a/app/views/bulk_upload_sales_results/show.html.erb b/app/views/bulk_upload_sales_results/show.html.erb index 776fdfa2f..6d0863617 100644 --- a/app/views/bulk_upload_sales_results/show.html.erb +++ b/app/views/bulk_upload_sales_results/show.html.erb @@ -13,7 +13,14 @@ Here’s 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. -

File: <%= @bulk_upload.filename %>

+

File name: <%= @bulk_upload.filename %>

+ + <% if current_user.support? %> +
+ <%= govuk_link_to "Download file", download_sales_bulk_upload_path(@bulk_upload) %> +
+ <% end %> + diff --git a/app/views/bulk_upload_sales_results/summary.html.erb b/app/views/bulk_upload_sales_results/summary.html.erb index 0e423621d..171cbf77f 100644 --- a/app/views/bulk_upload_sales_results/summary.html.erb +++ b/app/views/bulk_upload_sales_results/summary.html.erb @@ -5,13 +5,18 @@ Bulk upload for sales (<%= @bulk_upload.year_combo %>)

Fix <%= pluralize(@bulk_upload.bulk_upload_errors.count, "error") %> and upload file again

-

+

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.

-

- File: <%= @bulk_upload.filename %> -

+

File name: <%= @bulk_upload.filename %>

+ + <% if current_user.support? %> +
+ <%= govuk_link_to "Download file", download_sales_bulk_upload_path(@bulk_upload) %> +
+ <% end %> + diff --git a/app/views/bulk_upload_shared/_upload_filters.html.erb b/app/views/bulk_upload_shared/_upload_filters.html.erb new file mode 100644 index 000000000..39a51fa4b --- /dev/null +++ b/app/views/bulk_upload_shared/_upload_filters.html.erb @@ -0,0 +1,78 @@ +
+
+
+

Filters

+
+ +
+ <%= form_with html: { method: :get } do |f| %> + +
+

+ <%= filters_applied_text(@filter_type) %> +

+

+ <%= reset_filters_link(@filter_type, { search: request.params["search"] }.compact) %> +

+
+ + <%= render partial: "filters/checkbox_filter", + locals: { + f:, + options: collection_year_options, + label: "Collection year", + category: "years", + size: "s", + } %> + + <%= render partial: "filters/radio_filter", + locals: { + f:, + options: { + "all": { label: "Any user" }, + "you": { label: "You" }, + "specific_user": { + label: "Specific user", + conditional_filter: { + type: "text_select", + label: "User", + category: "user", + options: uploaded_by_filter_options, + caption_text: "User's name or email", + }, + }, + }, + label: "Uploaded by", + category: "uploaded_by", + size: "s", + } %> + + <%= render partial: "filters/radio_filter", locals: { + f:, + options: { + "all": { label: "Any organisation" }, + "specific_org": { + label: "Specific organisation", + conditional_filter: { + type: "select", + label: "Uploading Organisation", + category: "uploading_organisation", + options: all_owning_organisation_filter_options(current_user), + caption_text: "Organisation name", + }, + }, + }, + label: "Uploading organisation", + category: "uploading_organisation_select", + size: "s", + } %> + + <% if request.params["search"].present? %> + <%= f.hidden_field :search, value: request.params["search"] %> + <% end %> + + <%= f.govuk_submit "Apply filters", class: "govuk-!-margin-bottom-0" %> + <% end %> +
+
+
diff --git a/app/views/bulk_upload_shared/_upload_list.html.erb b/app/views/bulk_upload_shared/_upload_list.html.erb new file mode 100644 index 000000000..7b6057228 --- /dev/null +++ b/app/views/bulk_upload_shared/_upload_list.html.erb @@ -0,0 +1,15 @@ +

+
+
+ <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "files uploaded in the last 30 days", filters_count: applied_filters_count(@filter_type))) %> +
+
+ <% if searched || applied_filters_count(@filter_type).positive? %> +
+ <% end %> +
+
+

+<% bulk_uploads.map do |bulk_upload| %> + <%= render BulkUploadSummaryComponent.new(bulk_upload:) %> +<% end %> diff --git a/app/views/bulk_upload_shared/uploads.html.erb b/app/views/bulk_upload_shared/uploads.html.erb new file mode 100644 index 000000000..958887453 --- /dev/null +++ b/app/views/bulk_upload_shared/uploads.html.erb @@ -0,0 +1,28 @@ +<% item_label = format_label(@pagy.count, "uploads") %> +<% title = format_title(@searched, bulk_upload_title(controller.controller_name), current_user, item_label, @pagy.count, nil) %> + +<% content_for :title, title %> + +

+ <%= bulk_upload_title(controller.controller_name) %> +

+ +
+ <%= render partial: "bulk_upload_shared/upload_filters" %> + +
+ <%= render SearchComponent.new(current_user:, search_label: "Search by file name, user's name or email, or organisation", value: @searched) %> + <%= govuk_section_break(visible: true, size: "m") %> + <%= render partial: "bulk_upload_shared/upload_list", + locals: { + bulk_uploads: @bulk_uploads, + title: "Bulk uploads", + pagy: @pagy, + searched: @searched, + item_label:, + total_count: @total_count, + filter_type: @filter_type, + } %> + <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "bulk uploads" } %> +
+
diff --git a/app/views/layouts/application.html.erb b/app/views/layouts/application.html.erb index 9e68fa7b4..d235491a1 100644 --- a/app/views/layouts/application.html.erb +++ b/app/views/layouts/application.html.erb @@ -60,9 +60,6 @@ <% end %> - <% if Rails.env.production? && ENV["APP_HOST"].present? %> - - <% end %> diff --git a/app/views/logs/_create_for_org_actions.html.erb b/app/views/logs/_create_for_org_actions.html.erb index 0564e678a..9b5935cb0 100644 --- a/app/views/logs/_create_for_org_actions.html.erb +++ b/app/views/logs/_create_for_org_actions.html.erb @@ -1,12 +1,14 @@
<% if @organisation.data_protection_confirmed? %> <% if current_page?(controller: 'organisations', action: 'lettings_logs') %> - <%= govuk_button_to "Create a new lettings log for this organisation", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> - <%= govuk_button_link_to "Upload lettings logs in bulk", bulk_upload_lettings_log_path(id: "start", organisation_id: @organisation.id), secondary: true %> + <%= govuk_button_to "Create a new lettings log", lettings_logs_path(lettings_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "Upload lettings logs in bulk", bulk_upload_lettings_log_path(id: "start", organisation_id: @organisation.id), secondary: true, class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "View lettings bulk uploads", bulk_uploads_lettings_logs_path(organisation_id: @organisation.id, clear_old_filters: true), secondary: true %> <% end %> <% if current_page?(controller: 'organisations', action: 'sales_logs') %> - <%= govuk_button_to "Create a new sales log for this organisation", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> - <%= govuk_button_link_to "Upload sales logs in bulk", bulk_upload_sales_log_path(id: "start", organisation_id: @organisation.id), secondary: true %> + <%= govuk_button_to "Create a new sales log", sales_logs_path(sales_log: { owning_organisation_id: @organisation.id }, method: :post), class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "Upload sales logs in bulk", bulk_upload_sales_log_path(id: "start", organisation_id: @organisation.id), secondary: true, class: "govuk-!-margin-right-6" %> + <%= govuk_button_link_to "View sales bulk uploads", bulk_uploads_sales_logs_path(organisation_id: @organisation.id, clear_old_filters: true), secondary: true %> <% end %> <% end %>
diff --git a/app/views/organisations/index.html.erb b/app/views/organisations/index.html.erb index 3b96288f1..411d792c1 100644 --- a/app/views/organisations/index.html.erb +++ b/app/views/organisations/index.html.erb @@ -6,7 +6,7 @@ <%= render partial: "organisations/headings", locals: request.path == organisations_path ? { main: "Organisations", sub: nil } : { main: @organisation.name, sub: "Organisations" } %>
- <%= govuk_tabs(title: "Collection resources", classes: %w[app-tab__large-headers]) do |c| %> + <%= govuk_tabs(title: "Organisations", classes: %w[app-tab__large-headers]) do |c| %> <% c.with_tab(label: "All organisations") do %> <%= govuk_button_link_to "Create a new organisation", new_organisation_path, html: { method: :get } %> <%= render SearchComponent.new(current_user:, search_label: "Search by organisation name", value: @searched) %> diff --git a/config/locales/en.yml b/config/locales/en.yml index 691ba43f2..ae94ba0dc 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -392,6 +392,7 @@ en: tshortfall: outstanding_amount_not_expected: "You cannot answer the outstanding amount question if you don’t have outstanding rent or charges." more_than_total_charge: "Enter a value less than the total charge." + more_than_carehome_charge: "Enter a value less than the care home charge." must_be_positive: "Enter a value over £0.01 as you told us there is an outstanding amount." hbrentshortfall: outstanding_amount_not_expected: "Answer must be ‘yes’ as you have answered the outstanding amount question." @@ -464,6 +465,7 @@ en: carehome: out_of_range: "Household rent and other charges must be between %{min_chcharge} and %{max_chcharge} if paying %{period}." not_provided: "Enter how much rent and other charges the household pays %{period}." + less_than_shortfall: "The care home charge must be more than the outstanding amount." cash_discount_invalid: "Cash discount must be £0 - £999,999." staircasing: percentage_bought_must_be_greater_than_percentage_owned: "Total percentage %{buyer_now_owns} must be more than percentage bought in this transaction." diff --git a/config/routes.rb b/config/routes.rb index 3c4a8d90c..3fa83458a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -232,6 +232,7 @@ Rails.application.routes.draw do get "csv-download", to: "lettings_logs#download_csv" post "email-csv", to: "lettings_logs#email_csv" get "csv-confirmation", to: "lettings_logs#csv_confirmation" + get "bulk-uploads", to: "lettings_logs#bulk_uploads" get "delete-logs", to: "delete_logs#delete_lettings_logs" post "delete-logs", to: "delete_logs#delete_lettings_logs_with_selected_ids" @@ -273,6 +274,12 @@ Rails.application.routes.draw do end end + resources :bulk_uploads, path: "bulk-uploads", only: [] do + member do + get "download", to: "lettings_logs#download_bulk_upload", as: "download_lettings" + end + end + get "update-logs", to: "lettings_logs#update_logs" end @@ -304,6 +311,7 @@ Rails.application.routes.draw do get "csv-download", to: "sales_logs#download_csv" post "email-csv", to: "sales_logs#email_csv" get "csv-confirmation", to: "sales_logs#csv_confirmation" + get "bulk-uploads", to: "sales_logs#bulk_uploads" get "delete-logs", to: "delete_logs#delete_sales_logs" post "delete-logs", to: "delete_logs#delete_sales_logs_with_selected_ids" @@ -344,6 +352,12 @@ Rails.application.routes.draw do patch "*page", to: "bulk_upload_sales_soft_validations_check#update" end end + + resources :bulk_uploads, path: "bulk-uploads", only: [] do + member do + get "download", to: "sales_logs#download_bulk_upload", as: "download-sales" + end + end end member do diff --git a/db/migrate/20241002163937_add_failure_reason_and_processing_to_bulk_uploads.rb b/db/migrate/20241002163937_add_failure_reason_and_processing_to_bulk_uploads.rb new file mode 100644 index 000000000..ef242d0f0 --- /dev/null +++ b/db/migrate/20241002163937_add_failure_reason_and_processing_to_bulk_uploads.rb @@ -0,0 +1,8 @@ +class AddFailureReasonAndProcessingToBulkUploads < ActiveRecord::Migration[7.0] + def change + change_table :bulk_uploads, bulk: true do |t| + t.string :failure_reason + t.boolean :processing + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 174ae0199..684092f80 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do +ActiveRecord::Schema[7.0].define(version: 2024_10_02_163937) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -44,6 +44,8 @@ ActiveRecord::Schema[7.0].define(version: 2024_09_23_145326) do t.string "rent_type_fix_status", default: "not_applied" t.integer "organisation_id" t.integer "moved_user_id" + t.string "failure_reason" + t.boolean "processing" t.index ["identifier"], name: "index_bulk_uploads_on_identifier", unique: true t.index ["user_id"], name: "index_bulk_uploads_on_user_id" end diff --git a/spec/components/bulk_upload_summary_component_spec.rb b/spec/components/bulk_upload_summary_component_spec.rb new file mode 100644 index 000000000..9c0c68731 --- /dev/null +++ b/spec/components/bulk_upload_summary_component_spec.rb @@ -0,0 +1,148 @@ +require "rails_helper" + +RSpec.describe BulkUploadSummaryComponent, type: :component do + let(:user) { create(:user) } + let(:support_user) { create(:user, :support) } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, year: 2024, total_logs_count: 10) } + + it "shows the file name" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content(bulk_upload.filename) + end + + it "shows the collection year" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("2024/2025") + end + + it "includes a download file link" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_link("Download file", href: "/lettings-logs/bulk-uploads/#{bulk_upload.id}/download") + end + + it "shows the total log count" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("10 total logs") + end + + it "shows the uploaded by user" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Uploaded by: #{bulk_upload.user.name}") + end + + it "shows the uploading organisation" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Uploading organisation: #{bulk_upload.user.organisation.name}") + end + + it "shows the time of upload" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Time of upload: #{bulk_upload.created_at.to_formatted_s(:govuk_date_and_time)}") + end + + context "when bulk upload has only critical errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: nil) } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, total_logs_count: 10) } + + it "shows the critical errors status and error count" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Critical errors in CSV") + expect(result).to have_content("2 critical errors") + expect(result).to have_no_content("errors on important") + expect(result).to have_no_content("potential") + end + + it "includes a view error report link" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_link("View error report", href: "/lettings-logs/bulk-upload-results/#{bulk_upload.id}") + end + end + + context "when bulk upload has only potential errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "soft_validation") } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, total_logs_count: 16) } + + it "shows the potential errors status and error count" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Potential errors in CSV") + expect(result).to have_content("2 potential errors") + expect(result).to have_content("16 total logs") + expect(result).to have_no_content("errors on important") + expect(result).to have_no_content("critical") + end + end + + context "when bulk upload has only errors on important questions" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "setup") } + let(:bulk_upload) { create(:bulk_upload, :lettings, user:, bulk_upload_errors:, total_logs_count: 16) } + + it "shows the errors on important questions status and error count" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Errors on important questions in CSV") + expect(result).to have_content("2 errors on important questions") + expect(result).to have_content("16 total logs") + expect(result).to have_no_content("potential") + expect(result).to have_no_content("critical") + end + + it "includes a view error report link to the summary page" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_link("View error report", href: %r{.*/lettings-logs/bulk-upload-results/#{bulk_upload.id}/summary}) + end + end + + context "when a bulk upload is uploaded with no errors" do + let(:bulk_upload) { create(:bulk_upload, :sales, user:, total_logs_count: 1) } + + it "shows the logs uploaded with no errors status and no error counts" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Logs uploaded with no errors") + expect(result).to have_content("1 total log") + expect(result).to have_no_content("important questions") + expect(result).to have_no_content("potential") + expect(result).to have_no_content("critical") + end + end + + context "when a bulk upload is uploaded with errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 1) } + let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, total_logs_count: 21) } + + before do + create_list(:sales_log, 21, bulk_upload:) + end + + it "shows the logs upload with errors status and error count" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Logs uploaded with errors") + expect(result).to have_content("21 total logs") + expect(result).to have_content("1 critical error") + expect(result).to have_no_content("important questions") + expect(result).to have_no_content("potential") + end + end + + context "when a bulk upload uses the wrong template" do + let(:bulk_upload) { create(:bulk_upload, :sales, user:, failure_reason: "wrong_template") } + + it "shows the wrong template status and no error counts" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Wrong template") + expect(result).to have_no_content("important questions") + expect(result).to have_no_content("potential") + expect(result).to have_no_content("critical") + end + end + + context "when a bulk upload uses a blank template" do + let(:bulk_upload) { create(:bulk_upload, :sales, user:, failure_reason: "blank_template") } + + it "shows the wrong template status and no error counts" do + result = render_inline(described_class.new(bulk_upload:)) + expect(result).to have_content("Blank template") + expect(result).to have_no_content("important questions") + expect(result).to have_no_content("potential") + expect(result).to have_no_content("critical") + end + end +end diff --git a/spec/components/create_log_actions_component_spec.rb b/spec/components/create_log_actions_component_spec.rb index b1caa1443..eda25b246 100644 --- a/spec/components/create_log_actions_component_spec.rb +++ b/spec/components/create_log_actions_component_spec.rb @@ -37,13 +37,23 @@ RSpec.describe CreateLogActionsComponent, type: :component do expect(component.create_button_href).to eq("/lettings-logs") end - it "returns upload button copy" do - expect(component.upload_button_copy).to eq("Upload lettings logs in bulk") + it "does not show the upload button" do + render_inline(component) + expect(rendered_content).not_to have_link("Upload lettings logs in bulk", href: "/lettings-logs/bulk-upload-logs/start") end - it "returns upload button href" do + it "returns view uploads button copy" do + expect(component.view_uploads_button_copy).to eq("View lettings bulk uploads") + end + + it "returns view uploads button href" do render - expect(component.upload_button_href).to eq("/lettings-logs/bulk-upload-logs/start") + expect(component.view_uploads_button_href).to eq("/lettings-logs/bulk-uploads") + end + + it "shows the view uploads button" do + render_inline(component) + expect(rendered_content).to have_link("View lettings bulk uploads", href: "/lettings-logs/bulk-uploads") end context "when sales log type" do @@ -61,6 +71,16 @@ RSpec.describe CreateLogActionsComponent, type: :component do render expect(component.create_button_href).to eq("/sales-logs") end + + it "does not show the upload button" do + render_inline(component) + expect(rendered_content).not_to have_link("Upload sales logs in bulk", href: "/sales-logs/bulk-upload-logs/start") + end + + it "shows the view uploads button" do + render_inline(component) + expect(rendered_content).to have_link("View sales bulk uploads", href: "/sales-logs/bulk-uploads") + end end end diff --git a/spec/factories/bulk_upload.rb b/spec/factories/bulk_upload.rb index cefe95c2b..3c345b97a 100644 --- a/spec/factories/bulk_upload.rb +++ b/spec/factories/bulk_upload.rb @@ -10,6 +10,8 @@ FactoryBot.define do needstype { 1 } rent_type_fix_status { BulkUpload.rent_type_fix_statuses.values.sample } organisation_id { user.organisation_id } + total_logs_count { Faker::Number.number(digits: 2) } + processing { false } trait(:sales) do log_type { BulkUpload.log_types[:sales] } diff --git a/spec/factories/bulk_upload_error.rb b/spec/factories/bulk_upload_error.rb index bd2b9038c..6e2497972 100644 --- a/spec/factories/bulk_upload_error.rb +++ b/spec/factories/bulk_upload_error.rb @@ -11,5 +11,6 @@ FactoryBot.define do purchaser_code { SecureRandom.hex(4) } field { "field_#{rand(1..134)}" } error { "some error" } + category { nil } end end diff --git a/spec/features/accessibility_spec.rb b/spec/features/accessibility_spec.rb index 2a95b684e..15cd87ebb 100644 --- a/spec/features/accessibility_spec.rb +++ b/spec/features/accessibility_spec.rb @@ -110,6 +110,7 @@ RSpec.describe "Accessibility", js: true do log.save(validate: false) end allow(FormHandler.instance).to receive(:in_crossover_period?).and_return(true) + allow(storage_service).to receive(:get_presigned_url).with(bulk_upload.identifier, 60, response_content_disposition: "attachment; filename=#{bulk_upload.filename}").and_return("http://example.com/lettings-logs/bulk-uploads/#{bulk_upload.id}/download") end it "is has accessible pages" do @@ -148,6 +149,10 @@ RSpec.describe "Accessibility", js: true do }.uniq end + before do + allow(storage_service).to receive(:get_presigned_url).with(bulk_upload.identifier, 60, response_content_disposition: "attachment; filename=#{bulk_upload.filename}").and_return("http://example.com/sales-logs/bulk-uploads/#{bulk_upload.id}/download") + end + it "is has accessible pages" do sales_log_paths.each do |path| path += "?original_log_id=#{sales_log.id}" if path.include?("duplicate") diff --git a/spec/features/lettings_log_spec.rb b/spec/features/lettings_log_spec.rb index ea250529d..218850992 100644 --- a/spec/features/lettings_log_spec.rb +++ b/spec/features/lettings_log_spec.rb @@ -415,6 +415,37 @@ RSpec.describe "Lettings Log Features" do expect(deleted_log.status).to eq "deleted" expect(deleted_log.discarded_at).not_to be nil end + + context "when visiting the bulk uploads page" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: nil) } + let(:bulk_upload) { create(:bulk_upload, :lettings, user: support_user, bulk_upload_errors:, total_logs_count: 10) } + let(:mock_storage_service) { instance_double("S3Service") } + + before do + allow(Storage::S3Service).to receive(:new).and_return(mock_storage_service) + allow(mock_storage_service).to receive(:get_presigned_url).with(bulk_upload.identifier, 60, response_content_disposition: "attachment; filename=#{bulk_upload.filename}").and_return("/lettings-logs/bulk-uploads") + bulk_upload + visit("/lettings-logs/bulk-uploads") + end + + it "displays the right title" do + expect(page).to have_content("Lettings bulk uploads") + end + + it "shows the bulk upload file name" do + expect(page).to have_content(bulk_upload.filename) + end + + it "redirects to the error report page when clicking 'View error report'" do + click_link("View error report") + expect(page).to have_current_path("/lettings-logs/bulk-upload-results/#{bulk_upload.id}") + end + + it "allows the user to download the file" do + click_link("Download file", href: "/lettings-logs/bulk-uploads/#{bulk_upload.id}/download") + expect(page).to have_current_path("/lettings-logs/bulk-uploads") + end + end end context "when the signed is user is not a Support user" do diff --git a/spec/features/organisation_spec.rb b/spec/features/organisation_spec.rb index 98ae86475..6f90428be 100644 --- a/spec/features/organisation_spec.rb +++ b/spec/features/organisation_spec.rb @@ -139,7 +139,7 @@ RSpec.describe "User Features" do end it "shows a create button for that organisation" do - expect(page).to have_button("Create a new lettings log for this organisation") + expect(page).to have_button("Create a new lettings log") end it "shows a upload lettings logs in bulk link" do @@ -148,7 +148,7 @@ RSpec.describe "User Features" do context "when creating a log for that organisation" do it "pre-fills the value for owning organisation for that log" do - click_button("Create a new lettings log for this organisation") + click_button("Create a new lettings log") click_link("Set up this lettings log") expect(page).to have_content(org_name) end @@ -234,7 +234,7 @@ RSpec.describe "User Features" do end it "shows a create button for that organisation" do - expect(page).to have_button("Create a new sales log for this organisation") + expect(page).to have_button("Create a new sales log") end it "shows a upload sales logs in bulk link" do @@ -243,7 +243,7 @@ RSpec.describe "User Features" do context "when creating a log for that organisation" do it "pre-fills the value for owning organisation for that log" do - click_button("Create a new sales log for this organisation") + click_button("Create a new sales log") click_link("Set up this sales log") expect(page).to have_content(org_name) end diff --git a/spec/features/sales_log_spec.rb b/spec/features/sales_log_spec.rb index 779d978bf..8e4ffba42 100644 --- a/spec/features/sales_log_spec.rb +++ b/spec/features/sales_log_spec.rb @@ -279,6 +279,37 @@ RSpec.describe "Sales Log Features" do expect(breadcrumbs[2][:href]).to eq sales_log_path(sales_log.id) end end + + context "when visiting the bulk uploads page" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: nil) } + let(:bulk_upload) { create(:bulk_upload, :sales, user:, bulk_upload_errors:, total_logs_count: 10) } + let(:mock_storage_service) { instance_double("S3Service") } + + before do + allow(Storage::S3Service).to receive(:new).and_return(mock_storage_service) + allow(mock_storage_service).to receive(:get_presigned_url).with(bulk_upload.identifier, 60, response_content_disposition: "attachment; filename=#{bulk_upload.filename}").and_return("/sales-logs/bulk-uploads") + bulk_upload + visit("/sales-logs/bulk-uploads") + end + + it "displays the right title" do + expect(page).to have_content("Sales bulk uploads") + end + + it "shows the bulk upload file name" do + expect(page).to have_content(bulk_upload.filename) + end + + it "redirects to the error report page when clicking 'View error report'" do + click_link("View error report") + expect(page).to have_current_path("/sales-logs/bulk-upload-results/#{bulk_upload.id}") + end + + it "allows the user to download the file" do + click_link("Download file", href: "/sales-logs/bulk-uploads/#{bulk_upload.id}/download") + expect(page).to have_current_path("/sales-logs/bulk-uploads") + end + end end context "when a log becomes a duplicate" do diff --git a/spec/models/bulk_upload_spec.rb b/spec/models/bulk_upload_spec.rb index e38ea0402..af2547d7a 100644 --- a/spec/models/bulk_upload_spec.rb +++ b/spec/models/bulk_upload_spec.rb @@ -46,9 +46,215 @@ RSpec.describe BulkUpload, type: :model do let(:bulk_upload) { build(:bulk_upload, year: test_case[:year]) } it "returns the expected year combination string" do - expect(bulk_upload.year_combo).to eql(test_case[:expected_value]) + expect(bulk_upload.year_combo).to eq(test_case[:expected_value]) end end end end + + describe "scopes" do + let!(:lettings_bulk_upload_1) { create(:bulk_upload, log_type: "lettings") } + let!(:lettings_bulk_upload_2) { create(:bulk_upload, log_type: "lettings") } + let!(:sales_bulk_upload_1) { create(:bulk_upload, log_type: "sales") } + let!(:sales_bulk_upload_2) { create(:bulk_upload, log_type: "sales") } + + describe ".lettings" do + it "returns only lettings bulk uploads" do + expect(described_class.lettings).to match_array([lettings_bulk_upload_1, lettings_bulk_upload_2]) + end + end + + describe ".sales" do + it "returns only sales bulk uploads" do + expect(described_class.sales).to match_array([sales_bulk_upload_1, sales_bulk_upload_2]) + end + end + + describe ".search_by_filename" do + it "returns the correct bulk upload" do + expect(described_class.search_by_filename(lettings_bulk_upload_1.filename).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.search_by_filename(lettings_bulk_upload_1.filename).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".search_by_user_name" do + it "returns the correct bulk upload" do + expect(described_class.search_by_user_name(lettings_bulk_upload_1.user.name).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.search_by_user_name(lettings_bulk_upload_1.user.name).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".search_by_user_email" do + it "returns the correct bulk upload" do + expect(described_class.search_by_user_email(sales_bulk_upload_1.user.email).first).to eq(sales_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.search_by_user_email(sales_bulk_upload_1.user.email).first).not_to eq(sales_bulk_upload_2) + end + end + + describe ".search_by_organisation_name" do + it "returns the correct bulk upload" do + expect(described_class.search_by_organisation_name(lettings_bulk_upload_1.user.organisation.name).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.search_by_organisation_name(lettings_bulk_upload_1.user.organisation.name).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".filter_by_id" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_id(lettings_bulk_upload_1.id).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_id(lettings_bulk_upload_1.id).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".filter_by_years" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_years([lettings_bulk_upload_1.year]).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_years([lettings_bulk_upload_1.year]).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".filter_by_uploaded_by" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_uploaded_by(sales_bulk_upload_1.user.id).first).to eq(sales_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_uploaded_by(sales_bulk_upload_1.user.id).first).not_to eq(sales_bulk_upload_2) + end + end + + describe ".filter_by_user_text_search" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_user_text_search(lettings_bulk_upload_1.user.name).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_user_text_search(lettings_bulk_upload_1.user.name).first).not_to eq(lettings_bulk_upload_2) + end + end + + describe ".filter_by_user" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_user(sales_bulk_upload_1.user.id).first).to eq(sales_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_user(sales_bulk_upload_1.user.id).first).not_to eq(sales_bulk_upload_2) + end + end + + describe ".filter_by_uploading_organisation" do + it "returns the correct bulk upload" do + expect(described_class.filter_by_uploading_organisation(lettings_bulk_upload_1.user.organisation.id).first).to eq(lettings_bulk_upload_1) + end + + it "does not return the incorrect bulk upload" do + expect(described_class.filter_by_uploading_organisation(lettings_bulk_upload_1.user.organisation.id).first).not_to eq(lettings_bulk_upload_2) + end + end + end + + describe "#status" do + context "when the bulk upload was uploaded with a blank template" do + let(:bulk_upload) { create(:bulk_upload, failure_reason: "blank_template") } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:blank_template) + end + end + + context "when the bulk upload was uploaded with the wrong template" do + let(:bulk_upload) { create(:bulk_upload, failure_reason: "wrong_template") } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:wrong_template) + end + end + + context "when the bulk upload is processing" do + let(:bulk_upload) { create(:bulk_upload, processing: true) } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:processing) + end + end + + context "when the bulk upload has potential errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "soft_validation") } + let(:bulk_upload) { create(:bulk_upload, bulk_upload_errors:) } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:potential_errors) + end + end + + context "when the bulk upload has critical errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: nil) } + let(:bulk_upload) { create(:bulk_upload, bulk_upload_errors:) } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:critical_errors) + end + end + + context "when the bulk upload has important errors" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "setup") } + let(:bulk_upload) { create(:bulk_upload, bulk_upload_errors:) } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:important_errors) + end + end + + context "when the bulk upload has no errors" do + let(:bulk_upload) { create(:bulk_upload) } + + it "returns the correct status" do + expect(bulk_upload.status).to eq(:logs_uploaded_no_errors) + end + end + + context "when the bulk upload has visible logs, errors and is not complete" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "soft_validation") } + let(:bulk_upload) { create(:bulk_upload, :lettings, bulk_upload_errors:) } + + before do + create(:lettings_log, :in_progress, bulk_upload:) + end + + it "returns logs_uploaded_with_errors" do + expect(bulk_upload.status).to eq(:logs_uploaded_with_errors) + end + end + + context "when the bulk upload has visible logs, errors and is complete" do + let(:bulk_upload_errors) { create_list(:bulk_upload_error, 2, category: "soft_validation") } + let(:bulk_upload) { create(:bulk_upload, :lettings, bulk_upload_errors:) } + + before do + create(:lettings_log, :completed, bulk_upload:) + end + + it "returns errors_fixed_in_service" do + expect(bulk_upload.status).to eq(:errors_fixed_in_service) + end + end + end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index beb3d589e..89f4f9dee 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -176,6 +176,10 @@ RSpec.describe User, type: :model do it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user managing_organisation owning_organisation bulk_upload_id managing_organisation_text_search owning_organisation_text_search user_text_search]) end + + it "can filter schemes by status and owning_organisation" do + expect(user.scheme_filters).to match_array(%w[status owning_organisation owning_organisation_text_search]) + end end end @@ -217,6 +221,14 @@ RSpec.describe User, type: :model do it "can filter lettings logs by user, year, status, managing_organisation and owning_organisation" do expect(user.logs_filters).to match_array(%w[years status needstypes assigned_to user owning_organisation managing_organisation bulk_upload_id managing_organisation_text_search owning_organisation_text_search user_text_search]) end + + it "can filter bulk uploads by year, uploaded_by and uploading_organisation " do + expect(user.bulk_uploads_filters).to match_array(%w[user years uploaded_by uploading_organisation user_text_search uploading_organisation_text_search]) + end + + it "can filter schemes by status and owning_organisation" do + expect(user.scheme_filters).to match_array(%w[status owning_organisation owning_organisation_text_search]) + end end context "when the user is in development environment" do diff --git a/spec/models/validations/financial_validations_spec.rb b/spec/models/validations/financial_validations_spec.rb index 4e1aa400f..f35cdd097 100644 --- a/spec/models/validations/financial_validations_spec.rb +++ b/spec/models/validations/financial_validations_spec.rb @@ -123,6 +123,22 @@ RSpec.describe Validations::FinancialValidations do .to include(match I18n.t("validations.financial.tshortfall.more_than_total_charge")) end + it "validates that carehome charge is no less than the shortfall" do + record.hb = 6 + record.hbrentshortfall = 1 + record.tshortfall_known = 0 + record.tshortfall = 299.50 + record.chcharge = 198 + record.needstype = 2 + record.period = 2 + record.set_derived_fields! + financial_validator.validate_rent_amount(record) + expect(record.errors["chcharge"]) + .to include(match I18n.t("validations.financial.carehome.less_than_shortfall")) + expect(record.errors["tshortfall"]) + .to include(match I18n.t("validations.financial.tshortfall.more_than_carehome_charge")) + end + it "expects that rent can be less than the shortfall if total charge is higher" do record.hb = 6 record.hbrentshortfall = 1 diff --git a/spec/requests/check_errors_controller_spec.rb b/spec/requests/check_errors_controller_spec.rb index c101e7959..29130f547 100644 --- a/spec/requests/check_errors_controller_spec.rb +++ b/spec/requests/check_errors_controller_spec.rb @@ -300,6 +300,33 @@ RSpec.describe CheckErrorsController, type: :request do end end + context "and clearing ppostcode_full when previous_la_known is yes" do + let(:params) do + { + id: lettings_log.id, + lettings_log: { + layear: "1", + clear_question_ids: "ppostcode_full", + page: "time_lived_in_local_authority", + }, + check_errors: "", + } + end + + before do + lettings_log.update!(previous_la_known: 1, ppcodenk: 0, ppostcode_full: "AA11AA") + sign_in user + post "/lettings-logs/#{lettings_log.id}/time-lived-in-local-authority", params: + end + + it "clears related previous location fields" do + expect(lettings_log.reload.prevloc).to eq(nil) + expect(lettings_log.reload.previous_la_known).to eq(nil) + expect(lettings_log.reload.ppostcode_full).to eq(nil) + expect(lettings_log.reload.ppcodenk).to eq(nil) + end + end + context "and clearing specific sales question" do let(:params) do { diff --git a/spec/services/bulk_upload/downloader_spec.rb b/spec/services/bulk_upload/downloader_spec.rb index 48b046d8e..bae90aede 100644 --- a/spec/services/bulk_upload/downloader_spec.rb +++ b/spec/services/bulk_upload/downloader_spec.rb @@ -45,4 +45,16 @@ RSpec.describe BulkUpload::Downloader do expect(File).not_to exist(path) end end + + describe "#presigned_url" do + let(:mock_storage_service) { instance_double(Storage::S3Service, get_presigned_url: "https://example.com") } + + before do + allow(Storage::S3Service).to receive(:new).and_return(mock_storage_service) + end + + it "returns a presigned URL" do + expect(downloader.presigned_url).to eql("https://example.com") + end + end end diff --git a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb index 18c28189a..08109f4b9 100644 --- a/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2023/row_parser_spec.rb @@ -215,7 +215,7 @@ RSpec.describe BulkUpload::Lettings::Year2023::RowParser do field_131: "101.11", field_132: "1500.19", field_133: "1", - field_134: "234.56", + field_134: "34.56", field_27: "15", field_28: "0", diff --git a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb index 3910e1281..025d44d4a 100644 --- a/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/lettings/year2024/row_parser_spec.rb @@ -235,7 +235,7 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do field_127: "13.14", field_128: "101.11", field_129: "1", - field_130: "234.56", + field_130: "34.56", field_24: "15", field_30: now.day.to_s, @@ -782,6 +782,16 @@ RSpec.describe BulkUpload::Lettings::Year2024::RowParser do expect(parser.errors[:field_112]).to eql(["You must answer was the letting made under the Choice-Based Lettings (CBL)?"]) end end + + context "when an invalid value error has been added" do + let(:attributes) { setup_section_params.merge({ field_116: "100" }) } + + it "does not add an additional error" do + parser.valid? + expect(parser.errors[:field_116].length).to eq(1) + expect(parser.errors[:field_116]).to include(match "Enter a valid value for") + end + end end end diff --git a/spec/services/bulk_upload/processor_spec.rb b/spec/services/bulk_upload/processor_spec.rb index 08fe7d705..de0ed2dba 100644 --- a/spec/services/bulk_upload/processor_spec.rb +++ b/spec/services/bulk_upload/processor_spec.rb @@ -43,6 +43,13 @@ RSpec.describe BulkUpload::Processor do end describe "#call" do + it "changes processing from true to false" do + bulk_upload.update!(processing: true) + expect { + processor.call + }.to change { bulk_upload.reload.processing }.from(true).to(false) + end + context "when errors exist from prior job run" do let!(:existing_error) { create(:bulk_upload_error, bulk_upload:) } diff --git a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb index b74545a9a..f68ec9e9a 100644 --- a/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb +++ b/spec/services/bulk_upload/sales/year2024/row_parser_spec.rb @@ -313,6 +313,16 @@ RSpec.describe BulkUpload::Sales::Year2024::RowParser do expect(parser.errors[:field_23]).to eql(["You must answer address line 1"]) end end + + context "when an invalid value error has been added" do + let(:attributes) { setup_section_params.merge({ field_35: "100" }) } + + it "does not add an additional error" do + parser.valid? + expect(parser.errors[:field_35].length).to eq(1) + expect(parser.errors[:field_35]).to include(match "Enter a valid value for") + end + end end end diff --git a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb index 637dfa3a9..e7ffc68f8 100644 --- a/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb +++ b/spec/views/bulk_upload_lettings_results/show.html.erb_spec.rb @@ -1,10 +1,12 @@ require "rails_helper" RSpec.describe "bulk_upload_lettings_results/show.html.erb" do + let(:user) { create(:user) } let(:bulk_upload) { create(:bulk_upload, :lettings) } context "when mutiple rows in wrong order" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "C14", row: "14", col: "C") create(:bulk_upload_error, bulk_upload:, cell: "D10", row: "10", col: "D") end @@ -22,6 +24,7 @@ RSpec.describe "bulk_upload_lettings_results/show.html.erb" do context "when 1 row with 2 errors" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "AA100", row: "100", col: "AA") create(:bulk_upload_error, bulk_upload:, cell: "Z100", row: "100", col: "Z") end diff --git a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb index ac0c1f82d..5ebb05dbe 100644 --- a/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb +++ b/spec/views/bulk_upload_lettings_results/summary.html.erb_spec.rb @@ -1,10 +1,12 @@ require "rails_helper" RSpec.describe "bulk_upload_lettings_results/summary.html.erb" do + let(:user) { create(:user) } let(:bulk_upload) { create(:bulk_upload, :lettings) } context "when mutiple rows in wrong order" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "C14", row: "14", col: "C") create(:bulk_upload_error, bulk_upload:, cell: "D10", row: "10", col: "D") end @@ -22,6 +24,7 @@ RSpec.describe "bulk_upload_lettings_results/summary.html.erb" do context "when 1 row with 2 errors" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "AA100", row: "100", col: "AA") create(:bulk_upload_error, bulk_upload:, cell: "Z100", row: "100", col: "Z") end diff --git a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb index dc6751dc8..0ad13fbb3 100644 --- a/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb +++ b/spec/views/bulk_upload_sales_results/show.html.erb_spec.rb @@ -1,10 +1,12 @@ require "rails_helper" RSpec.describe "bulk_upload_sales_results/show.html.erb" do + let(:user) { create(:user) } let(:bulk_upload) { create(:bulk_upload, :sales) } context "when mutiple rows in wrong order" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "C14", row: "14", col: "C") create(:bulk_upload_error, bulk_upload:, cell: "D10", row: "10", col: "D") end @@ -22,6 +24,7 @@ RSpec.describe "bulk_upload_sales_results/show.html.erb" do context "when 1 row with 2 errors" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "AA100", row: "100", col: "AA") create(:bulk_upload_error, bulk_upload:, cell: "Z100", row: "100", col: "Z") end diff --git a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb index b3d9aa006..d999ad9d4 100644 --- a/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb +++ b/spec/views/bulk_upload_sales_results/summary.html.erb_spec.rb @@ -1,10 +1,12 @@ require "rails_helper" RSpec.describe "bulk_upload_sales_results/summary.html.erb" do + let(:user) { create(:user) } let(:bulk_upload) { create(:bulk_upload, :sales) } context "when mutiple rows in wrong order" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "C14", row: "14", col: "C") create(:bulk_upload_error, bulk_upload:, cell: "D10", row: "10", col: "D") end @@ -22,6 +24,7 @@ RSpec.describe "bulk_upload_sales_results/summary.html.erb" do context "when 1 row with 2 errors" do before do + allow(view).to receive(:current_user).and_return(user) create(:bulk_upload_error, bulk_upload:, cell: "AA100", row: "100", col: "AA") create(:bulk_upload_error, bulk_upload:, cell: "Z100", row: "100", col: "Z") end diff --git a/spec/views/logs/_create_for_org_actions.html.erb_spec.rb b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb index e82cb8d27..df448844d 100644 --- a/spec/views/logs/_create_for_org_actions.html.erb_spec.rb +++ b/spec/views/logs/_create_for_org_actions.html.erb_spec.rb @@ -14,8 +14,8 @@ RSpec.describe "logs/_create_for_org_actions.html.erb" do context "with data sharing agreement" do it "does include create log buttons" do render - expect(fragment).to have_button("Create a new lettings log for this organisation") - expect(fragment).to have_button("Create a new sales log for this organisation") + expect(fragment).to have_button("Create a new lettings log") + expect(fragment).to have_button("Create a new sales log") end end @@ -24,8 +24,8 @@ RSpec.describe "logs/_create_for_org_actions.html.erb" do it "does not include create log buttons" do render - expect(fragment).not_to have_button("Create a new lettings log for this organisation") - expect(fragment).not_to have_button("Create a new sales log for this organisation") + expect(fragment).not_to have_button("Create a new lettings log") + expect(fragment).not_to have_button("Create a new sales log") end end end