rework csv service for readability, remove delegating methods from lettings log to keep all code to do with mapping between our domain and desired export format in one place

app/models/lettings_log.rb

@@ -425,31 +425,13 @@ class LettingsLog < Log
     created_by&.is_dpo
   end
 
-  delegate :service_name, :sensitive, :registered_under_care_act, :primary_client_group, :has_other_client_group, :secondary_client_group, :owning_organisation, :managing_organisation, :support_type, :intended_stay, :created_at, prefix: "scheme", to: :scheme, allow_nil: true
-  delegate :service_name_before_type_cast, :sensitive_before_type_cast, :registered_under_care_act_before_type_cast, :primary_client_group_before_type_cast, :has_other_client_group_before_type_cast, :secondary_client_group_before_type_cast, :owning_organisation_before_type_cast, :managing_organisation_before_type_cast, :support_type_before_type_cast, :intended_stay_before_type_cast, :created_at_before_type_cast, prefix: "scheme", to: :scheme, allow_nil: true
-  delegate :scheme_type, to: :scheme, allow_nil: true
-  delegate :scheme_type_before_type_cast, to: :scheme, allow_nil: true
-
   def scheme_code
     scheme&.id ? "S#{scheme.id}" : nil
   end
 
-  def scheme_owning_organisation_name
-    scheme_owning_organisation&.name
-  end
-
-  delegate :postcode, :name, :units, :type_of_unit, :mobility_type, :startdate, prefix: "location", to: :location, allow_nil: true
-  delegate :postcode_before_type_cast, :name_before_type_cast, :units_before_type_cast, :type_of_unit_before_type_cast, :mobility_type_before_type_cast, :startdate_before_type_cast, prefix: "location", to: :location, allow_nil: true
-  delegate :location_admin_district, to: :location, allow_nil: true
-  delegate :location_admin_district_before_type_cast, to: :location, allow_nil: true
-
-  # This is not the location_code in the db, location.id is just called code in the UI
-  def location_code
-    location&.id
-  end
-
   def self.to_csv(user = nil, codes_only_export:)
-    Csv::LettingsLogCsvService.new(user).to_csv(codes_only_export:)
+    export_type = codes_only_export ? "codes" : "labels"
+    Csv::LettingsLogCsvService.new(user, export_type:).to_csv
   end
 
   def beds_for_la_rent_range

app/services/csv/lettings_log_csv_service.rb

@@ -2,47 +2,134 @@ module Csv
   class LettingsLogCsvService
     CSV_FIELDS_TO_OMIT = %w[hhmemb net_income_value_check first_time_property_let_as_social_housing renttype needstype postcode_known is_la_inferred totchild totelder totadult net_income_known is_carehome previous_la_known is_previous_la_inferred age1_known age2_known age3_known age4_known age5_known age6_known age7_known age8_known letting_allocation_unknown details_known_2 details_known_3 details_known_4 details_known_5 details_known_6 details_known_7 details_known_8 rent_type_detail wrent wscharge wpschrge wsupchrg wtcharge wtshortfall rent_value_check old_form_id old_id retirement_value_check tshortfall_known pregnancy_value_check hhtype new_old vacdays la prevloc unresolved updated_by_id bulk_upload_id].freeze
 
-    def initialize(user)
+    def initialize(user, export_type:)
       @user = user
+      @export_type = export_type
       set_csv_attributes
     end
 
-    def to_csv(codes_only_export:)
+    def to_csv
       CSV.generate(headers: true) do |csv|
         csv << @attributes
 
         LettingsLog.all.find_each do |record|
-          csv << @attributes.map do |att|
-            label_from_value(record, att, codes_only_export:)
-          end
+          csv << @attributes.map { |attribute| get_value(attribute, record) }
         end
       end
     end
 
   private
 
