Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 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,9 +3,6 @@
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)

# Skip regular session-based authentication for API
Expand Down Expand Up @@ -220,7 +217,7 @@ def render_json(data, status: :ok)
end

def valid_uuid?(value)
value.to_s.match?(UUID_PATTERN)
UuidFormat.valid?(value)
end

def safe_page_param
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
17 changes: 15 additions & 2 deletions app/models/family/data_importer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -627,7 +627,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 @@ -655,7 +655,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 @@ -688,6 +688,19 @@ def resolve_rule_action_value(action_data)
value
end

def rule_operand_value(data)
value = data["value"].presence
value_ref_name = data.dig("value_ref", "name")

return value_ref_name if value.present? && uuid_like?(value) && value_ref_name.present?

value || 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
96 changes: 96 additions & 0 deletions test/models/family/data_importer_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1201,6 +1201,102 @@ 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 "imports rules with compound conditions" do
ndjson = build_ndjson([
{
Expand Down