CLDC-1633 build feature csv download of sales logs (#1568)

* create a method on the FormHandler that returns the sales form questions for all years in the order that they appear in the form

* update csv email job to accomodate sales log export as well as lettings
add to tests to reflec the changes made

* write tests to cover the desired functionality of the SalesLogCsvService

* create the SalesLogCsvService
create a necessary method on the log to enable submission method to be included on the csv
derive values for the two halves of previous postcode for export

* add relevant links in the UI and pipe everything together in controllers
amend organisations controller to have flexibility to download logs of either type
add necessary methods to sales log controller, raising shared method to logs controller
update routing for amendments and additions
extract helper method to build urls for downloading logs within an organisation

* correct various linter complaints and tech review suggestions

* minor amendment to add old_id and reorder early columns

* undo my 'clever' refactor that broke things

* refactoring of csv service after some tech review and some UI testing in review app

* update tests to include a test of a full export and all values in teh csv

* correct minor routing error to ensure correct url is shown and tab selected after requesting csv email

* update organisations controller requests spec file to cover new functionality and make a minor amendment to authentication scope in the controller after error found in testing

* write request tests for the new functionality in the sales log controller, define authorisation in the controller

* minor correction after rubocop's kind suggestion'

* various corrections from first pass at PO, tech review, linter, etc

* refactor :ordered_sales_questions_for_all_years

* first pass at implementing flexible code-based form fixtures for testing

* second pass

* refactor all tests of :ordered_sales_questions_for_all_years to use new factories

* some refactoring in the testing of the csv service

* use that fact that params is always available in controllers and don't pass it around, inline some methods calls

* correct minor bug to ensure that "Return to logs" link returns to the correct index page

* remove reminder comments

* write further tests on the manipulation of questions into the csv headers, update factories of form constituents to allow the creation of forms with richer questions

* fix linter complaints

* minor alterations after rebase to account for changes made on other branches

* refactor after code review

* tweak fixtures after rebase containing alterations to the factory defaults

app/controllers/lettings_logs_controller.rb

@@ -7,11 +7,6 @@ class LettingsLogsController < LogsController
   before_action :extract_bulk_upload_from_session_filters, only: [:index]
   before_action :redirect_if_bulk_upload_resolved, only: [:index]
 
-  def authenticate_scope!
-    codes_only_export = codes_only_export?(params)
-    head :unauthorized and return unless current_user.support? || !codes_only_export
-  end
-
   def index
     respond_to do |format|
       format.html do
@@ -86,19 +81,13 @@ class LettingsLogsController < LogsController
 
   def download_csv
     unpaginated_filtered_logs = filtered_logs(current_user.lettings_logs, search_term, @session_filters)
-    codes_only = codes_only_export?(params)
-
-    render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only: }
-  end
 
-  def codes_only_export?(params)
+    render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_lettings_logs_path, codes_only: codes_only_export? }
-    params.require(:codes_only) == "true"
   end
 
   def email_csv
     all_orgs = params["organisation_select"] == "all"
-    codes_only_export = params.require(:codes_only) == "true"
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, codes_only_export?)
-    EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, codes_only_export)
     redirect_to csv_confirmation_lettings_logs_path
   end
 
@@ -124,6 +113,10 @@ class LettingsLogsController < LogsController
 
 private
 
+  def authenticate_scope!
+    head :unauthorized and return if codes_only_export? && !current_user.support?
+  end
+
   def redirect_if_bulk_upload_resolved
     if @bulk_upload && @bulk_upload.lettings_logs.in_progress.count.zero?
       redirect_to resume_bulk_upload_lettings_result_path(@bulk_upload)

app/controllers/logs_controller.rb

@@ -28,6 +28,10 @@ private
     end
   end
 
+  def codes_only_export?
+    params.require(:codes_only) == "true"
+  end
+
   def post_create_redirect_url
     raise "implement in sub class"
   end

app/controllers/organisations_controller.rb

@@ -6,8 +6,8 @@ class OrganisationsController < ApplicationController
   before_action :authenticate_user!
   before_action :find_resource, except: %i[index new create]
   before_action :authenticate_scope!, except: [:index]
-  before_action -> { session_filters(specific_org: true) }, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_csv download_csv]
+  before_action -> { session_filters(specific_org: true) }, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
-  before_action :set_session_filters, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_csv download_csv]
+  before_action :set_session_filters, if: -> { current_user.support? || current_user.organisation.has_managing_agents? }, only: %i[lettings_logs sales_logs email_lettings_csv download_lettings_csv email_sales_csv download_sales_csv]
 
   def index
     redirect_to organisation_path(current_user.organisation) unless current_user.support?
@@ -99,23 +99,23 @@ class OrganisationsController < ApplicationController
         @pagy, @logs = pagy(unpaginated_filtered_logs)
         @searched = search_term.presence
         @total_count = organisation_logs.size
+        @log_type = :lettings
         render "logs", layout: "application"
       end
     end
   end
 
-  def download_csv
+  def download_lettings_csv
     organisation_logs = LettingsLog.visible.where(owning_organisation_id: @organisation.id)
     unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters)
     codes_only = params.require(:codes_only) == "true"
 
-    render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: logs_email_csv_organisation_path, codes_only: }
+    render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: lettings_logs_email_csv_organisation_path, codes_only: }
   end
 
-  def email_csv
+  def email_lettings_csv
-    codes_only_export = params.require(:codes_only) == "true"
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export?)
-    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export)
+    redirect_to lettings_logs_csv_confirmation_organisation_path
-    redirect_to logs_csv_confirmation_organisation_path
   end
 
   def sales_logs
@@ -128,6 +128,7 @@ class OrganisationsController < ApplicationController
         @pagy, @logs = pagy(unpaginated_filtered_logs)
         @searched = search_term.presence
         @total_count = organisation_logs.size
+        @log_type = :sales
         render "logs", layout: "application"
       end
 
@@ -137,6 +138,19 @@ class OrganisationsController < ApplicationController
     end
   end
 
+  def download_sales_csv
+    organisation_logs = SalesLog.visible.where(owning_organisation_id: @organisation.id)
+    unpaginated_filtered_logs = filtered_logs(organisation_logs, search_term, @session_filters)
+    codes_only = params.require(:codes_only) == "true"
+
+    render "logs/download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: sales_logs_email_csv_organisation_path, codes_only: }
+  end
+
+  def email_sales_csv
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, false, @organisation, codes_only_export?, "sales")
+    redirect_to sales_logs_csv_confirmation_organisation_path
+  end
+
   def merge_request
     @merge_request = MergeRequest.new
   end
@@ -147,12 +161,16 @@ private
     params.require(:organisation).permit(:name, :address_line1, :address_line2, :postcode, :phone, :holds_own_stock, :provider_type, :housing_registration_no)
   end
 
+  def codes_only_export?
+    params.require(:codes_only) == "true"
+  end
+
   def search_term
     params["search"]
   end
 
   def authenticate_scope!
-    if %w[create new lettings_logs download_csv email_csv].include? action_name
+    if %w[create new lettings_logs sales_logs download_lettings_csv email_lettings_csv email_sales_csv download_sales_csv].include? action_name
       head :unauthorized and return unless current_user.support?
     elsif current_user.organisation != @organisation && !current_user.support?
       render_not_found

app/controllers/sales_logs_controller.rb

@@ -1,6 +1,7 @@
 class SalesLogsController < LogsController
   before_action :session_filters, if: :current_user, only: %i[index email_csv download_csv]
   before_action :set_session_filters, if: :current_user, only: %i[index email_csv download_csv]
+  before_action :authenticate_scope!, only: %i[download_csv email_csv]
 
   def create
     super { SalesLog.new(log_params) }
@@ -36,6 +37,20 @@ class SalesLogsController < LogsController
     end
   end
 