-    def label_from_value(record, att, codes_only_export:)
-      if %w[la prevloc].include? att
-        record.send(att)
-      elsif %w[mrcdate startdate voiddate].include? att
-        record.send(att)&.to_formatted_s(:govuk_date)
-      elsif codes_only_export && att.start_with?("location_", "scheme_")
-        att += "_before_type_cast" unless %w[location_code scheme_code scheme_owning_organisation_name scheme_created_at location_startdate].include? att
-        record.send(att)
+    ATTRIBUTES_OF_RELATED_OBJECTS = {
+      location_code: {
+        labels: %i[location id],
+        codes: %i[location id],
+      },
+      location_postcode: {
+        labels: %i[location postcode],
+        codes: %i[location postcode],
+      },
+      location_name: {
+        labels: %i[location name],
+        codes: %i[location name],
+      },
+      location_units: {
+        labels: %i[location units],
+        codes: %i[location units],
+      },
+      location_type_of_unit: {
+        labels: %i[location type_of_unit],
+        codes: %i[location type_of_unit_before_type_cast],
+      },
+      location_mobility_type: {
+        labels: %i[location mobility_type],
+        codes: %i[location mobility_type_before_type_cast],
+      },
+      location_admin_district: {
+        labels: %i[location location_admin_district],
+        codes: %i[location location_admin_district],
+      },
+      location_startdate: {
+        labels: %i[location startdate],
+        codes: %i[location startdate],
+      },
+      scheme_service_name: {
+        labels: %i[scheme service_name],
+        codes: %i[scheme service_name],
+      },
+      scheme_sensitive: {
+        labels: %i[scheme sensitive],
+        codes: %i[scheme sensitive_before_type_cast],
+      },
+      scheme_type: {
+        labels: %i[scheme scheme_type],
+        codes: %i[scheme scheme_type_before_type_cast],
+      },
+      scheme_registered_under_care_act: {
+        labels: %i[scheme registered_under_care_act],
+        codes: %i[scheme registered_under_care_act_before_type_cast],
+      },
+      scheme_owning_organisation_name: {
+        labels: %i[scheme owning_organisation name],
+        codes: %i[scheme owning_organisation name],
+      },
+      scheme_primary_client_group: {
+        labels: %i[scheme primary_client_group],
+        codes: %i[scheme primary_client_group_before_type_cast],
+      },
+      scheme_has_other_client_group: {
+        labels: %i[scheme has_other_client_group],
+        codes: %i[scheme has_other_client_group_before_type_cast],
+      },
+      scheme_secondary_client_group: {
+        labels: %i[scheme secondary_client_group],
+        codes: %i[scheme secondary_client_group_before_type_cast],
+      },
+      scheme_support_type: {
+        labels: %i[scheme support_type],
+        codes: %i[scheme support_type_before_type_cast],
+      },
+      scheme_intended_stay: {
+        labels: %i[scheme intended_stay],
+        codes: %i[scheme intended_stay_before_type_cast],
+      },
+      scheme_created_at: {
+        labels: %i[scheme created_at],
+        codes: %i[scheme created_at],
+      },
+    }.freeze
+
+    def get_value(attribute, record)
+      attribute = "rent_type" if attribute == "rent_type_detail" # rent_type_detail is the requested column header for rent_type, so as not to confuse with renttype
+      if ATTRIBUTES_OF_RELATED_OBJECTS.key? attribute.to_sym
+        call_chain = ATTRIBUTES_OF_RELATED_OBJECTS[attribute.to_sym][@export_type.to_sym]
+        call_chain.reduce(record) { |object, next_call| object&.send(next_call) }
+      elsif %w[la prevloc].include? attribute # for all exports we output both the codes and labels for these location attributes
+        record.send(attribute)
+      elsif %w[la_label prevloc_label].include? attribute # as above
+        attribute = attribute.remove("_label")
+        field_value = record.send(attribute)
+        get_label(field_value, attribute, record)
+      elsif %w[mrcdate startdate voiddate].include? attribute
+        record.send(attribute)&.to_formatted_s(:govuk_date)
       else
-        att = att.remove("_label", "_detail") # a couple of csv column headers have suffixes for the user that are not reflected in the app domain
-        field_value = record.send(att)
-        if codes_only_export
+        field_value = record.send(attribute)
+        case @export_type
+        when "codes"
           field_value
-        else
-          answer_label = record.form
-                               .get_question(att, record)
-                               &.label_from_value(field_value)
+        when "labels"
+          answer_label = get_label(field_value, attribute, record)
           answer_label || label_if_boolean_value(field_value) || field_value
         end
       end
     end
 
+    def get_label(value, attribute, record)
+      record.form
+            .get_question(attribute, record)
+            &.label_from_value(value)
+    end
+
     def label_if_boolean_value(value)
       return "Yes" if value == true
       return "No" if value == false

spec/fixtures/files/lettings_logs_download_codes_only.csv

