Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix: PG::UniqueViolation (MAYBE-RAILS-CW) #1680

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

revise-dev[bot]
Copy link

@revise-dev revise-dev bot commented Jan 24, 2025

The error PG::UniqueViolation indicates that the application is attempting to create duplicate Transfer records with the same inflow_transaction_id and outflow_transaction_id pair. This violates the unique constraint defined in the database migration.

The root cause is in the auto_match_for_account method's LEFT JOIN condition. The current implementation uses OR logic which means it only excludes transfers where either the inflow OR outflow transaction matches, but not necessarily both. This can lead to situations where a transfer exists between two transactions, but the method still tries to create another transfer between the same pair.

Changes made:

  1. Modified the LEFT JOIN condition in auto_match_for_account:

    • Changed from OR to AND logic
    • This ensures we only exclude cases where both inflow AND outflow transactions match exactly
    • Prevents duplicate transfer creation while still allowing new legitimate transfers
  2. Added a new test "auto_match_for_account does not create duplicate transfers":

    • Creates two transactions that would normally be matched
    • Manually creates a transfer between them
    • Verifies that auto_match_for_account doesn't create a duplicate
    • Uses assert_no_difference to ensure Transfer count stays the same
    • Follows existing test patterns and conventions in the file
  3. Added test "auto_match_for_account handles multiple potential matches":

    • Tests the scenario where multiple transactions could potentially match
    • Ensures correct matching behavior when there are multiple candidates
    • Verifies that only valid, non-duplicate transfers are created

The original intent of the code was to automatically match transfers between accounts by finding transactions with opposite amounts within a 4-day window. The changes preserve this functionality while properly handling the case where a transfer already exists between two transactions.

Tip

You can make revisions or ask questions of Revise.dev by using /revise in any comment or review!

  • /revise Add a comment above the method to explain why we're making this change.
  • /revise Why did you choose to make this change specifically?

Important

If something doesn't look right, click to retry this interaction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

0 participants