+  def download_csv
+    unpaginated_filtered_logs = filtered_logs(current_user.sales_logs, search_term, @session_filters)
+
+    render "download_csv", locals: { search_term:, count: unpaginated_filtered_logs.size, post_path: email_csv_sales_logs_path, codes_only: codes_only_export? }
+  end
+
+  def email_csv
+    all_orgs = params["organisation_select"] == "all"
+    EmailCsvJob.perform_later(current_user, search_term, @session_filters, all_orgs, nil, codes_only_export?, "sales")
+    redirect_to csv_confirmation_sales_logs_path
+  end
+
+  def csv_confirmation; end
+
   def post_create_redirect_url(log)
     sales_log_url(log)
   end
@@ -43,4 +58,10 @@ class SalesLogsController < LogsController
   def permitted_log_params
     params.require(:sales_log).permit(SalesLog.editable_fields)
   end
+
+private
+
+  def authenticate_scope!
+    head :unauthorized and return if codes_only_export? && !current_user.support?
+  end
 end

app/helpers/logs_helper.rb

@@ -1,10 +1,8 @@
 module LogsHelper
   def log_type_for_controller(controller)
-    case controller.class.to_s
+    case controller.class.name
-    when "LettingsLogsController"
+    when "LettingsLogsController" then "lettings"
-      "lettings"
+    when "SalesLogsController" then "sales"
-    when "SalesLogsController"
-      "sales"
     else
       raise "Log type not found for #{controller.class}"
     end
@@ -12,10 +10,8 @@ module LogsHelper
 
   def bulk_upload_path_for_controller(controller, id:)
     case log_type_for_controller(controller)
-    when "lettings"
+    when "lettings" then bulk_upload_lettings_log_path(id:)
-      bulk_upload_lettings_log_path(id:)
+    when "sales" then bulk_upload_sales_log_path(id:)
-    when "sales"
-      bulk_upload_sales_log_path(id:)
     end
   end
 
@@ -26,16 +22,29 @@ module LogsHelper
 
   def search_label_for_controller(controller)
     case log_type_for_controller(controller)
-    when "lettings"
+    when "lettings" then "Search by log ID, tenant code, property reference or postcode"
-      "Search by log ID, tenant code, property reference or postcode"
+    when "sales" then "Search by log ID, purchaser code or postcode"
-    when "sales"
-      "Search by log ID, purchaser code or postcode"
     end
   end
 
-  def csv_download_url_for_controller(controller_type:, search:, codes_only:)
+  def csv_download_url_for_controller(controller:, search:, codes_only:)
-    case log_type_for_controller(controller_type)
+    case log_type_for_controller(controller)
     when "lettings" then csv_download_lettings_logs_path(search:, codes_only:)
+    when "sales" then csv_download_sales_logs_path(search:, codes_only:)
+    end
+  end
+
+  def logs_path_for_controller(controller)
+    case log_type_for_controller(controller)
+    when "lettings" then lettings_logs_path
+    when "sales" then sales_logs_path
+    end
+  end
+
+  def csv_download_url_by_log_type(log_type, organisation, search:, codes_only:)
+    case log_type
+    when :lettings then lettings_logs_csv_download_organisation_path(organisation, search:, codes_only:)
+    when :sales then sales_logs_csv_download_organisation_path(organisation, search:, codes_only:)
     end
   end
 end

app/jobs/email_csv_job.rb

@@ -5,14 +5,23 @@ class EmailCsvJob < ApplicationJob
 
   EXPIRATION_TIME = 3.hours.to_i
 
-  def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false) # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
+  def perform(user, search_term = nil, filters = {}, all_orgs = false, organisation = nil, codes_only_export = false, log_type = "lettings") # rubocop:disable Style/OptionalBooleanParameter - sidekiq can't serialise named params
-    unfiltered_logs = organisation.present? && user.support? ? LettingsLog.visible.where(owning_organisation_id: organisation.id) : user.lettings_logs.visible
+    case log_type
-    filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
+    when "lettings"
+      unfiltered_logs = organisation.present? && user.support? ? LettingsLog.visible.where(owning_organisation_id: organisation.id) : user.lettings_logs.visible
+      filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
+      csv_string = filtered_logs.to_csv(user, codes_only_export:)
+    when "sales"
+      unfiltered_logs = organisation.present? && user.support? ? SalesLog.visible.where(owning_organisation_id: organisation.id) : user.sales_logs.visible
+      filtered_logs = FilterService.filter_logs(unfiltered_logs, search_term, filters, all_orgs, user)
+      export_type = codes_only_export ? "codes" : "labels"
+      csv_string = Csv::SalesLogCsvService.new(export_type:).prepare_csv(filtered_logs)
+    end
 
-    filename = organisation.present? ? "logs-#{organisation.name}-#{Time.zone.now}.csv" : "logs-#{Time.zone.now}.csv"
+    filename = "#{[log_type, 'logs', organisation&.name, Time.zone.now].compact.join('-')}.csv"
 
     storage_service = Storage::S3Service.new(Configuration::EnvConfigurationService.new, ENV["CSV_DOWNLOAD_PAAS_INSTANCE"])
-    storage_service.write_file(filename, BYTE_ORDER_MARK + filtered_logs.to_csv(user, codes_only_export:))
+    storage_service.write_file(filename, BYTE_ORDER_MARK + csv_string)
 
     url = storage_service.get_presigned_url(filename, EXPIRATION_TIME)
 

app/models/derived_variables/sales_log_variables.rb

@@ -17,7 +17,8 @@ module DerivedVariables::SalesLogVariables
       self.hoyear = hodate.year
     end
     self.deposit = value if outright_sale? && mortgage_not_used?
-    self.pcode1, self.pcode2 = postcode_full.split(" ") if postcode_full.present?
+    self.pcode1, self.pcode2 = postcode_full.split if postcode_full.present?
+    self.ppostc1, self.ppostc2 = ppostcode_full.split if ppostcode_full.present?
     self.totchild = total_child
     self.totadult = total_adult + total_elder
     self.hhmemb = number_of_household_members

app/models/form_handler.rb

@@ -40,6 +40,28 @@ class FormHandler
     }
   end
 
+  def ordered_sales_questions_for_all_years
+    sales_forms = forms.filter { |name, _form| name.end_with? "sales" }.values
+    ordered_questions = sales_forms.pop.questions.uniq(&:id)
+    question_ids = ordered_questions.map(&:id)
+    all_questions_from_previous_forms = sales_forms.flat_map(&:questions)
+    deprecated_questions_by_preceding_question_id(question_ids, all_questions_from_previous_forms).each do |preceding_question_id, deprecated_question|
+      index_of_preceding_question = ordered_questions.index { |q| q.id == preceding_question_id }
+      ordered_questions.insert(index_of_preceding_question + 1, deprecated_question)
+    end
+    ordered_questions
+  end
+
+  def deprecated_questions_by_preceding_question_id(current_form_question_ids, all_questions_from_previous_forms)
+    deprecated_questions = {}
+    all_questions_from_previous_forms.each_cons(2) do |preceding_question, question|
+      next if current_form_question_ids.include?(question.id) || deprecated_questions.values.map(&:id).include?(question.id)
+
+      deprecated_questions[preceding_question.id] = question
+    end
+    deprecated_questions
+  end
+
   def lettings_forms
     forms = {}
     directories.each do |directory|
@@ -95,9 +117,9 @@ class FormHandler
     forms.count { |form| now.between?(form.start_date, form.end_date) } > 1
   end
 
-  def use_fake_forms!
+  def use_fake_forms!(fake_forms = nil)
     @directories = ["spec/fixtures/forms"]
-    @forms = get_all_forms
+    @forms = fake_forms || get_all_forms
   end
 
   def use_real_forms!

app/models/log.rb

@@ -136,6 +136,10 @@ class Log < ApplicationRecord
     format_as_currency(field_value)
   end
 
+  def creation_method
+    bulk_upload_id ? "bulk upload" : "single log"
+  end
+
 private
 
   # Handle logs that are older than previous collection start date

app/services/csv/sales_log_csv_service.rb

@@ -0,0 +1,107 @@
+module Csv
+  class SalesLogCsvService
+    def initialize(export_type:)
+      @export_type = export_type
+      @attributes = sales_log_attributes
+    end
+
+    def prepare_csv(logs)
+      CSV.generate(headers: true) do |csv|
+        csv << @attributes
+
+        logs.find_each do |log|
+          csv << @attributes.map { |attribute| value(attribute, log) }
+        end
+      end
+    end
+
+  private
+
+    ATTRIBUTES_OF_RELATED_OBJECTS = {
+      day: %i[saledate day],
+      month: %i[saledate month],
+      year: %i[saledate year],
+      is_dpo: %i[created_by is_dpo],
+      created_by_name: %i[created_by name],
+      owning_organisation_name: %i[owning_organisation name],
+    }.freeze
+
+    FIELDS_ALWAYS_EXPORTED_AS_CODES = %w[
+      la
+      prevloc
+    ].freeze
+
+    FIELDS_ALWAYS_EXPORTED_AS_LABELS = {
+      "la_label" => "la",
+      "prevloc_label" => "prevloc",
+    }.freeze
+
+    DATE_FIELDS = %w[
+      created_at
+      updated_at
+    ].freeze
+
+    def value(attribute, log)
+      if ATTRIBUTES_OF_RELATED_OBJECTS.key? attribute.to_sym
+        call_chain = ATTRIBUTES_OF_RELATED_OBJECTS[attribute.to_sym]
+        call_chain.reduce(log) { |object, next_call| object&.public_send(next_call) }
+      elsif FIELDS_ALWAYS_EXPORTED_AS_CODES.include? attribute
+        log.send(attribute)
+      elsif FIELDS_ALWAYS_EXPORTED_AS_LABELS.key? attribute
+        attribute = FIELDS_ALWAYS_EXPORTED_AS_LABELS[attribute]
+        field_value = log.send(attribute)
+        get_label(field_value, attribute, log)
+      elsif DATE_FIELDS.include? attribute
+        log.send(attribute)&.iso8601
+      else
+        value = log.public_send(attribute)
+        case @export_type
+        when "codes"
+          value
+        when "labels"
+          answer_label = get_label(value, attribute, log)
+          answer_label || label_if_boolean_value(value) || value
+        end
+      end
+    end
+
+    def get_label(value, attribute, log)
+      log.form
+         .get_question(attribute, log)
+         &.label_from_value(value)
+    end
+
+    def label_if_boolean_value(value)
+      return "Yes" if value == true
+      return "No" if value == false
+    end
+
+    ATTRIBUTE_MAPPINGS = {
+      "saledate" => %w[day month year],
+      "exdate" => %w[exday exmonth exyear],
+      "hodate" => %w[hoday homonth hoyear],
+      "postcode_full" => %w[pcode1 pcode2],
+      "ppostcode_full" => %w[ppostc1 ppostc2],
+      "la" => %w[la la_label],
+      "prevloc" => %w[prevloc prevloc_label],
+      "created_by_id" => %w[created_by_name],
+      "owning_organisation_id" => %w[owning_organisation_name],
+    }.freeze
+
+    def sales_log_attributes
+      ordered_questions = FormHandler.instance.ordered_sales_questions_for_all_years
+      ordered_questions.reject! { |q| q.id.match?(/((?<!la)_known)|(_check)|(_asked)/) }
+      attributes = ordered_questions.flat_map do |question|
+        if question.type == "checkbox"
+          question.answer_options.keys
+        elsif ATTRIBUTE_MAPPINGS.key? question.id
+          ATTRIBUTE_MAPPINGS[question.id]
+        else
+          question.id
+        end
+      end
+      non_question_fields = %w[id status created_at updated_at old_id collection_start_year creation_method is_dpo]
+      non_question_fields + attributes
+    end
+  end
+end

app/views/logs/_log_list.html.erb

@@ -1,6 +1,6 @@
 <h2 class="govuk-body">
   <%= render(SearchResultCaptionComponent.new(searched:, count: pagy.count, item_label:, total_count:, item: "logs", path: request.path)) %>
-  <% if logs&.first&.lettings? %>
+  <% if logs&.any? %>
     <%= govuk_link_to "Download (CSV)", csv_download_url, type: "text/csv", class: "govuk-!-margin-right-4" %>
     <% if @current_user.support? %>
       <%= govuk_link_to "Download (CSV, codes only)", csv_codes_only_download_url, type: "text/csv" %>

app/views/logs/csv_confirmation.html.erb

@@ -9,7 +9,7 @@
     <p class="govuk-body">Open your email inbox and click the link to download your CSV file.</p>
 
     <p class="govuk-body">
-      <%= govuk_link_to "Return to logs", lettings_logs_path %>
+      <%= govuk_link_to "Return to logs", logs_path_for_controller(controller) %>
     </p>
   </div>
 </div>

app/views/logs/index.html.erb

@@ -78,8 +78,8 @@
                  searched: @searched,
                  item_label:,
                  total_count: @total_count,
-                 csv_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: false),
+                 csv_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: false),
-                 csv_codes_only_download_url: csv_download_url_for_controller(controller_type: controller, search: @search_term, codes_only: true),
+                 csv_codes_only_download_url: csv_download_url_for_controller(controller:, search: @search_term, codes_only: true),
                } %>
     <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
   </div>

app/views/organisations/logs.html.erb

@@ -34,8 +34,8 @@
                  searched: @searched,
                  item_label:,
                  total_count: @total_count,
-                 csv_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term, codes_only: false),
+                 csv_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: false),
-                 csv_codes_only_download_url: logs_csv_download_organisation_path(@organisation, search: @search_term, codes_only: true),
+                 csv_codes_only_download_url: csv_download_url_by_log_type(@log_type, @organisation, search: @search_term, codes_only: true),
                } %>
     <%== render partial: "pagy/nav", locals: { pagy: @pagy, item_name: "logs" } %>
   </div>

config/routes.rb

@@ -113,9 +113,12 @@ Rails.application.routes.draw do
       get "users/invite", to: "users/account#new"
       get "lettings-logs", to: "organisations#lettings_logs"
       get "sales-logs", to: "organisations#sales_logs"
-      get "logs/csv-download", to: "organisations#download_csv"
+      get "lettings-logs/csv-download", to: "organisations#download_lettings_csv"
-      post "logs/email-csv", to: "organisations#email_csv"
+      post "lettings-logs/email-csv", to: "organisations#email_lettings_csv"
-      get "logs/csv-confirmation", to: "lettings_logs#csv_confirmation"
+      get "lettings-logs/csv-confirmation", to: "lettings_logs#csv_confirmation"
+      get "sales-logs/csv-download", to: "organisations#download_sales_csv"
+      post "sales-logs/email-csv", to: "organisations#email_sales_csv"
+      get "sales-logs/csv-confirmation", to: "sales_logs#csv_confirmation"
       get "schemes", to: "organisations#schemes"
       get "stock-owners", to: "organisation_relationships#stock_owners"
       get "stock-owners/add", to: "organisation_relationships#add_stock_owner"
@@ -198,6 +201,10 @@ Rails.application.routes.draw do
 
   resources :sales_logs, path: "/sales-logs" do
     collection 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"
+
       resources :bulk_upload_sales_logs, path: "bulk-upload-logs" do
         collection do
           get :start

spec/factories/form.rb

@@ -0,0 +1,25 @@
+class FormFixture < Form
+  attr_accessor :sections, :subsections, :pages, :questions
+end
+
+class FormFactory
+  def initialize(year:, type:)
+    @year = year
+    @type = type
+  end
+
+  def with_sections(sections)
+    @sections = sections
+    self
+  end
+
+  def build
+    form = FormFixture.new(nil, @year, [], @type)
+    @sections.each { |section| section.form = form }
+    form.sections = @sections
+    form.subsections = form.sections.flat_map(&:subsections)
+    form.pages = form.subsections.flat_map(&:pages)
+    form.questions = form.pages.flat_map(&:questions)
+    form
+  end
+end

spec/factories/page.rb

@@ -0,0 +1,21 @@
+FactoryBot.define do
+  factory :page, class: "Form::Page" do
+    id { "page_id" }
+    initialize_with { new(id, nil, nil) }
+    trait :with_question do
+      transient do
+        question_id { nil }
+        question { nil }
+      end
+
+      after :build do |page, evaluator|
+        page.questions = if (q = evaluator.question)
+                           q.page = page
+                           [q]
+                         else
+                           [build(:question, id: evaluator.question_id, page:)]
+                         end
+      end
+    end
+  end
+end

spec/factories/question.rb

@@ -0,0 +1,6 @@
+FactoryBot.define do
+  factory :question, class: "Form::Question" do
+    initialize_with { new(id, nil, nil) }
+    type { "text" }
+  end
+end

spec/factories/sales_log.rb

@@ -120,7 +120,8 @@ FactoryBot.define do
       has_mscharge { 1 }
       mscharge { 100 }
       mortlen { 10 }
-      pcodenk { 1 }
+      pcodenk { 0 }
+      postcode_full { "SW1A 1AA" }
       is_la_inferred { false }
       mortgagelender { 5 }
       extrabor { 1 }

spec/factories/section.rb

@@ -0,0 +1,16 @@
+FactoryBot.define do
+  factory :section, class: "Form::Section" do
+    id { "section_id" }
+    initialize_with { new(id, nil, nil) }
+    trait :with_questions do
+      transient do
+        question_ids { nil }
+        questions { nil }
+      end
+
+      after :build do |section, evaluator|
+        section.subsections = [build(:subsection, :with_questions, question_ids: evaluator.question_ids, questions: evaluator.questions, section:)]
+      end
+    end
+  end
+end

spec/factories/subsection.rb

@@ -0,0 +1,20 @@
+FactoryBot.define do
+  factory :subsection, class: "Form::Subsection" do
+    id { "subsection_id" }
+    initialize_with { new(id, nil, nil) }
+    trait :with_questions do
+      transient do
+        question_ids { nil }
+        questions { nil }
+      end
+
+      after :build do |subsection, evaluator|
+        subsection.pages = if evaluator.questions
+                             evaluator.questions.map { |question| build(:page, :with_question, question:, subsection:) }
+                           else
+                             evaluator.question_ids.map { |question_id| build(:page, :with_question, question_id:, subsection:) }
+                           end
+      end
+    end
+  end
+end

spec/fixtures/files/sales_logs_csv_export_codes.csv

@@ -0,0 +1,2 @@
+id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by_name,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant
+,completed,2023-02-08T16:52:15+00:00,2023-02-08T16:52:15+00:00,,2022,single log,false,DLUHC,Danny Rojas,2,2,2023,,2,8,,,,1,1,2,1,1,0,,,,,,,SW1A,1AA,1,E09000003,Barnet,1,2,1,30,X,17,17,18,1,1,P,35,X,,,,1,1,1,C,14,X,9,X,18,X,3,R,40,X,2,R,40,X,1,1,1,,,0,,,1,1,1,1,,,,1,4,5,1,1,0,10000,1,0,10000,1,4,1,,1,2,10,,,,,,,,,,,,,,,,,110000.0,,1,20000.0,5,,10,1,80000.0,1000.0,,1,100.0,,10000.0

spec/fixtures/files/sales_logs_csv_export_labels.csv

@@ -0,0 +1,2 @@
+id,status,created_at,updated_at,old_id,collection_start_year,creation_method,is_dpo,owning_organisation_name,created_by_name,day,month,year,purchid,ownershipsch,type,othtype,companybuy,buylivein,jointpur,jointmore,beds,proptype,builtype,pcodenk,uprn,uprn_confirmed,address_line1,address_line2,town_or_city,county,pcode1,pcode2,la_known,la,la_label,wchair,noint,privacynotice,age1,sex1,ethnic_group,ethnic,national,ecstat1,buy1livein,relat2,age2,sex2,ethnic_group2,ethnicbuy2,nationalbuy2,ecstat2,buy2livein,hholdcount,relat3,age3,sex3,ecstat3,relat4,age4,sex4,ecstat4,relat5,age5,sex5,ecstat5,relat6,age6,sex6,ecstat6,prevten,ppcodenk,ppostc1,ppostc2,previous_la_known,prevloc,prevloc_label,pregyrha,pregother,pregla,pregghb,pregblank,buy2living,prevtenbuy2,hhregres,hhregresstill,armedforcesspouse,disabled,wheel,income1nk,income1,inc1mort,income2nk,income2,inc2mort,hb,savingsnk,savings,prevown,prevshared,proplen,staircase,stairbought,stairowned,staircasesale,resale,exday,exmonth,exyear,hoday,homonth,hoyear,lanomagr,soctenant,frombeds,fromprop,socprevten,value,equity,mortgageused,mortgage,mortgagelender,mortgagelenderother,mortlen,extrabor,deposit,cashdis,mrent,has_mscharge,mscharge,discount,grant
+,completed,2023-02-08T16:52:15+00:00,2023-02-08T16:52:15+00:00,,2022,single log,false,DLUHC,Danny Rojas,2,2,2023,,Yes - a discounted ownership scheme,Right to Acquire (RTA),,,,Yes,Yes,2,Flat or maisonette,Purpose built,Yes,,,,,,,SW1A,1AA,Yes,E09000003,Barnet,Yes,Yes,1,30,Non-binary,Buyer 1 prefers not to say,17,United Kingdom,Full-time - 30 hours or more,Yes,Partner,35,Non-binary,,,,Full-time - 30 hours or more,Yes,1,Child,14,Non-binary,Child under 16,Other,18,Non-binary,"In government training into work, such as New Deal",Person prefers not to say,40,Non-binary,Part-time - Less than 30 hours,Person prefers not to say,40,Non-binary,Full-time - 30 hours or more,Local authority tenant,No,,,No,,,1,1,1,1,,,,Yes,Yes,No,Yes,Yes,Yes,10000,Yes,Yes,10000,Yes,Don’t know ,No,,Yes,2,10,,,,,,,,,,,,,,,,,110000.0,,Yes,20000.0,Cambridge Building Society,,10,Yes,80000.0,1000.0,,Yes,100.0,,10000.0

spec/jobs/email_csv_job_spec.rb

@@ -26,6 +26,7 @@ describe EmailCsvJob do
 
     let(:storage_service) { instance_double(Storage::S3Service) }
     let(:mailer) { instance_double(CsvDownloadMailer) }
