fix: maintain the sort order of the server sync #3748
Conversation
actual-github-bot
bot
changed the title
✅ Deploy Preview for actualbudget ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Bundle Stats — desktop-clientHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset No files were changed View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger No assets were bigger Smaller No assets were smaller Unchanged
|
Bundle Stats — loot-coreHey there, this message comes from a GitHub action that helps you and reviewers to understand how these changes affect the size of this project's bundle. As this PR is updated, I'll keep you updated on how the bundle size is impacted. Total
Changeset
View detailed bundle breakdownAdded No assets were added Removed No assets were removed Bigger
Smaller No assets were smaller Unchanged No assets were unchanged |
UnderKoen
changed the title
WalkthroughThe recent changes in the pull request focus on enhancing the The core functionality and error handling of the Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/loot-core/src/server/accounts/sync.ts (1)
415-419: LGTM! Consider using higher timestamp precision for better uniqueness.The implementation effectively maintains the sort order of transactions by using timestamp-based ordering with index offsets. The nullish coalescing operator ensures we don't override any existing sort orders.
For even better uniqueness guarantee in high-frequency scenarios, consider using higher precision timestamps:
- const now = Date.now(); + const now = process.hrtime.bigint(); added.forEach((t, index) => { - t.sort_order ??= now - index; + t.sort_order ??= Number(now) - index; });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (2)
packages/loot-core/src/server/accounts/__snapshots__/parse-file.test.ts.snapis excluded by!**/*.snapupcoming-release-notes/3748.mdis excluded by!**/*.md
📒 Files selected for processing (1)
- packages/loot-core/src/server/accounts/sync.ts (1 hunks)
🔇 Additional comments (1)
packages/loot-core/src/server/accounts/sync.ts (1)
415-419: Verify sort order usage in queries.Let's ensure that the sort order is consistently used in transaction queries throughout the codebase.
✅ Verification successful
Let me analyze the SQL usage further to ensure we have a complete picture.
Based on the gathered information, I can now provide a final response:
The sort order implementation is consistent and properly maintained
The codebase shows that:
sort_orderis properly defined as a REAL column in relevant tables- There's a dedicated index
trans_sortedon transactions that includessort_order- The implementation maintains consistency with existing sorting patterns in the database
The changes in sync.ts that set
sort_orderfor new transactions align with the database schema and existing query patterns, ensuring proper transaction ordering.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check how sort_order is used in SQL queries and table definitions # Test 1: Check table definition echo "Checking table definition for sort_order column..." rg -l "sort_order" --type sql # Test 2: Check SQL queries using sort_order echo "Checking SQL queries using sort_order..." rg "ORDER BY.*sort_order" --type tsLength of output: 2449
Script:
#!/bin/bash # Check table definitions and migrations for sort_order echo "Checking sort_order in table definitions..." rg -A 5 "sort_order" packages/loot-core/src/server/sql/init.sql # Check if sort_order is used in transaction-related tables/queries echo "Checking transaction-related sort_order usage..." rg -A 5 "transactions.*sort_order|sort_order.*transactions" --type sql --type ts # Check if there are any other sort_order manipulations in the codebase echo "Checking sort_order manipulations..." rg "\.sort_order\s*=" --type tsLength of output: 1507
|
Hi, I'm running the following: I'm still seeing this issue. Happens both on local and on server. Is there a fix for this or an option to easily re-order the transaction?
|
|
Hi @youaintnobody, could you make a new bug issue for this? As this PR only fixxed the order of transactions for bank sync. Alternatively you could upvote #1165 |
Makes actualbudget/actual-server#470 actually work.
Testing: