Skip to content

Commit 019282b

Browse files
wesmclaude
andcommitted
fix: add duplicate target payee check in batch_update_merchant
Added defensive check to prevent unsafe merges when the target payee has duplicates. Previously, the code only checked for duplicates in the source payee, but not the target. Changes: - Check target_payee_result["duplicates_found"] before merging - Return error with method="duplicate_target_payees_found" if found - Add integration test with mocking to verify the check works This prevents transactions from being assigned to the wrong payee when duplicate target payees exist (rare but possible via imports). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 (1M context) <noreply@anthropic.com>
1 parent 4ccedef commit 019282b

File tree

2 files changed

+69
-0
lines changed

2 files changed

+69
-0
lines changed

integration_tests/test_ynab_payee_operations.py

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -254,6 +254,55 @@ def test_find_payee_detects_single_payee(
254254
assert result["duplicates_found"] is False
255255
assert result["duplicate_ids"] == []
256256

257+
def test_batch_update_rejects_duplicate_target_payee(
258+
self,
259+
ynab_client: YNABClient,
260+
create_test_transaction: Callable[..., Dict[str, Any]],
261+
test_payee_prefix: str,
262+
):
263+
"""
264+
Test that batch_update_merchant rejects merges when target has duplicates.
265+
266+
Note: YNAB's API typically prevents creating duplicate payees, so this
267+
test verifies the defensive check is in place even though duplicates
268+
are rare in practice (usually only happen via manual UI operations or
269+
imports). We test this by mocking the duplicate detection response.
270+
"""
271+
from unittest.mock import patch
272+
273+
source_name = "SourcePayee_DuplicateTargetTest"
274+
target_name = "TargetPayee_DuplicateTargetTest"
275+
full_source = f"{test_payee_prefix}{source_name}"
276+
full_target = f"{test_payee_prefix}{target_name}"
277+
278+
# Create source payee via transaction
279+
create_test_transaction(source_name, -10.00)
280+
281+
# Create target payee via transaction
282+
target_txn = create_test_transaction(target_name, -20.00)
283+
284+
# Mock _find_or_create_payee to simulate duplicate target payees
285+
original_find = ynab_client._find_or_create_payee
286+
287+
def mock_find_payee(merchant_name: str):
288+
result = original_find(merchant_name)
289+
# If this is the target payee, simulate duplicates
290+
if merchant_name == full_target and result["payee"]:
291+
result["duplicates_found"] = True
292+
result["duplicate_ids"] = [result["payee"].id, "fake-duplicate-id"]
293+
return result
294+
295+
with patch.object(ynab_client, "_find_or_create_payee", side_effect=mock_find_payee):
296+
# Try to merge source into target (which has duplicates)
297+
result = ynab_client.batch_update_merchant(full_source, full_target)
298+
299+
# Should fail with duplicate target error
300+
assert result["success"] is False
301+
assert result["method"] == "duplicate_target_payees_found"
302+
assert "duplicate" in result["message"].lower()
303+
assert "duplicate_ids" in result
304+
assert len(result["duplicate_ids"]) == 2
305+
257306
def test_batch_update_with_nonexistent_payee(
258307
self,
259308
ynab_client: YNABClient,

moneyflow/ynab_client.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -460,6 +460,26 @@ def batch_update_merchant(
460460
# Check if target payee name already exists
461461
target_payee_result = self._find_or_create_payee(new_merchant_name)
462462
if target_payee_result["payee"]:
463+
# Check for duplicates in target - abort if found
464+
if target_payee_result["duplicates_found"]:
465+
logger.error(
466+
f"Found duplicate target payees with name '{new_merchant_name}': "
467+
f"{target_payee_result['duplicate_ids']}. Cannot safely perform batch merge. "
468+
"Please merge duplicate payees in YNAB first."
469+
)
470+
return {
471+
"success": False,
472+
"payee_id": None,
473+
"transactions_affected": 0,
474+
"method": "duplicate_target_payees_found",
475+
"message": (
476+
f"Target payee '{new_merchant_name}' has "
477+
f"{len(target_payee_result['duplicate_ids'])} duplicates. "
478+
"Merge duplicates in YNAB first."
479+
),
480+
"duplicate_ids": target_payee_result["duplicate_ids"],
481+
}
482+
463483
# Target payee already exists - use batch transaction update to merge
464484
target_payee = target_payee_result["payee"]
465485
logger.info(

0 commit comments

Comments
 (0)