+    let(:sales_log_csv_service) { instance_double(Csv::SalesLogCsvService) }
 
     before do
       FactoryBot.create(:lettings_log,
@@ -39,18 +40,44 @@ describe EmailCsvJob do
       allow(storage_service).to receive(:write_file)
       allow(storage_service).to receive(:get_presigned_url).and_return(test_url)
 
+      allow(Csv::SalesLogCsvService).to receive(:new).and_return(sales_log_csv_service)
+      allow(sales_log_csv_service).to receive(:prepare_csv).and_return("")
+
       allow(CsvDownloadMailer).to receive(:new).and_return(mailer)
       allow(mailer).to receive(:send_csv_download_mail)
     end
 
-    it "uses an appropriate filename in S3" do
+    context "when exporting lettings logs" do
-      expect(storage_service).to receive(:write_file).with(/logs-.*\.csv/, anything)
+      it "uses an appropriate filename in S3" do
-      job.perform(user)
+        expect(storage_service).to receive(:write_file).with(/lettings-logs-.*\.csv/, anything)
+        job.perform(user)
+      end
+
+      it "includes the organisation name in the filename when one is provided" do
+        expect(storage_service).to receive(:write_file).with(/lettings-logs-#{organisation.name}-.*\.csv/, anything)
+        job.perform(user, nil, {}, nil, organisation)
+      end
     end
 
-    it "includes the organisation name in the filename when one is provided" do
+    context "when exporting sales logs" do
-      expect(storage_service).to receive(:write_file).with(/logs-#{organisation.name}-.*\.csv/, anything)
+      it "uses an appropriate filename in S3" do
-      job.perform(user, nil, {}, nil, organisation)
+        expect(storage_service).to receive(:write_file).with(/sales-logs-.*\.csv/, anything)
+        job.perform(user, nil, {}, nil, nil, nil, "sales")
+      end
+
+      it "includes the organisation name in the filename when one is provided" do
+        expect(storage_service).to receive(:write_file).with(/sales-logs-#{organisation.name}-.*\.csv/, anything)
+        job.perform(user, nil, {}, nil, organisation, nil, "sales")
+      end
+
+      it "creates a SalesLogCsvService with the correct export type" do
+        expect(Csv::SalesLogCsvService).to receive(:new).with(export_type: "labels")
+        codes_only = false
+        job.perform(user, nil, {}, nil, nil, codes_only, "sales")
+        expect(Csv::SalesLogCsvService).to receive(:new).with(export_type: "codes")
+        codes_only = true
+        job.perform(user, nil, {}, nil, nil, codes_only, "sales")
+      end
     end
 
     it "sends an E-mail with the presigned URL and duration" do

spec/models/form_handler_spec.rb

@@ -219,5 +219,56 @@ RSpec.describe FormHandler do
       end
     end
   end
+
+  describe "#ordered_sales_questions_for_all_years" do
+    let(:result) { described_class.instance.ordered_sales_questions_for_all_years }
+    let(:now) { Time.zone.now }
+
+    it "returns an array of questions" do
+      section = build(:section, :with_questions, question_ids: %w[1 2 3])
+      sales_form = FormFactory.new(year: 1936, type: "sales")
+                              .with_sections([section])
+                              .build
+      described_class.instance.use_fake_forms!({ only_sales: sales_form })
+      expect(result).to(satisfy { |result| result.all? { |element| element.is_a?(Form::Question) } })
+    end
+
+    it "does not return multiple questions with the same id" do
+      first_section = build(:section, :with_questions, question_ids: %w[1 2 3])
+      second_section = build(:section, :with_questions, question_ids: %w[2 3 4 5])
+      sales_form = FormFactory.new(year: 1936, type: "sales")
+                              .with_sections([first_section, second_section])
+                              .build
+      described_class.instance.use_fake_forms!({ only_sales: sales_form })
+      expect(result.map(&:id)).to eq %w[1 2 3 4 5]
+    end
+
+    it "returns the questions in the same order as the form" do
+      first_section = build(:section, :with_questions, question_ids: %w[1 2 3])
+      second_section = build(:section, :with_questions, question_ids: %w[4 5 6])
+      sales_form = FormFactory.new(year: 1945, type: "sales")
+                              .with_sections([first_section, second_section])
+                              .build
+      described_class.instance.use_fake_forms!({ only_sales: sales_form })
+      expect(result.map(&:id)).to eq %w[1 2 3 4 5 6]
+    end
+
+    it "inserts questions from all years in their correct positions" do
+      original_section = build(:section, :with_questions, question_ids: %w[1 1a_deprecated 2 3])
+      new_section = build(:section, :with_questions, question_ids: %w[1 2 2a_new 3])
+      original_form = FormFactory.new(year: 1066, type: "sales")
+                                 .with_sections([original_section])
+                                 .build
+      new_form = FormFactory.new(year: 1485, type: "sales")
+                            .with_sections([new_section])
+                            .build
+      fake_forms = {
+        earlier_sales: original_form,
+        newer_sales: new_form,
+      }
+      described_class.instance.use_fake_forms!(fake_forms)
+      expect(result.map(&:id)).to eq %w[1 1a_deprecated 2 2a_new 3]
+    end
+  end
   # rubocop:enable RSpec/PredicateMatcher
 end

spec/requests/organisations_controller_spec.rb

@@ -367,7 +367,7 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "when viewing logs for other organisation" do
+      context "when viewing lettings logs for other organisation" do
         it "does not display the lettings logs" do
           get "/organisations/#{unauthorised_organisation.id}/lettings-logs", headers:, params: {}
           expect(response).to have_http_status(:unauthorized)
@@ -375,13 +375,13 @@ RSpec.describe OrganisationsController, type: :request do
 
         it "prevents CSV download" do
           expect {
-            post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {}
+            post "/organisations/#{unauthorised_organisation.id}/lettings-logs/email-csv", headers:, params: {}
           }.not_to enqueue_job(EmailCsvJob)
           expect(response).to have_http_status(:unauthorized)
         end
       end
 
-      context "when viewing logs for your organisation" do
+      context "when viewing lettings logs for your organisation" do
         it "does not display the logs" do
           get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {}
           expect(response).to have_http_status(:unauthorized)
@@ -389,7 +389,35 @@ RSpec.describe OrganisationsController, type: :request do
 
         it "prevents CSV download" do
           expect {
-            post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {}
+            post "/organisations/#{organisation.id}/lettings-logs/email-csv", headers:, params: {}
+          }.not_to enqueue_job(EmailCsvJob)
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "when viewing sales logs for other organisation" do
+        it "does not display the sales logs" do
+          get "/organisations/#{unauthorised_organisation.id}/sales-logs", headers:, params: {}
+          expect(response).to have_http_status(:unauthorized)
+        end
+
+        it "prevents CSV download" do
+          expect {
+            post "/organisations/#{unauthorised_organisation.id}/sales-logs/email-csv", headers:, params: {}
+          }.not_to enqueue_job(EmailCsvJob)
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "when viewing sales logs for your organisation" do
+        it "does not display the logs" do
+          get "/organisations/#{organisation.id}/sales-logs", headers:, params: {}
+          expect(response).to have_http_status(:unauthorized)
+        end
+
+        it "prevents CSV download" do
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv", headers:, params: {}
           }.not_to enqueue_job(EmailCsvJob)
           expect(response).to have_http_status(:unauthorized)
         end
@@ -544,7 +572,7 @@ RSpec.describe OrganisationsController, type: :request do
         end
       end
 
-      context "when viewing logs for other organisation" do
+      context "when viewing lettings logs for other organisation" do
         it "does not display the logs" do
           get "/organisations/#{unauthorised_organisation.id}/lettings-logs", headers:, params: {}
           expect(response).to have_http_status(:unauthorized)
@@ -552,13 +580,13 @@ RSpec.describe OrganisationsController, type: :request do
 
         it "prevents CSV download" do
           expect {
-            post "/organisations/#{unauthorised_organisation.id}/logs/email-csv", headers:, params: {}
+            post "/organisations/#{unauthorised_organisation.id}/lettings-logs/email-csv", headers:, params: {}
           }.not_to enqueue_job(EmailCsvJob)
           expect(response).to have_http_status(:unauthorized)
         end
       end
 
-      context "when viewing logs for your organisation" do
+      context "when viewing lettings logs for your organisation" do
         it "does not display the logs" do
           get "/organisations/#{organisation.id}/lettings-logs", headers:, params: {}
           expect(response).to have_http_status(:unauthorized)
@@ -566,7 +594,35 @@ RSpec.describe OrganisationsController, type: :request do
 
         it "prevents CSV download" do
           expect {
-            post "/organisations/#{organisation.id}/logs/email-csv", headers:, params: {}
+            post "/organisations/#{organisation.id}/lettings-logs/email-csv", headers:, params: {}
+          }.not_to enqueue_job(EmailCsvJob)
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "when viewing sales logs for other organisation" do
+        it "does not display the logs" do
+          get "/organisations/#{unauthorised_organisation.id}/sales-logs", headers:, params: {}
+          expect(response).to have_http_status(:unauthorized)
+        end
+
+        it "prevents CSV download" do
+          expect {
+            post "/organisations/#{unauthorised_organisation.id}/sales-logs/email-csv", headers:, params: {}
+          }.not_to enqueue_job(EmailCsvJob)
+          expect(response).to have_http_status(:unauthorized)
+        end
+      end
+
+      context "when viewing sales logs for your organisation" do
+        it "does not display the logs" do
+          get "/organisations/#{organisation.id}/sales-logs", headers:, params: {}
+          expect(response).to have_http_status(:unauthorized)
+        end
+
+        it "prevents CSV download" do
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv", headers:, params: {}
           }.not_to enqueue_job(EmailCsvJob)
           expect(response).to have_http_status(:unauthorized)
         end
@@ -1176,15 +1232,15 @@ RSpec.describe OrganisationsController, type: :request do
       sign_in user
     end
 
-    context "when they view the logs tab" do
+    context "when they view the lettings logs tab" do
       before do
         FactoryBot.create(:lettings_log, owning_organisation: organisation)
       end
 
       it "has CSV download buttons with the correct paths if at least 1 log exists" do
         get "/organisations/#{organisation.id}/lettings-logs"
-        expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/logs/csv-download?codes_only=false")
+        expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=false")
-        expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/logs/csv-download?codes_only=true")
+        expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=true")
       end
 
       context "when you download the CSV" do
@@ -1197,62 +1253,151 @@ RSpec.describe OrganisationsController, type: :request do
         end
 
         it "only includes logs from that organisation" do
-          get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false"
+          get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=false"
 
           expect(page).to have_text("You've selected 3 logs.")
         end
 
         it "provides the organisation to the mail job" do
           expect {
-            post "/organisations/#{organisation.id}/logs/email-csv?status[]=completed&codes_only=false", headers:, params: {}
+            post "/organisations/#{organisation.id}/lettings-logs/email-csv?status[]=completed&codes_only=false", headers:, params: {}
           }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false)
         end
 
         it "provides the export type to the mail job" do
           codes_only_export_type = false
           expect {
-            post "/organisations/#{organisation.id}/logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
+            post "/organisations/#{organisation.id}/lettings-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
           }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type)
           codes_only_export_type = true
           expect {
-            post "/organisations/#{organisation.id}/logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
+            post "/organisations/#{organisation.id}/lettings-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
           }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type)
         end
       end
     end
 
