Skip to content

Conversation

@gfyrag
Copy link
Contributor

@gfyrag gfyrag commented Mar 5, 2025

No description provided.

@gfyrag gfyrag requested a review from a team as a code owner March 5, 2025 16:21
@coderabbitai
Copy link

coderabbitai bot commented Mar 5, 2025

Walkthrough

The changes modify the SQL conflict handling in the UpdateAccountsMetadata method by updating the case of conflict keywords and reversing the merge order for the metadata field. Additionally, the test suites have been extended: one test now validates metadata updates via the accounts metadata API, and another ensures that transactions correctly override an account’s metadata. No public API modifications were introduced.

Changes

File(s) Change Summary
internal/storage/ledger/accounts.go Updated UpdateAccountsMetadata: changed SQL conflict clause keywords (case change) and reversed the metadata merging order during upsert operations.
test/e2e/api_accounts_metadata_test.go, test/e2e/api_transactions_create_test.go Extended test suite: added a context to validate metadata updates via the accounts metadata API and a new test scenario for overriding account metadata during transaction creation.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant API
    participant Store
    participant DB

    Client->>API: Request account metadata update
    API->>Store: Call UpdateAccountsMetadata
    Store->>DB: Execute SQL with conflict handling
    DB-->>Store: Return update result
    Store-->>API: Send update confirmation
    API-->>Client: Confirm metadata update
Loading
sequenceDiagram
    participant Tester
    participant TxAPI
    participant Store
    participant DB

    Tester->>TxAPI: Create transaction with metadata override
    TxAPI->>Store: Process transaction with new metadata
    Store->>DB: Execute SQL update for metadata override
    DB-->>Store: Return updated record
    Store-->>TxAPI: Confirm override success
    TxAPI-->>Tester: Transaction and metadata update successful
Loading

Suggested reviewers

  • Dav-14
  • flemzord

Poem

I'm a bunny with a skip and a hop,
Leaping through changes that never stop.
Metadata now merges with a twist so neat,
SQL conflict fixed—oh, what a treat!
My whiskers twitch with joyous glee,
Celebrating code that sets us free!
🐰 Happy hops in every byte!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between be31a3e and 2565942.

📒 Files selected for processing (3)
  • internal/storage/ledger/accounts.go (1 hunks)
  • test/e2e/api_accounts_metadata_test.go (2 hunks)
  • test/e2e/api_transactions_create_test.go (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Tests
🔇 Additional comments (4)
internal/storage/ledger/accounts.go (1)

51-52: Key change: SQL conflict handling is now more predictable for metadata merging.

The case of SQL keywords has been lowered, which is fine as PostgreSQL is case-insensitive for keywords. The critical change here is the metadata merge order. Now, when there's a conflict, the incoming metadata (excluded.metadata) will override the existing metadata (accounts.metadata) rather than the other way around.

This aligns with the PR objective to fix metadata override behavior, ensuring new values take precedence over existing ones.

test/e2e/api_accounts_metadata_test.go (1)

77-111: Good test coverage for updating account metadata!

This new test case verifies that when account metadata is updated with the same key ("clientType") but a different value ("silver" replacing "gold"), the new value properly overrides the old one. This directly tests the behavior fixed in the UpdateAccountsMetadata method.

test/e2e/api_transactions_create_test.go (2)

83-119: Excellent test for metadata override via transactions!

This test validates that metadata override works correctly when modified through a transaction script using set_account_meta. It ensures that the account metadata is properly updated from "gold" to "silver", confirming that the fix works for both direct metadata updates and updates via transaction scripts.


95-102: Code style is consistent with the project standards.

The script format with indentation and the use of set_account_meta function follows the established patterns in other transaction tests. The test properly sets up the conditions (pre-existing metadata) and then verifies the expected outcome.

✨ Finishing Touches
  • 📝 Generate Docstrings

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@codecov
Copy link

codecov bot commented Mar 5, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.01%. Comparing base (be31a3e) to head (2565942).
Report is 2 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #734      +/-   ##
==========================================
+ Coverage   81.97%   82.01%   +0.03%     
==========================================
  Files         135      135              
  Lines        7291     7290       -1     
==========================================
+ Hits         5977     5979       +2     
+ Misses       1008     1006       -2     
+ Partials      306      305       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@gfyrag gfyrag enabled auto-merge March 5, 2025 16:42
@gfyrag gfyrag added this pull request to the merge queue Mar 5, 2025
Merged via the queue into main with commit 523ac1c Mar 5, 2025
10 checks passed
@gfyrag gfyrag deleted the fix/metadata-override branch March 5, 2025 16:45
@gfyrag gfyrag mentioned this pull request Mar 14, 2025
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.

5 participants