feat(exports): add rule operand references (#1726)

* feat(exports): add rule operand references

* fix(exports): preserve rule operand references

* refactor(exports): simplify rule operand branches

* refactor(validation): centralize UUID format checks

* fix(imports): preserve false rule operands

app/controllers/api/v1/base_controller.rb

@@ -3,14 +3,11 @@
 class Api::V1::BaseController < ApplicationController
   include Doorkeeper::Rails::Helpers
 
-  UUID_PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i
-  private_constant :UUID_PATTERN
-
   InvalidFilterError = Class.new(StandardError)
 
   class << self
     def valid_uuid?(value)
-      value.to_s.match?(UUID_PATTERN)
+      UuidFormat.valid?(value)
     end
   end
 

app/controllers/api/v1/valuations_controller.rb

@@ -269,10 +269,6 @@ class Api::V1::ValuationsController < Api::V1::BaseController
       raise InvalidFilterError, "#{key} must be an ISO 8601 date"
     end
 
-    def valid_uuid?(value)
-      value.to_s.match?(/\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i)
-    end
-
     def safe_page_param
       page = params[:page].to_i
       page > 0 ? page : 1

app/models/family/data_exporter.rb

@@ -493,11 +493,14 @@ class Family::DataExporter
     end
 
     def serialize_condition(condition)
+      operand = resolve_condition_operand(condition)
       data = {
         condition_type: condition.condition_type,
         operator: condition.operator,
-        value: resolve_condition_value(condition)
+        value: operand[:value]
       }
+      value_ref = operand[:value_ref]
+      data[:value_ref] = value_ref if value_ref.present?
 
       if condition.compound? && condition.sub_conditions.any?
         data[:sub_conditions] = condition.sub_conditions.map { |sub| serialize_condition(sub) }
@@ -507,52 +510,79 @@ class Family::DataExporter
     end
 
     def serialize_action(action)
-      {
+      operand = resolve_action_operand(action)
+      data = {
         action_type: action.action_type,
-        value: resolve_action_value(action)
+        value: operand[:value]
       }
+      value_ref = operand[:value_ref]
+      data[:value_ref] = value_ref if value_ref.present?
+
+      data
     end
 
-    def resolve_condition_value(condition)
-      return condition.value unless condition.value.present?
+    def resolve_condition_operand(condition)
+      return rule_operand(condition.value) unless condition.value.present?
 
       # Map category UUIDs to names for portability
-      if condition.condition_type == "transaction_category" && condition.value.present?
-        category = @family.categories.find_by(id: condition.value)
-        return category&.name || condition.value
+      if condition.condition_type == "transaction_category"
+        return rule_operand(condition.value, type: "Category", relation: @family.categories)
       end
 
       # Map merchant UUIDs to names for portability
-      if condition.condition_type == "transaction_merchant" && condition.value.present?
-        merchant = @family.merchants.find_by(id: condition.value)
-        return merchant&.name || condition.value
+      if condition.condition_type == "transaction_merchant"
+        return rule_operand(condition.value, type: "Merchant", relation: @family.merchants)
       end
 
-      condition.value
+      rule_operand(condition.value)
     end
 
-    def resolve_action_value(action)
-      return action.value unless action.value.present?
+    def resolve_action_operand(action)
+      return rule_operand(action.value) unless action.value.present?
 
       # Map category UUIDs to names for portability
-      if action.action_type == "set_transaction_category" && action.value.present?
-        category = @family.categories.find_by(id: action.value) || @family.categories.find_by(name: action.value)
-        return category&.name || action.value
+      if action.action_type == "set_transaction_category"
+        return rule_operand(action.value, type: "Category", relation: @family.categories, fallback_to_name: true)
       end
 
       # Map merchant UUIDs to names for portability
-      if action.action_type == "set_transaction_merchant" && action.value.present?
-        merchant = @family.merchants.find_by(id: action.value) || @family.merchants.find_by(name: action.value)
-        return merchant&.name || action.value
+      if action.action_type == "set_transaction_merchant"
+        return rule_operand(action.value, type: "Merchant", relation: @family.merchants, fallback_to_name: true)
       end
 
       # Map tag UUIDs to names for portability
-      if action.action_type == "set_transaction_tags" && action.value.present?
-        tag = @family.tags.find_by(id: action.value) || @family.tags.find_by(name: action.value)
-        return tag&.name || action.value
+      if action.action_type == "set_transaction_tags"
+        return rule_operand(action.value, type: "Tag", relation: @family.tags, fallback_to_name: true)
       end
 
-      action.value
+      rule_operand(action.value)
+    end
+
+    def rule_operand(value, type: nil, relation: nil, fallback_to_name: false)
+      record = relation && resolve_rule_operand_record(relation, value, fallback_to_name: fallback_to_name)
+
+      {
+        value: record&.name || value,
+        value_ref: record ? rule_value_ref(type, record) : nil
+      }
+    end
+
+    def resolve_rule_operand_record(relation, value, fallback_to_name:)
+      return relation.find_by(id: value) if uuid_like?(value)
+
+      relation.find_by(name: value) if fallback_to_name
+    end
+
+    def rule_value_ref(type, record)
+      {
+        type: type,
+        id: record.id,
+        name: record.name
+      }
+    end
+
+    def uuid_like?(value)
+      UuidFormat.valid?(value)
     end
 
     def serialize_conditions_for_csv(conditions)

app/models/family/data_importer.rb

@@ -671,7 +671,7 @@ class Family::DataImporter
 
     def resolve_rule_condition_value(condition_data)
       condition_type = condition_data["condition_type"]
-      value = condition_data["value"]
+      value = rule_operand_value(condition_data)
 
       return value unless value.present?
 
@@ -699,7 +699,7 @@ class Family::DataImporter
 
     def resolve_rule_action_value(action_data)
       action_type = action_data["action_type"]
-      value = action_data["value"]
+      value = rule_operand_value(action_data)
 
       return value unless value.present?
 
@@ -732,6 +732,21 @@ class Family::DataImporter
       value
     end
 
+    def rule_operand_value(data)
+      raw_value = data["value"]
+      value = raw_value.is_a?(String) ? raw_value.presence : raw_value
+      value_ref_name = data.dig("value_ref", "name")
+
+      return value_ref_name if value.is_a?(String) && uuid_like?(value) && value_ref_name.present?
+      return value unless value.nil?
+
+      value_ref_name
+    end
+
+    def uuid_like?(value)
+      UuidFormat.valid?(value)
+    end
+
     def importable_cost_basis_source(value)
       source = value.to_s
       Holding::COST_BASIS_SOURCES.include?(source) ? source : nil

lib/uuid_format.rb

@@ -0,0 +1,11 @@
+# frozen_string_literal: true
+
+module UuidFormat
+  PATTERN = /\A[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}\z/i.freeze
+
+  module_function
+
+  def valid?(value)
+    PATTERN.match?(value.to_s)
+  end
+end

test/lib/uuid_format_test.rb

@@ -0,0 +1,17 @@
+require "test_helper"
+
+class UuidFormatTest < ActiveSupport::TestCase
+  test "valid matches canonical UUID values" do
+    uuid = SecureRandom.uuid
+
+    assert UuidFormat.valid?(uuid)
+    assert UuidFormat.valid?(uuid.upcase)
+  end
+
+  test "valid rejects non UUID values" do
+    refute UuidFormat.valid?(nil)
+    refute UuidFormat.valid?("")
+    refute UuidFormat.valid?("not-a-uuid")
+    refute UuidFormat.valid?("#{SecureRandom.uuid}-extra")
+  end
+end

test/models/family/data_exporter_test.rb

@@ -318,9 +318,56 @@ class Family::DataExporterTest < ActiveSupport::TestCase
       assert_equal "set_transaction_category", actions[0]["action_type"]
       # Should export category name instead of UUID
       assert_equal "Test Category", actions[0]["value"]
+      assert_equal({ "type" => "Category", "id" => @category.id, "name" => "Test Category" }, actions[0]["value_ref"])
     end
   end
 
+  test "exports rule condition value refs for mapped operands" do
+    category_rule = @family.rules.build(
+      name: "Category Condition Rule",
+      resource_type: "transaction",
+      active: true
+    )
+    category_rule.conditions.build(
+      condition_type: "transaction_category",
+      operator: "=",
+      value: @category.id
+    )
+    category_rule.actions.build(action_type: "auto_categorize")
+    category_rule.save!
+
+    zip_data = @exporter.generate_export
+
+    Zip::File.open_buffer(zip_data) do |zip|
+      rule_data = zip.read("all.ndjson").split("\n").filter_map do |line|
+        parsed = JSON.parse(line)
+        parsed if parsed["type"] == "Rule" && parsed["data"]["name"] == "Category Condition Rule"
+      end.first
+
+      condition = rule_data["data"]["conditions"].first
+      assert_equal "Test Category", condition["value"]
+      assert_equal({ "type" => "Category", "id" => @category.id, "name" => "Test Category" }, condition["value_ref"])
+    end
+  end
+
+  test "rule operand lookup skips name fallback for stale UUID values" do
+    stale_uuid = SecureRandom.uuid
+    relation = mock
+    relation.expects(:find_by).with(id: stale_uuid).once.returns(nil)
+    relation.expects(:find_by).with(name: stale_uuid).never
+
+    operand = @exporter.send(
+      :rule_operand,
+      stale_uuid,
+      type: "Category",
+      relation: relation,
+      fallback_to_name: true
+    )
+
+    assert_equal stale_uuid, operand[:value]
+    assert_nil operand[:value_ref]
+  end
+
   test "exports rule actions and maps tag UUIDs to names" do
     # Create a rule with a tag action
     tag_rule = @family.rules.build(
@@ -359,6 +406,7 @@ class Family::DataExporterTest < ActiveSupport::TestCase
       assert_equal "set_transaction_tags", actions[0]["action_type"]
       # Should export tag name instead of UUID
       assert_equal "Test Tag", actions[0]["value"]
+      assert_equal({ "type" => "Tag", "id" => @tag.id, "name" => "Test Tag" }, actions[0]["value_ref"])
     end
   end
 

test/models/family/data_importer_test.rb

@@ -1498,6 +1498,119 @@ class Family::DataImporterTest < ActiveSupport::TestCase
     assert_equal category.id, action.value
   end
 
+  test "imports rules from normalized operand value refs" do
+    ndjson = build_ndjson([
+      {
+        type: "Rule",
+        version: 1,
+        data: {
+          name: "Map Merchant To Dining",
+          resource_type: "transaction",
+          active: true,
+          conditions: [
+            {
+              condition_type: "transaction_merchant",
+              operator: "=",
+              value_ref: {
+                type: "Merchant",
+                id: "source-merchant-id",
+                name: "Coffee Bar"
+              }
+            }
+          ],
+          actions: [
+            {
+              action_type: "set_transaction_category",
+              value_ref: {
+                type: "Category",
+                id: "source-category-id",
+                name: "Dining"
+              }
+            }
+          ]
+        }
+      }
+    ])
+
+    importer = Family::DataImporter.new(@family, ndjson)
+    importer.import!
+
+    rule = @family.rules.find_by!(name: "Map Merchant To Dining")
+    merchant = @family.merchants.find_by!(name: "Coffee Bar")
+    category = @family.categories.find_by!(name: "Dining")
+
+    assert_equal merchant.id, rule.conditions.first.value
+    assert_equal category.id, rule.actions.first.value
+  end
+
+  test "imports rule value refs when legacy operand values are stale UUIDs" do
+    stale_merchant_id = SecureRandom.uuid
+    stale_category_id = SecureRandom.uuid
+    ndjson = build_ndjson([
+      {
+        type: "Rule",
+        version: 1,
+        data: {
+          name: "Map Stale UUID Operands",
+          resource_type: "transaction",
+          active: true,
+          conditions: [
+            {
+              condition_type: "transaction_merchant",
+              operator: "=",
+              value: stale_merchant_id,
+              value_ref: {
+                type: "Merchant",
+                id: stale_merchant_id,
+                name: "Coffee Bar"
+              }
+            }
+          ],
+          actions: [
+            {
+              action_type: "set_transaction_category",
+              value: stale_category_id,
+              value_ref: {
+                type: "Category",
+                id: stale_category_id,
+                name: "Dining"
+              }
+            }
+          ]
+        }
+      }
+    ])
+
+    importer = Family::DataImporter.new(@family, ndjson)
+    importer.import!
+
+    rule = @family.rules.find_by!(name: "Map Stale UUID Operands")
+    merchant = @family.merchants.find_by!(name: "Coffee Bar")
+    category = @family.categories.find_by!(name: "Dining")
+
+    assert_equal merchant.id, rule.conditions.first.value
+    assert_equal category.id, rule.actions.first.value
+    assert_not @family.merchants.exists?(name: stale_merchant_id)
+    assert_not @family.categories.exists?(name: stale_category_id)
+  end
+
+  test "preserves explicit false rule operand values" do
+    importer = Family::DataImporter.new(@family, "")
+
+    value = importer.send(
+      :rule_operand_value,
+      {
+        "value" => false,
+        "value_ref" => {
+          "type" => "Category",
+          "name" => "Fallback"
+        }
+      }
+    )
+
+    assert_equal false, value
+  end
+
   test "imports rules with compound conditions" do
     ndjson = build_ndjson([
       {