fix: ecurrency issues #733
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughThis PR addresses multiple issues in the eCurrency application: implementing fuzzy search with relevance-based sorting in UserService, fixing transaction history clearing on redundant account selection, preserving group account context across page refreshes, and improving mobile responsiveness by adjusting Transactions header layout. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@platforms/eCurrency/client/src/pages/currency-detail.tsx`:
- Around line 146-152: The current semantic comparison sets contextChanged
incorrectly when finalContext is null; update the logic around accountContext
and finalContext so that when both are null it returns no change. Specifically,
in the code that computes contextChanged (referencing accountContext and
finalContext), add an explicit guard: if finalContext === null && accountContext
=== null then contextChanged = false; otherwise perform the existing null-safe
comparisons (compare finalContext.type and finalContext.id to
accountContext.type and accountContext.id). This ensures re-selecting the same
(null) context does not trigger an update.
🧹 Nitpick comments (2)
platforms/eCurrency-api/src/services/UserService.ts (1)
51-53: LIKE pattern special characters are not escaped.If the search query contains SQL LIKE wildcards (
%or_), they will be interpreted as pattern characters rather than literal characters. For example, searching fortest_userwould matchtest1user,testAuser, etc.Consider escaping these characters:
♻️ Suggested fix
async searchUsers(query: string, limit: number = 10): Promise<User[]> { const q = query.trim().toLowerCase(); + const escaped = q.replace(/[%_]/g, '\\$&'); - const patternPartial = `%${q}%`; - const patternPrefix = `${q}%`; + const patternPartial = `%${escaped}%`; + const patternPrefix = `${escaped}%`;Also update the CASE statement to use escaped patterns for consistency.
platforms/eCurrency/client/src/pages/dashboard.tsx (1)
86-95: Missing semantic comparison optimization present incurrency-detail.tsx.The
handleAccountContextChangefunction incurrency-detail.tsx(lines 146-152) includes a semantic comparison that returns early if the context hasn't actually changed, preventing unnecessary transaction history clearing. This optimization is absent here, meaning clicking the currently selected account in the dashboard will still trigger state updates and clear the transaction list (issue#711).Consider applying the same pattern here for consistency:
♻️ Suggested fix
const handleAccountContextChange = (context: { type: "user" | "group"; id: string } | null) => { // If null is passed, default to user account const finalContext = context || (user ? { type: "user" as const, id: user.id } : null); + + // Check if context actually changed (semantic comparison) + const contextChanged = !accountContext || + accountContext.type !== finalContext?.type || + accountContext.id !== finalContext?.id; + + // Only update state and storage when context meaningfully changed + if (!contextChanged) return; + setAccountContext(finalContext);
Description of change
Transactions header uses flex-col sm:flex-row and gap-3 so the title and actions stack on small screens and sit in one row on larger ones.
Action buttons sit in a flex flex-wrap gap-2 container so they wrap instead of forcing horizontal scroll.
handleAccountContextChange checks semantic equality (contextChanged).
If type and id are unchanged, it returns without calling setAccountContext, so the effect that clears allTransactions and transactionOffset does not run and the list stays visible.
Search still matches only name and handle (not ename).
A numeric relevance is computed in SQL: exact match → 0, prefix match → 1, partial match → 2.
Results are ordered by that relevance, then by user.name.
eCurrency-api build completes successfully.
Issue Number
closes #712
closes #711
closes #709
closes #708
closes #695
Type of change
How the change has been tested
Change checklist
Summary by CodeRabbit
Bug Fixes
Refactor
✏️ Tip: You can customize this high-level summary in your review settings.