@@ -1,2 +1,2 @@
 id,status,created_at,updated_at,created_by_name,is_dpo,owning_organisation_name,managing_organisation_name,collection_start_year,needstype,renewal,startdate,rent_type_detail,irproduct_other,tenancycode,propcode,age1,sex1,ecstat1,hhmemb,relat2,age2,sex2,retirement_value_check,ecstat2,armedforces,leftreg,illness,housingneeds_a,housingneeds_b,housingneeds_c,housingneeds_h,is_previous_la_inferred,prevloc_label,prevloc,illness_type_1,illness_type_2,is_la_inferred,la_label,la,postcode_known,postcode_full,previous_la_known,wchair,preg_occ,cbl,earnings,incfreq,net_income_value_check,benefits,hb,period,brent,scharge,pscharge,supcharg,tcharge,offered,layear,ppostcode_full,mrcdate,declaration,ethnic,national,prevten,age3,sex3,ecstat3,age4,sex4,ecstat4,age5,sex5,ecstat5,age6,sex6,ecstat6,age7,sex7,ecstat7,age8,sex8,ecstat8,homeless,underoccupation_benefitcap,reservist,startertenancy,tenancylength,tenancy,rsnvac,unittype_gn,beds,waityear,reasonpref,chr,cap,reasonother,housingneeds_f,housingneeds_g,illness_type_3,illness_type_4,illness_type_8,illness_type_5,illness_type_6,illness_type_7,illness_type_9,illness_type_10,rp_homeless,rp_insan_unsat,rp_medwel,rp_hardship,rp_dontknow,tenancyother,property_owner_organisation,property_manager_organisation,purchaser_code,reason,majorrepairs,hbrentshortfall,property_relet,incref,first_time_property_let_as_social_housing,unitletas,builtype,voiddate,renttype,lettype,totchild,totelder,totadult,net_income_known,nocharge,is_carehome,household_charge,referral,tshortfall,chcharge,ppcodenk,age1_known,age2_known,age3_known,age4_known,age5_known,age6_known,age7_known,age8_known,ethnic_group,letting_allocation_unknown,details_known_2,details_known_3,details_known_4,details_known_5,details_known_6,details_known_7,details_known_8,has_benefits,wrent,wscharge,wpschrge,wsupchrg,wtcharge,wtshortfall,refused,housingneeds,wchchrg,newprop,relat3,relat4,relat5,relat6,relat7,relat8,rent_value_check,old_form_id,lar,irproduct,old_id,joint,tshortfall_known,sheltered,pregnancy_value_check,hhtype,new_old,vacdays,major_repairs_date_value_check,void_date_value_check,housingneeds_type,housingneeds_other,unresolved,updated_by_id,unittype_sh,scheme_code,scheme_service_name,scheme_sensitive,scheme_type,scheme_registered_under_care_act,scheme_owning_organisation_name,scheme_primary_client_group,scheme_has_other_client_group,scheme_secondary_client_group,scheme_support_type,scheme_intended_stay,scheme_created_at,location_code,location_postcode,location_name,location_units,location_type_of_unit,location_mobility_type,location_admin_district,location_startdate
-{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,false,DLUHC,DLUHC,2021,2,,2 October 2021,2,,,,,,,,,,,,,,,,,,,,false,,,,,false,E09000033,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,9,1,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},0,1,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,6,A,Westminster,{location_startdate}
+{id},in_progress,2022-02-08 16:52:15 +0000,2022-02-08 16:52:15 +0000,Danny Rojas,false,DLUHC,DLUHC,2021,2,,2 October 2021,2,,,,,,,,,,,,,,,,,,,,false,,,,,false,Westminster,E09000033,,SE1 1TE,,2,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,,2,8,0,0,0,,0,,,,,,,,,,,,,,,,,,,,,,,,0,,,,,,,0,,,,,,,,,,,,,,,,,,,9,1,,,,,,,,6,{scheme_code},{scheme_service_name},{scheme_sensitive},0,1,DLUHC,{scheme_primary_client_group},,{scheme_secondary_client_group},{scheme_support_type},{scheme_intended_stay},2021-04-01 00:00:00 +0100,{location_code},SE1 1TE,Downing Street,20,6,A,Westminster,{location_startdate}

spec/services/csv/lettings_log_csv_service_spec.rb

@@ -222,7 +222,7 @@ RSpec.describe Csv::LettingsLogCsvService do
                                    location_mobility_type
                                    location_admin_district
                                    location_startdate]
-      csv = CSV.parse(described_class.new(user).to_csv(codes_only_export: false))
+      csv = CSV.parse(described_class.new(user, export_type: "labels").to_csv)
       expect(csv.first).to eq(expected_csv_attributes)
     end
   end