Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions app/controllers/api/v1/base_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
4 changes: 0 additions & 4 deletions app/controllers/api/v1/valuations_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -269,10 +269,6 @@ def parse_date_param(key)
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
Expand Down
78 changes: 54 additions & 24 deletions app/models/family/data_exporter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -493,11 +493,14 @@ def serialize_rule_for_export(rule)
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) }
Expand All @@ -507,52 +510,79 @@ def serialize_condition(condition)
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)
Expand Down
19 changes: 17 additions & 2 deletions app/models/family/data_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ def build_rule_action(rule, action_data)

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?

Expand Down Expand Up @@ -699,7 +699,7 @@ def resolve_rule_condition_value(condition_data)

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?

Expand Down Expand Up @@ -732,6 +732,21 @@ def resolve_rule_action_value(action_data)
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
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def uuid_like?(value)
UuidFormat.valid?(value)
end
Comment thread
coderabbitai[bot] marked this conversation as resolved.

def importable_cost_basis_source(value)
source = value.to_s
Holding::COST_BASIS_SOURCES.include?(source) ? source : nil
Expand Down
11 changes: 11 additions & 0 deletions lib/uuid_format.rb
Original file line number Diff line number Diff line change
@@ -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
17 changes: 17 additions & 0 deletions test/lib/uuid_format_test.rb
Original file line number Diff line number Diff line change
@@ -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
48 changes: 48 additions & 0 deletions test/models/family/data_exporter_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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

Expand Down
Loading
Loading