Refactor to improve readability and simplify logic

app/models/csv_variable_definition.rb

@@ -5,4 +5,8 @@ class CsvVariableDefinition < ApplicationRecord
   validates :user_type, presence: true, inclusion: { in: %w[user support] }
   validates :year, presence: true, numericality: { only_integer: true, greater_than_or_equal_to: 2000, less_than_or_equal_to: 2099 }
   attribute :last_accessed, :datetime
+
+  scope :user, -> { where(user_type: "user") }
+  scope :lettings, -> { where(log_type: "lettings") }
+  scope :sales, -> { where(log_type: "sales") }
 end

app/models/user.rb

@@ -110,6 +110,14 @@ class User < ApplicationRecord
     SalesLog.filter_by_managing_organisation(organisation.absorbed_organisations + [organisation])
   end
 
+  def variable_definitions
+    if support?
+      CsvVariableDefinition.all
+    else
+      CsvVariableDefinition.user
+    end
+  end
+
   def schemes
     if support?
       Scheme.all
@@ -263,6 +271,10 @@ class User < ApplicationRecord
     :active
   end
 
+  def user_type
+    support? ? "support" : "user"
+  end
+
   def discard!
     self.discarded_at = Time.zone.now
     save!(validate: false)

app/services/csv/lettings_log_csv_service.rb

@@ -12,10 +12,10 @@ module Csv
       CSV.generate(headers: true) do |csv|
         if @year >= 2023
           csv << @attributes.map do |attribute|
-            definition = @definitions.find { |record| record.variable == attribute.downcase }
-            if definition
-              definition.update!(last_accessed: Time.zone.now)
-              definition.definition
+            record = @definitions.find { |r| r.variable == attribute.downcase }
+            if record
+              record.update!(last_accessed: Time.zone.now)
+              record.definition
             end
           end
         end
@@ -269,23 +269,19 @@ module Csv
     end
 
     def lettings_log_definitions
-      definitions = if @user.support?
-                      CsvVariableDefinition.where("log_type = 'lettings'")
-                    else
-                      CsvVariableDefinition.where("user_type = 'user' AND log_type = 'lettings'")
-                    end
+
+      definitions = @user.variable_definitions.lettings
 
       definitions.group_by { |record| [record.variable, record.definition] }
-                 .map do |_, records|
-        record = nil
-        year = @year
-        while year >= 2021 && record.nil?
-          record = records.find do |r|
-            r.year == year && (!@user.support? || r.user_type == "support" || r.user_type == "user")
-          end
-          year -= 1
-        end
-        record || records.first
+                 .map do |_, options|
+
+        exact_match = options.find { |definition| definition.year == @year && definition.user_type == @user.user_type }
+        next exact_match if exact_match
+
+        recent_match = options.select { |definition| definition.user_type == @user.user_type }.max_by(&:year)
+        next recent_match if recent_match
+
+        options.max_by(&:year)
       end
     end
 

app/services/csv/sales_log_csv_service.rb

@@ -13,10 +13,10 @@ module Csv
         formatted_attributes = formatted_attribute_headers
         if @year >= 2023
           csv << formatted_attributes.map do |attribute|
-            definition = @definitions.find { |record| record.variable == attribute.downcase }
-            if definition
-              definition.update!(last_accessed: Time.zone.now)
-              definition.definition
+            record = @definitions.find { |r| r.variable == attribute.downcase }
+            if record
+              record.update!(last_accessed: Time.zone.now)
+              record.definition
             end
           end
         end
@@ -173,23 +173,19 @@ module Csv
     end
 
     def sales_log_definitions
-      definitions = if @user.support?
-                      CsvVariableDefinition.where("log_type = 'sales'")
-                    else
-                      CsvVariableDefinition.where("user_type = 'user' AND log_type = 'sales'")
-                    end
+
+      definitions = @user.variable_definitions.sales
 
       definitions.group_by { |record| [record.variable, record.definition] }
-                 .map do |_, records|
-        record = nil
-        year = @year
-        while year >= 2022 && record.nil?
-          record = records.find do |r|
-            r.year == year && (!@user.support? || r.user_type == "support" || r.user_type == "user")
-          end
-          year -= 1
-        end
-        record || records.first
+                 .map do |_, options|
+
+        exact_match = options.find { |definition| definition.year == @year && definition.user_type == @user.user_type }
+        next exact_match if exact_match
+
+        recent_match = options.select { |definition| definition.user_type == @user.user_type }.max_by(&:year)
+        next recent_match if recent_match
+
+        options.max_by(&:year)
       end
     end