Merchants improvements (#594)

* FIX logos

* Implement merchant mods

* FIX confirm issue

* FIX linter

* Add recently seen merchants to re-add if needed

* Update merge.html.erb

* FIX do security check

* Add error handling for update failures.
This commit is contained in:
soky srm
2026-01-09 19:38:04 +01:00
committed by GitHub
parent 140ea78b0e
commit 76dc91377c
25 changed files with 431 additions and 36 deletions

View File

@@ -45,6 +45,15 @@ class Family < ApplicationRecord
Merchant.where(id: merchant_ids)
end
def available_merchants
assigned_ids = transactions.where.not(merchant_id: nil).pluck(:merchant_id).uniq
recently_unlinked_ids = FamilyMerchantAssociation
.where(family: self)
.recently_unlinked
.pluck(:merchant_id)
Merchant.where(id: (assigned_ids + recently_unlinked_ids).uniq)
end
def auto_categorize_transactions_later(transactions, rule_run_id: nil)
AutoCategorizeJob.perform_later(self, transaction_ids: transactions.pluck(:id), rule_run_id: rule_run_id)
end

View File

@@ -102,21 +102,33 @@ class Family::AutoMerchantDetector
end
def find_or_create_ai_merchant(auto_detection)
# Only use (source, name) for find_or_create since that's the uniqueness constraint
ProviderMerchant.find_or_create_by!(
source: "ai",
name: auto_detection.business_name
) do |pm|
pm.website_url = auto_detection.business_url
if Setting.brand_fetch_client_id.present?
pm.logo_url = "#{default_logo_provider_url}/#{auto_detection.business_url}/icon/fallback/lettermark/w/40/h/40?c=#{Setting.brand_fetch_client_id}"
end
# Strategy 1: Find existing merchant by website_url (most reliable for deduplication)
if auto_detection.business_url.present?
existing = ProviderMerchant.find_by(website_url: auto_detection.business_url)
return existing if existing
end
# Strategy 2: Find by exact name match
existing = ProviderMerchant.find_by(source: "ai", name: auto_detection.business_name)
return existing if existing
# Strategy 3: Create new merchant
ProviderMerchant.create!(
source: "ai",
name: auto_detection.business_name,
website_url: auto_detection.business_url,
logo_url: build_logo_url(auto_detection.business_url)
)
rescue ActiveRecord::RecordInvalid, ActiveRecord::RecordNotUnique
# Race condition: another process created the merchant between our find and create
ProviderMerchant.find_by(source: "ai", name: auto_detection.business_name)
end
def build_logo_url(business_url)
return nil unless Setting.brand_fetch_client_id.present? && business_url.present?
"#{default_logo_provider_url}/#{business_url}/icon/fallback/lettermark/w/40/h/40?c=#{Setting.brand_fetch_client_id}"
end
def enhance_provider_merchant(merchant, auto_detection)
updates = {}

View File

@@ -0,0 +1,6 @@
class FamilyMerchantAssociation < ApplicationRecord
belongs_to :family
belongs_to :merchant
scope :recently_unlinked, -> { where(unlinked_at: 30.days.ago..).where.not(unlinked_at: nil) }
end

View File

@@ -0,0 +1,54 @@
class Merchant::Merger
class UnauthorizedMerchantError < StandardError; end
attr_reader :family, :target_merchant, :source_merchants, :merged_count
def initialize(family:, target_merchant:, source_merchants:)
@family = family
@target_merchant = target_merchant
@merged_count = 0
validate_merchant_belongs_to_family!(target_merchant, "Target merchant")
sources = Array(source_merchants)
sources.each { |m| validate_merchant_belongs_to_family!(m, "Source merchant '#{m.name}'") }
@source_merchants = sources.reject { |m| m.id == target_merchant.id }
end
private
def validate_merchant_belongs_to_family!(merchant, label)
return if family_merchant_ids.include?(merchant.id)
raise UnauthorizedMerchantError, "#{label} does not belong to this family"
end
def family_merchant_ids
@family_merchant_ids ||= begin
family_ids = family.merchants.pluck(:id)
assigned_ids = family.assigned_merchants.pluck(:id)
(family_ids + assigned_ids).uniq
end
end
public
def merge!
return false if source_merchants.empty?
Merchant.transaction do
source_merchants.each do |source|
# Reassign family's transactions to target
family.transactions.where(merchant_id: source.id).update_all(merchant_id: target_merchant.id)
# Delete FamilyMerchant, keep ProviderMerchant (it may be used by other families)
source.destroy! if source.is_a?(FamilyMerchant)
@merged_count += 1
end
end
true
end
end

View File

@@ -3,4 +3,34 @@ class ProviderMerchant < Merchant
validates :name, uniqueness: { scope: [ :source ] }
validates :source, presence: true
# Convert this ProviderMerchant to a FamilyMerchant for a specific family.
# Only affects transactions belonging to that family.
# Returns the newly created FamilyMerchant.
def convert_to_family_merchant_for(family, attributes = {})
transaction do
family_merchant = family.merchants.create!(
name: attributes[:name].presence || name,
color: attributes[:color].presence || FamilyMerchant::COLORS.sample,
logo_url: logo_url,
website_url: website_url
)
# Update only this family's transactions to point to new merchant
family.transactions.where(merchant_id: id).update_all(merchant_id: family_merchant.id)
family_merchant
end
end
# Unlink from family's transactions (set merchant_id to null).
# Does NOT delete the ProviderMerchant since it may be used by other families.
# Tracks the unlink in FamilyMerchantAssociation so it shows as "recently unlinked".
def unlink_from_family(family)
family.transactions.where(merchant_id: id).update_all(merchant_id: nil)
# Track that this merchant was unlinked from this family
association = FamilyMerchantAssociation.find_or_initialize_by(family: family, merchant: self)
association.update!(unlinked_at: Time.current)
end
end

View File

@@ -27,7 +27,25 @@ class Security < ApplicationRecord
)
end
def brandfetch_icon_url(width: 40, height: 40)
return nil unless Setting.brand_fetch_client_id.present? && website_url.present?
domain = extract_domain(website_url)
return nil unless domain.present?
"https://cdn.brandfetch.io/#{domain}/icon/fallback/lettermark/w/#{width}/h/#{height}?c=#{Setting.brand_fetch_client_id}"
end
private
def extract_domain(url)
uri = URI.parse(url)
host = uri.host || url
host.sub(/\Awww\./, "")
rescue URI::InvalidURIError
nil
end
def upcase_symbols
self.ticker = ticker.upcase
self.exchange_operating_mic = exchange_operating_mic.upcase if exchange_operating_mic.present?

View File

@@ -68,7 +68,7 @@ module Security::Provided
return
end
if self.name.present? && self.logo_url.present? && !clear_cache
if self.name.present? && (self.logo_url.present? || self.website_url.present?) && !clear_cache
return
end
@@ -81,6 +81,7 @@ module Security::Provided
update(
name: response.data.name,
logo_url: response.data.logo_url,
website_url: response.data.links
)
else
Rails.logger.warn("Failed to fetch security info for #{ticker} from #{provider.class.name}: #{response.error.message}")

View File

@@ -9,6 +9,8 @@ class Transaction < ApplicationRecord
accepts_nested_attributes_for :taggings, allow_destroy: true
after_save :clear_merchant_unlinked_association, if: :merchant_id_previously_changed?
enum :kind, {
standard: "standard", # A regular transaction, included in budget analytics
funds_movement: "funds_movement", # Movement of funds between accounts, excluded from budget analytics
@@ -39,4 +41,14 @@ class Transaction < ApplicationRecord
rescue
false
end
private
def clear_merchant_unlinked_association
return unless merchant_id.present? && merchant.is_a?(ProviderMerchant)
family = entry&.account&.family
return unless family
FamilyMerchantAssociation.where(family: family, merchant: merchant).delete_all
end
end