-    describe "GET #download_csv" do
+    context "when they view the sales logs tab" do
+      before do
+        FactoryBot.create(:sales_log, owning_organisation: organisation)
+      end
+
+      it "has CSV download buttons with the correct paths if at least 1 log exists" do
+        get "/organisations/#{organisation.id}/sales-logs"
+        expect(page).to have_link("Download (CSV)", href: "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=false")
+        expect(page).to have_link("Download (CSV, codes only)", href: "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=true")
+      end
+
+      context "when you download the CSV" do
+        let(:other_organisation) { FactoryBot.create(:organisation) }
+
+        before do
+          FactoryBot.create_list(:sales_log, 2, owning_organisation: organisation)
+          FactoryBot.create(:sales_log, owning_organisation: organisation, status: "pending", skip_update_status: true)
+          FactoryBot.create_list(:sales_log, 2, owning_organisation: other_organisation)
+        end
+
+        it "only includes logs from that organisation" do
+          get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=false"
+
+          expect(page).to have_text("You've selected 3 logs.")
+        end
+
+        it "provides the organisation to the mail job" do
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv?status[]=completed&codes_only=false", headers:, params: {}
+          }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false, "sales")
+        end
+
+        it "provides the log type to the mail job" do
+          log_type = "sales"
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv?status[]=completed&codes_only=false", headers:, params: {}
+          }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, organisation, false, log_type)
+        end
+
+        it "provides the export type to the mail job" do
+          codes_only_export_type = false
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
+          }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type, "sales")
+          codes_only_export_type = true
+          expect {
+            post "/organisations/#{organisation.id}/sales-logs/email-csv?codes_only=#{codes_only_export_type}", headers:, params: {}
+          }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, organisation, codes_only_export_type, "sales")
+        end
+      end
+    end
+
+    describe "GET #download_lettings_csv" do
+      it "renders a page with the correct header" do
+        get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=false", headers:, params: {}
+        header = page.find_css("h1")
+        expect(header.text).to include("Download CSV")
+      end
+
+      it "renders a form with the correct target containing a button with the correct text" do
+        get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=false", headers:, params: {}
+        form = page.find("form.button_to")
+        expect(form[:method]).to eq("post")
+        expect(form[:action]).to eq("/organisations/#{organisation.id}/lettings-logs/email-csv")
+        expect(form).to have_button("Send email")
+      end
+
+      it "when codes_only query parameter is false, form contains hidden field with correct value" do
+        codes_only = false
+        get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
+        hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden")
+        expect(hidden_field.value).to eq(codes_only.to_s)
+      end
+
+      it "when codes_only query parameter is true, form contains hidden field with correct value" do
+        codes_only = true
+        get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
+        hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden")
+        expect(hidden_field.value).to eq(codes_only.to_s)
+      end
+
+      it "when query string contains search parameter, form contains hidden field with correct value" do
+        search_term = "blam"
+        get "/organisations/#{organisation.id}/lettings-logs/csv-download?codes_only=true&search=#{search_term}", headers:, params: {}
+        hidden_field = page.find("form.button_to").find_field("search", type: "hidden")
+        expect(hidden_field.value).to eq(search_term)
+      end
+    end
+
+    describe "GET #download_sales_csv" do
       it "renders a page with the correct header" do
-        get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false", headers:, params: {}
+        get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=false", headers:, params: {}
         header = page.find_css("h1")
         expect(header.text).to include("Download CSV")
       end
 
       it "renders a form with the correct target containing a button with the correct text" do
-        get "/organisations/#{organisation.id}/logs/csv-download?codes_only=false", headers:, params: {}
+        get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=false", headers:, params: {}
         form = page.find("form.button_to")
         expect(form[:method]).to eq("post")
-        expect(form[:action]).to eq("/organisations/#{organisation.id}/logs/email-csv")
+        expect(form[:action]).to eq("/organisations/#{organisation.id}/sales-logs/email-csv")
         expect(form).to have_button("Send email")
       end
 
       it "when codes_only query parameter is false, form contains hidden field with correct value" do
         codes_only = false
-        get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
+        get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
         hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden")
         expect(hidden_field.value).to eq(codes_only.to_s)
       end
 
       it "when codes_only query parameter is true, form contains hidden field with correct value" do
         codes_only = true
-        get "/organisations/#{organisation.id}/logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
+        get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=#{codes_only}", headers:, params: {}
         hidden_field = page.find("form.button_to").find_field("codes_only", type: "hidden")
         expect(hidden_field.value).to eq(codes_only.to_s)
       end
 
       it "when query string contains search parameter, form contains hidden field with correct value" do
         search_term = "blam"
-        get "/organisations/#{organisation.id}/logs/csv-download?codes_only=true&search=#{search_term}", headers:, params: {}
+        get "/organisations/#{organisation.id}/sales-logs/csv-download?codes_only=true&search=#{search_term}", headers:, params: {}
         hidden_field = page.find("form.button_to").find_field("search", type: "hidden")
         expect(hidden_field.value).to eq(search_term)
       end

spec/requests/sales_logs_controller_spec.rb

@@ -148,6 +148,16 @@ RSpec.describe SalesLogsController, type: :request do
           expect(page).to have_content(other_organisation.name)
         end
 
