From e573896efe6dee2ee71fa187a869b4e593603565 Mon Sep 17 00:00:00 2001 From: BitToby Date: Sun, 15 Feb 2026 06:33:51 -0300 Subject: [PATCH] fix: locale-dependent category duplication bug (#956) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix: locale-dependent category duplication bug * fix: use family locale for investment contributions category to prevent duplicates and handle legacy data * Remove v* tag trigger from flutter-build to fix double-runs publish.yml already calls flutter-build via workflow_call on v* tags, so the direct push trigger was causing duplicate workflow runs. Co-Authored-By: Claude Opus 4.6 * Refactor mobile release asset flow * fix: category uniqueness and workflow issues * fix: fix test issue * fix: solve test issue * fix: resolve legacy problem * fix: solve lint test issue * fix: revert unrelated changes --------- Co-authored-by: Juan José Mata Co-authored-by: Claude Opus 4.6 --- .github/workflows/flutter-build.yml | 2 +- .github/workflows/mobile-release.yml | 2 +- .github/workflows/publish.yml | 2 +- app/models/category.rb | 8 ++ app/models/family.rb | 41 +++++++-- test/models/category_test.rb | 10 +++ test/models/family_test.rb | 121 +++++++++++++++++++++++++++ test/test_helper.rb | 11 ++- 8 files changed, 185 insertions(+), 12 deletions(-) diff --git a/.github/workflows/flutter-build.yml b/.github/workflows/flutter-build.yml index 886ae376f..779e852d2 100644 --- a/.github/workflows/flutter-build.yml +++ b/.github/workflows/flutter-build.yml @@ -179,4 +179,4 @@ jobs: path: | mobile/build/ios/iphoneos/Runner.app mobile/build/ios-build-info.txt - retention-days: 30 + retention-days: 30 \ No newline at end of file diff --git a/.github/workflows/mobile-release.yml b/.github/workflows/mobile-release.yml index cfe8ef986..af9962b79 100644 --- a/.github/workflows/mobile-release.yml +++ b/.github/workflows/mobile-release.yml @@ -111,4 +111,4 @@ jobs: - **Android APK**: Debug build for testing on Android devices - **iOS Build**: Unsigned iOS build (requires code signing for installation) - > **Note**: These are builds intended for testing purposes. For production use, please build from source with proper signing credentials. + > **Note**: These are builds intended for testing purposes. For production use, please build from source with proper signing credentials. \ No newline at end of file diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index b6076071e..14ccd6d53 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -465,4 +465,4 @@ jobs: echo "Push failed (attempt $attempts). Retrying in ${delay} seconds..." sleep ${delay} git pull --rebase origin $SOURCE_BRANCH - done + done \ No newline at end of file diff --git a/app/models/category.rb b/app/models/category.rb index c4239879d..02acee0f6 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -127,6 +127,14 @@ class Category < ApplicationRecord I18n.t(INVESTMENT_CONTRIBUTIONS_NAME_KEY) end + # Returns all possible investment contributions names across all supported locales + # Used to detect investment contributions category regardless of locale + def all_investment_contributions_names + LanguagesHelper::SUPPORTED_LOCALES.map do |locale| + I18n.t(INVESTMENT_CONTRIBUTIONS_NAME_KEY, locale: locale) + end.uniq + end + private def default_categories [ diff --git a/app/models/family.rb b/app/models/family.rb index 10cc39f72..7296fa205 100644 --- a/app/models/family.rb +++ b/app/models/family.rb @@ -123,14 +123,45 @@ class Family < ApplicationRecord # Returns the Investment Contributions category for this family, creating it if it doesn't exist. # This is used for auto-categorizing transfers to investment accounts. + # Always uses the family's locale to ensure consistent category naming across all users. def investment_contributions_category - categories.find_or_create_by!(name: Category.investment_contributions_name) do |cat| - cat.color = "#0d9488" - cat.classification = "expense" - cat.lucide_icon = "trending-up" + # Find ALL legacy categories (created under old request-locale behavior) + legacy = categories.where(name: Category.all_investment_contributions_names).order(:created_at).to_a + + if legacy.any? + keeper = legacy.first + duplicates = legacy[1..] + + # Reassign transactions and subcategories from duplicates to keeper + if duplicates.any? + duplicate_ids = duplicates.map(&:id) + categories.where(parent_id: duplicate_ids).update_all(parent_id: keeper.id) + Transaction.where(category_id: duplicate_ids).update_all(category_id: keeper.id) + BudgetCategory.where(category_id: duplicate_ids).update_all(category_id: keeper.id) + categories.where(id: duplicate_ids).delete_all + end + + # Rename keeper to family's locale name if needed + I18n.with_locale(locale) do + correct_name = Category.investment_contributions_name + keeper.update!(name: correct_name) unless keeper.name == correct_name + end + return keeper + end + + # Create new category using family's locale + I18n.with_locale(locale) do + categories.find_or_create_by!(name: Category.investment_contributions_name) do |cat| + cat.color = "#0d9488" + cat.classification = "expense" + cat.lucide_icon = "trending-up" + end end rescue ActiveRecord::RecordNotUnique, ActiveRecord::RecordInvalid - categories.find_by(name: Category.investment_contributions_name) + # Handle race condition: another process created the category + I18n.with_locale(locale) do + categories.find_by!(name: Category.investment_contributions_name) + end end # Returns account IDs for tax-advantaged accounts (401k, IRA, HSA, etc.) diff --git a/test/models/category_test.rb b/test/models/category_test.rb index e6dccd849..da4f9a48a 100644 --- a/test/models/category_test.rb +++ b/test/models/category_test.rb @@ -30,4 +30,14 @@ class CategoryTest < ActiveSupport::TestCase assert_equal "Validation failed: Parent can't have more than 2 levels of subcategories", error.message end + + test "all_investment_contributions_names returns all locale variants" do + names = Category.all_investment_contributions_names + + assert_includes names, "Investment Contributions" # English + assert_includes names, "Contributions aux investissements" # French + assert_includes names, "Investeringsbijdragen" # Dutch + assert names.all? { |name| name.is_a?(String) } + assert_equal names, names.uniq # No duplicates + end end diff --git a/test/models/family_test.rb b/test/models/family_test.rb index 240547c6e..bfff617be 100644 --- a/test/models/family_test.rb +++ b/test/models/family_test.rb @@ -36,6 +36,127 @@ class FamilyTest < ActiveSupport::TestCase end end + test "investment_contributions_category uses family locale consistently" do + family = families(:dylan_family) + family.update!(locale: "fr") + family.categories.where(name: [ "Investment Contributions", "Contributions aux investissements" ]).destroy_all + + # Simulate different request locales (e.g., from Accept-Language header) + # The category should always be created with the family's locale (French) + category_from_english_request = I18n.with_locale(:en) do + family.investment_contributions_category + end + + assert_equal "Contributions aux investissements", category_from_english_request.name + + # Second request with different locale should find the same category + assert_no_difference "Category.count" do + category_from_dutch_request = I18n.with_locale(:nl) do + family.investment_contributions_category + end + + assert_equal category_from_english_request.id, category_from_dutch_request.id + assert_equal "Contributions aux investissements", category_from_dutch_request.name + end + end + + test "investment_contributions_category prevents duplicate categories across locales" do + family = families(:dylan_family) + family.update!(locale: "en") + family.categories.where(name: [ "Investment Contributions", "Contributions aux investissements" ]).destroy_all + + # Create category under English family locale + english_category = family.investment_contributions_category + assert_equal "Investment Contributions", english_category.name + + # Simulate a request with French locale (e.g., from browser Accept-Language) + # Should still return the English category, not create a French one + assert_no_difference "Category.count" do + I18n.with_locale(:fr) do + french_request_category = family.investment_contributions_category + assert_equal english_category.id, french_request_category.id + assert_equal "Investment Contributions", french_request_category.name + end + end + end + + test "investment_contributions_category reuses legacy category with wrong locale" do + family = families(:dylan_family) + family.update!(locale: "fr") + family.categories.where(name: [ "Investment Contributions", "Contributions aux investissements" ]).destroy_all + + # Simulate legacy: category was created with English name (old bug behavior) + legacy_category = family.categories.create!( + name: "Investment Contributions", + color: "#0d9488", + classification: "expense", + lucide_icon: "trending-up" + ) + + # Should find and reuse the legacy category, updating its name to French + assert_no_difference "Category.count" do + result = family.investment_contributions_category + assert_equal legacy_category.id, result.id + assert_equal "Contributions aux investissements", result.name + end + end + + test "investment_contributions_category merges multiple locale variants" do + family = families(:dylan_family) + family.update!(locale: "en") + family.categories.where(name: [ "Investment Contributions", "Contributions aux investissements" ]).destroy_all + + # Simulate legacy: multiple categories created under different locales + english_category = family.categories.create!( + name: "Investment Contributions", + color: "#0d9488", + classification: "expense", + lucide_icon: "trending-up" + ) + + french_category = family.categories.create!( + name: "Contributions aux investissements", + color: "#0d9488", + classification: "expense", + lucide_icon: "trending-up" + ) + + # Create transactions pointing to both categories + account = family.accounts.first + txn1 = Transaction.create!(category: english_category) + Entry.create!( + account: account, + entryable: txn1, + amount: 100, + currency: "USD", + date: Date.current, + name: "Test 1" + ) + + txn2 = Transaction.create!(category: french_category) + Entry.create!( + account: account, + entryable: txn2, + amount: 200, + currency: "USD", + date: Date.current, + name: "Test 2" + ) + + # Should merge both categories into one, keeping the oldest + assert_difference "Category.count", -1 do + result = family.investment_contributions_category + assert_equal english_category.id, result.id + assert_equal "Investment Contributions", result.name + + # Both transactions should now point to the keeper + assert_equal english_category.id, txn1.reload.category_id + assert_equal english_category.id, txn2.reload.category_id + + # French category should be deleted + assert_nil Category.find_by(id: french_category.id) + end + end test "moniker helpers return expected singular and plural labels" do family = families(:dylan_family) diff --git a/test/test_helper.rb b/test/test_helper.rb index 867f19833..afeab9f2e 100644 --- a/test/test_helper.rb +++ b/test/test_helper.rb @@ -81,11 +81,14 @@ module ActiveSupport # Ensures the Investment Contributions category exists for a family # Used in transfer tests where this bootstrapped category is required + # Uses family locale to ensure consistent naming def ensure_investment_contributions_category(family) - family.categories.find_or_create_by!(name: Category.investment_contributions_name) do |c| - c.color = "#0d9488" - c.lucide_icon = "trending-up" - c.classification = "expense" + I18n.with_locale(family.locale) do + family.categories.find_or_create_by!(name: Category.investment_contributions_name) do |c| + c.color = "#0d9488" + c.lucide_icon = "trending-up" + c.classification = "expense" + end end end end