+        it "shows a link for labelled CSV download of logs" do
+          get "/sales-logs", headers: headers, params: {}
+          expect(page).to have_link("Download (CSV)", href: "/sales-logs/csv-download?codes_only=false")
+        end
+
+        it "shows a link for codes only CSV download of logs" do
+          get "/sales-logs", headers: headers, params: {}
+          expect(page).to have_link("Download (CSV, codes only)", href: "/sales-logs/csv-download?codes_only=true")
+        end
+
         context "when there are no logs in the database" do
           before do
             SalesLog.destroy_all
@@ -157,6 +167,12 @@ RSpec.describe SalesLogsController, type: :request do
             get "/sales-logs", headers: headers, params: {}
             expect(page).to have_title("Logs - Submit social housing lettings and sales data (CORE) - GOV.UK")
           end
+
+          it "does not show CSV download links" do
+            get "/sales-logs", headers: headers, params: {}
+            expect(page).not_to have_link("Download (CSV)")
+            expect(page).not_to have_link("Download (CSV, codes only)")
+          end
         end
 
         context "when there is a pending log" do
@@ -298,6 +314,19 @@ RSpec.describe SalesLogsController, type: :request do
           expect(page).not_to have_content("Managing organisation")
         end
 
+        it "displays standard CSV download link only, with the correct path" do
+          get "/sales-logs", headers:, params: {}
+          expect(page).to have_link("Download (CSV)", href: "/sales-logs/csv-download?codes_only=false")
+          expect(page).not_to have_link("Download (CSV, codes only)")
+        end
+
+        it "does not display CSV download links if there are no logs" do
+          SalesLog.destroy_all
+          get "/sales-logs", headers:, params: {}
+          expect(page).not_to have_link("Download (CSV)")
+          expect(page).not_to have_link("Download (CSV, codes only)")
+        end
+
         context "when using a search query" do
           let(:logs) { FactoryBot.create_list(:sales_log, 3, :completed, owning_organisation: user.organisation, created_by: user) }
           let(:log_to_search) { FactoryBot.create(:sales_log, :completed, owning_organisation: user.organisation, created_by: user) }
@@ -316,6 +345,13 @@ RSpec.describe SalesLogsController, type: :request do
             end
           end
 
+          it "displays the labelled CSV download link, with the search included in the query params" do
+            get "/sales-logs?search=#{log_to_search.id}", headers:, params: {}
+            download_link = page.find_link("Download (CSV)")
+            download_link_params = CGI.parse(URI.parse(download_link[:href]).query)
+            expect(download_link_params).to include("search" => [log_to_search.id.to_s])
+          end
+
           context "when search query doesn't match any logs" do
             it "doesn't display any logs" do
               get "/sales-logs?search=foobar", headers:, params: {}
@@ -352,7 +388,7 @@ RSpec.describe SalesLogsController, type: :request do
           end
         end
 
-        context "when there are less than 20 logs" do
+        context "when there are fewer than 20 logs" do
           before do
             get "/sales-logs", headers:, params: {}
           end
@@ -469,5 +505,151 @@ RSpec.describe SalesLogsController, type: :request do
         page.assert_selector(".govuk-button", text: "Create a new lettings log", count: 0)
       end
     end
+
+    context "when requesting CSV download" do
+      let(:headers) { { "Accept" => "text/html" } }
+      let(:search_term) { "foot" }
+      let(:codes_only) { false }
+
+      before do
+        allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+        sign_in user
+        get "/sales-logs/csv-download?search=#{search_term}&codes_only=#{codes_only}", headers:
+      end
+
+      it "returns http success" do
+        expect(response).to have_http_status(:success)
+      end
+
+      it "shows a confirmation button" do
+        expect(page).to have_button("Send email")
+      end
+
+      it "has a hidden field with the search term" do
+        expect(page).to have_field("search", type: "hidden", with: search_term)
+      end
+
+      context "when user is not support" do
+        context "and export type is not codes only" do
+          it "has a hidden field with the export type" do
+            expect(page).to have_field("codes_only", type: "hidden", with: codes_only)
+          end
+        end
+
+        context "and export type is codes only" do
+          let(:codes_only) { true }
+
+          it "the user is not authorised" do
+            expect(response).to have_http_status(:unauthorized)
+          end
+        end
+      end
+
+      context "when user is support" do
+        let(:user) { FactoryBot.create(:user, :support) }
+
+        context "and export type is not codes only" do
+          it "has a hidden field with the export type" do
+            expect(page).to have_field("codes_only", type: "hidden", with: codes_only)
+          end
+        end
+
+        context "and export type is codes only" do
+          let(:codes_only) { true }
+
+          it "has a hidden field with the export type" do
+            expect(page).to have_field("codes_only", type: "hidden", with: codes_only)
+          end
+        end
+      end
+    end
+
+    context "when confirming the CSV email" do
+      let(:headers) { { "Accept" => "text/html" } }
+
+      it "confirms that the user will receive an email with the requested CSV" do
+        sign_in user
+        get "/sales-logs/csv-confirmation"
+        expect(CGI.unescape_html(response.body)).to include("We’re sending you an email")
+      end
+    end
+  end
+
+  describe "POST #email-csv" do
+    let(:other_organisation) { FactoryBot.create(:organisation) }
+    let(:user) { FactoryBot.create(:user, :support) }
+    let!(:sales_log) do
+      FactoryBot.create(
+        :sales_log,
+        created_by: user,
+      )
+    end
+
+    before do
+      allow(user).to receive(:need_two_factor_authentication?).and_return(false)
+      sign_in user
+      FactoryBot.create(:sales_log)
+      FactoryBot.create(:sales_log,
+                        :completed,
+                        owning_organisation:,
+                        created_by: user)
+    end
+
+    it "creates an E-mail job with the correct log type" do
+      expect {
+        post "/sales-logs/email-csv?codes_only=true", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true, "sales")
+    end
+
+    it "redirects to the confirmation page" do
+      post "/sales-logs/email-csv?codes_only=true", headers:, params: {}
+      expect(response).to redirect_to(csv_confirmation_sales_logs_path)
+    end
+
+    it "passes the search term" do
+      expect {
+        post "/sales-logs/email-csv?search=#{sales_log.id}&codes_only=false", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, sales_log.id.to_s, {}, false, nil, false, "sales")
+    end
+
+    it "passes filter parameters" do
+      expect {
+        post "/sales-logs/email-csv?status[]=completed&codes_only=true", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, nil, { "status" => %w[completed] }, false, nil, true, "sales")
+    end
+
+    it "passes export type flag" do
+      expect {
+        post "/sales-logs/email-csv?codes_only=true", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, true, "sales")
+      expect {
+        post "/sales-logs/email-csv?codes_only=false", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false, "sales")
+    end
+
+    it "passes a combination of search term, export type and filter parameters" do
+      postcode = "XX1 1TG"
+
+      expect {
+        post "/sales-logs/email-csv?status[]=completed&search=#{postcode}&codes_only=false", headers:, params: {}
+      }.to enqueue_job(EmailCsvJob).with(user, postcode, { "status" => %w[completed] }, false, nil, false, "sales")
+    end
+
+    context "when the user is not a support user" do
+      let(:user) { FactoryBot.create(:user, :data_coordinator) }
+
+      it "has permission to download human readable csv" do
+        codes_only_export = false
+        expect {
+          post "/sales-logs/email-csv?codes_only=#{codes_only_export}", headers:, params: {}
+        }.to enqueue_job(EmailCsvJob).with(user, nil, {}, false, nil, false, "sales")
+      end
+
+      it "is not authorized to download codes only csv" do
+        codes_only_export = true
+        post "/sales-logs/email-csv?codes_only=#{codes_only_export}", headers:, params: {}
+        expect(response).to have_http_status(:unauthorized)
+      end
+    end
   end
 end

spec/services/csv/sales_log_csv_service_spec.rb

@@ -0,0 +1,189 @@
+require "rails_helper"
+
+RSpec.describe Csv::SalesLogCsvService do
+  let(:form_handler_mock) { instance_double(FormHandler) }
+  let(:organisation) { create(:organisation) }
+  let!(:log) { create(:sales_log, :completed, owning_organisation: organisation) }
+  let(:service) { described_class.new(export_type: "labels") }
+  let(:csv) { CSV.parse(service.prepare_csv(SalesLog.all)) }
+
+  it "calls the form handler to get all questions in order when initialized" do
+    allow(FormHandler).to receive(:instance).and_return(form_handler_mock)
+    allow(form_handler_mock).to receive(:ordered_sales_questions_for_all_years).and_return([])
+    described_class.new(export_type: "codes")
+    expect(form_handler_mock).to have_received(:ordered_sales_questions_for_all_years)
+  end
+
+  it "returns a string" do
+    result = service.prepare_csv(SalesLog.all)
+    expect(result).to be_a String
+  end
+
+  it "returns a csv with headers" do
+    expect(csv.first.first).to eq "id"
+    expect(csv.second.first).not_to be log.id.to_s
+  end
+
+  context "when stubbing :ordered_sales_questions_for_all_years" do
+    let(:sales_form) do
+      FormFactory.new(year: 1936, type: "sales")
+                 .with_sections([build(:section, :with_questions, question_ids:, questions:)])
+                 .build
+    end
+    let(:question_ids) { nil }
+    let(:questions) { nil }
+
+    before do
+      allow(FormHandler).to receive(:instance).and_return(form_handler_mock)
+      allow(form_handler_mock).to receive(:form_name_from_start_year)
+      allow(form_handler_mock).to receive(:get_form).and_return(sales_form)
+      allow(form_handler_mock).to receive(:ordered_sales_questions_for_all_years).and_return(sales_form.questions)
+    end
+
+    context "when it returns questions with particular ids" do
+      let(:question_ids) { %w[type age1 buy1livein exdate] }
+
+      it "includes log attributes related to questions to the headers" do
+        headers = csv.first
+        expect(headers).to include(*question_ids.first(3))
+      end
+
+      it "removes some log attributes related to questions from the headers and replaces them with their derived values in the correct order" do
+        headers = csv.first
+        expect(headers).not_to include "exdate"
+        expect(headers.last(4)).to eq %w[buy1livein exday exmonth exyear]
+      end
+    end
+
+    context "when it returns questions with particular features" do
+      let(:questions) do
+        [
+          build(:question, id: "attribute_value_check", type: "interruption_screen"),
+          build(:question, id: "something_or_other_known", type: "radio"),
+          build(:question, id: "whatchamacallit_asked", type: "radio"),
+          build(:question, id: "ownershipsch"),
+          build(:question, id: "checkbox_question", type: "checkbox", answer_options: { "pregyrha" => {}, "pregother" => {} }),
+          build(:question, id: "type"),
+        ]
+      end
+
+      it "does not add questions for checks, whether some other attribute is known or whether something else was asked" do
+        headers = csv.first
+        expect(headers).not_to include "attribute_value_check"
+        expect(headers).not_to include "something_or_other_known"
+        expect(headers).not_to include "whatchamacallit_asked"
+      end
+
+      it "does not add the id of checkbox questions, but adds the related attributes of the log in the correct order" do
+        headers = csv.first
+        expect(headers.last(4)).to eq %w[ownershipsch pregyrha pregother type]
+      end
+    end
+  end
+
+  it "includes attributes not related to questions to the headers" do
+    headers = csv.first
+    expect(headers).to include(*%w[id status created_at updated_at old_id])
+  end
+
+  it "returns a csv with the correct number of logs" do
+    create_list(:sales_log, 15)
+    log_count = SalesLog.count
+    expected_row_count_with_headers = log_count + 1
+    expect(csv.size).to be expected_row_count_with_headers
+  end
+
+  context "when exporting with human readable labels" do
+    it "gives answers to radio questions as their labels" do
+      national_column_index = csv.first.index("national")
+      national_value = csv.second[national_column_index]
+      expect(national_value).to eq "United Kingdom"
+      relat2_column_index = csv.first.index("relat2")
+      relat2_value = csv.second[relat2_column_index]
+      expect(relat2_value).to eq "Partner"
+    end
+
+    it "gives answers to free input questions as the user input" do
+      age1_column_index = csv.first.index("age1")
+      age1_value = csv.second[age1_column_index]
+      expect(age1_value).to eq 30.to_s
+      postcode_part1, postcode_part2 = log.postcode_full.split
+      postcode_part1_column_index = csv.first.index("pcode1")
+      postcode_part1_value = csv.second[postcode_part1_column_index]
+      expect(postcode_part1_value).to eq postcode_part1
+      postcode_part2_column_index = csv.first.index("pcode2")
+      postcode_part2_value = csv.second[postcode_part2_column_index]
+      expect(postcode_part2_value).to eq postcode_part2
+    end
+
+    it "exports the code for the local authority under the heading 'la'" do
+      la_column_index = csv.first.index("la")
+      la_value = csv.second[la_column_index]
+      expect(la_value).to eq "E09000003"
+    end
+
+    it "exports the label for the local authority under the heading 'la_label'" do
+      la_label_column_index = csv.first.index("la_label")
+      la_label_value = csv.second[la_label_column_index]
+      expect(la_label_value).to eq "Barnet"
+    end
+
+    it "exports the CSV with all values correct" do
+      expected_content = CSV.read("spec/fixtures/files/sales_logs_csv_export_labels.csv")
+      values_to_delete = %w[id]
+      values_to_delete.each do |attribute|
+        index = csv.first.index(attribute)
+        csv.second[index] = nil
+      end
+      expect(csv).to eq expected_content
+    end
+  end
+
+  context "when exporting values as codes" do
+    let(:service) { described_class.new(export_type: "codes") }
+
+    it "gives answers to radio questions as their codes" do
+      national_column_index = csv.first.index("national")
+      national_value = csv.second[national_column_index]
+      expect(national_value).to eq 18.to_s
+      relat2_column_index = csv.first.index("relat2")
+      relat2_value = csv.second[relat2_column_index]
+      expect(relat2_value).to eq "P"
+    end
+
+    it "gives answers to free input questions as the user input" do
+      age1_column_index = csv.first.index("age1")
+      age1_value = csv.second[age1_column_index]
+      expect(age1_value).to eq 30.to_s
+      postcode_part1, postcode_part2 = log.postcode_full.split
+      postcode_part1_column_index = csv.first.index("pcode1")
+      postcode_part1_value = csv.second[postcode_part1_column_index]
+      expect(postcode_part1_value).to eq postcode_part1
+      postcode_part2_column_index = csv.first.index("pcode2")
+      postcode_part2_value = csv.second[postcode_part2_column_index]
+      expect(postcode_part2_value).to eq postcode_part2
+    end
+
+    it "exports the code for the local authority under the heading 'la'" do
+      la_column_index = csv.first.index("la")
+      la_value = csv.second[la_column_index]
+      expect(la_value).to eq "E09000003"
+    end
+
+    it "exports the label for the local authority under the heading 'la_label'" do
+      la_label_column_index = csv.first.index("la_label")
+      la_label_value = csv.second[la_label_column_index]
+      expect(la_label_value).to eq "Barnet"
+    end
+
+    it "exports the CSV with all values correct" do
+      expected_content = CSV.read("spec/fixtures/files/sales_logs_csv_export_codes.csv")
+      values_to_delete = %w[id]
+      values_to_delete.each do |attribute|
+        index = csv.first.index(attribute)
+        csv.second[index] = nil
+      end
+      expect(csv).to eq expected_content
+    end
+  end
+end