fix: Remove old personal card model - 1 #3397
Conversation
sumrender
requested review from
Dimple16
and removed request for
arjunaj5 and
Chethan-Fyle
|
Caution Review failedThe pull request is closed. WalkthroughIn this epic saga of code transformation, we’re migrating personal cards to the platform API with the swagger of a superstar! The changes span multiple files, focusing on replacing Changes
Suggested labels
Suggested reviewers
Poem
Possibly related PRs
Thalaiva's code style: Simple, powerful, and always a blockbuster! 😎🔥 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (32)
🪧 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: 7
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (10)
src/app/core/mock-data/personal-cards.data.ts(1 hunks)src/app/core/services/personal-cards.service.spec.ts(2 hunks)src/app/core/services/personal-cards.service.ts(2 hunks)src/app/fyle/personal-cards/personal-cards.page.spec.ts(8 hunks)src/app/fyle/personal-cards/personal-cards.page.ts(6 hunks)src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.html(2 hunks)src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.spec.ts(3 hunks)src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.ts(4 hunks)src/app/shared/components/bank-account-cards/bank-account-cards.component.spec.ts(3 hunks)src/app/shared/components/bank-account-cards/bank-account-cards.component.ts(2 hunks)
🔇 Additional comments (28)
src/app/fyle/personal-cards/personal-cards.page.ts (5)
78-78: Excellent shift to PlatformPersonalCard, boss!
ChanginglinkedAccounts$toObservable<PlatformPersonalCard[]>keeps it consistent with the new model. Keep rocking!
110-110: Mighty transformation for selectedAccount, Superstar!
SwitchingselectedAccounttoPlatformPersonalCardaligns with the platform-based approach. Smooth transition indeed.
179-179: Aiyoo, direct usage of getPersonalCards is good, but have you tested?
Ensure everything callingloadLinkedAccountshas been updated to handlePlatformPersonalCard[]responses seamlessly.
211-211: The counting logic is simpler, da!
Using the newgetPersonalCardsCount()is a clean approach. Confirm that upstream usage has also been updated, boss.
406-406: Method signature changed like lightning, pa!
Ensure that event bindings and references toonCardChangednow passPlatformPersonalCardobjects. No mismatch, right?src/app/shared/components/bank-account-cards/bank-account-cards.component.ts (3)
2-2: Full power import, boss!
You're now importingPlatformPersonalCardinstead of the old model. Perfect for the new architecture.
15-15: Unleashing an array of PlatformPersonalCards, da!
@Input() linkedAccountsaligns with the reworked service model. This is neat and tidy.
21-21: Event emitter reloaded!
EmittingPlatformPersonalCardis consistent with the rest of the changes. No problem there.src/app/shared/components/bank-account-cards/bank-account-cards.component.spec.ts (3)
5-5: Swapping mocks, good move!
UsinglinkedAccountsin tests ensures the spec matches the updated code. Everything is big-time consistent.
22-22: Be sure your test data is correct, da!
Assigningcomponent.linkedAccounts = linkedAccountsis the right step. Just confirm the fixture runs with no issues.
43-43: Your test calls are correct, superstar!
component.changed.emitwithlinkedAccounts[1]ensures the spec aligns with the updated emitter.src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.ts (4)
2-2: PlatformPersonalCard import is matching your grand vision!
No more confusion between the old and new models—a sweet transition, boss.
21-21: Account details upgraded to the next level, da!
@Input() accountDetails: PlatformPersonalCardkeeps the code base consistent with the updated data flow.
42-45: Mighty replacement for last synced date, Superstar!
Usingyodlee_last_synced_atinstead of the older property is correct. Perfect alignment with the new platform approach.
93-93: Boss, no more old references!
Switching fromaccount_numbertocard_numberensures the UI message fits the new object structure.src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.spec.ts (3)
10-10: Looking good, my friend!Switching over to
linkedAccountsfrom the olderapiLinkedAccResis a wise move for consistency. Great job on maintaining code clarity!
81-81: You are unstoppable!Using
linkedAccounts[1]improves test clarity here. Just confirm that no test scenario or data coverage got lost in this shift.
159-159: All set, boss!Modifying the confirmation message to display
bank_nameandcard_numberis a superb user-friendly approach. Just double-check if any place in the code still relies onaccount_number.src/app/fyle/personal-cards/personal-cards.page.spec.ts (8)
26-26: Superb import usage, da!Ensuring
linkedAccountsis imported neatly here keeps your tests consistent and fosters clarity across your codebase.
217-217: Lightning-fast reflexes!Substituting
of(linkedAccounts)in place of older data sources keeps your tests aligned with the new model. Let’s keep an eye out for references to old mocks just in case.
354-354: All good, ThalaivaaAssigning “card changed” logic to the newly minted array suits the flow. Excellent job!
369-369: Thorough approach, boss!Handling null-ish query params gracefully is crucial—super smooth.
484-484: Mind-blowing consistency, my friendUsing
linkedAccounts[0]infetchNewTransactionsis coherent with your data transition. Keep verifying that the correct references are used for each test.
885-885: Dashing, I say!Using
linkedAccounts[0]again for theopenYoodleflow ensures uniform test data. Double-check you’re capturing any mismatch in real usage vs. test usage.
979-979: Rock-solid coverage, da!The
loadLinkedAccountsmethod leverages the updatedgetPersonalCardsreturn. Good synergy between service and test.
984-984: Kanakku (accounting) checks done rightThe final assertion ensures the correct data is being loaded. Looks perfect.
src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.html (2)
31-31: 🧹 Nitpick (assertive)Style consistency is like my signature walk - it should be perfect!
Hey boss! You've made the same card_number change in the minimal view. That's style! But remember, when we're making such powerful moves, we need to ensure our documentation and tests are updated too!
Consider adding a tooltip or info icon to explain the switch from account to card numbers to our users. They should feel the power of this change! 💫
Let me verify the documentation updates:
17-17: Mind it! Verify the card number masking, macha!Listen here, partner! When you're showing card numbers instead of account numbers, you better make sure they're properly masked like a superhero's secret identity! 🕶️
Let me help you verify the masking implementation with this powerful script:
✅ Verification successful
Mind it! Your card numbers are safe and secure, thalaiva!
Listen here, my friend! After investigating the codebase like a true detective, I can tell you that your card numbers are as protected as Fort Knox! Here's why, style-ah:
- The
MaskNumberpipe is doing its job perfectly, showing only the last 4 digits with asterisks in front - that's the style! 🕶️- The dedicated
CardNumberComponentis showing those fancy dots for the first 12 digits and only the last 4 digits of the number - extra protection, extra style!- Both components are properly imported in the shared module, working together like a well-choreographed action sequence!
The masking implementation is solid as a rock, partner! Your card numbers will be displayed with the same security as my signature sunglasses - revealing just enough, but keeping the important parts hidden! 😎
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for proper card number masking implementation # Look for masking pipes or functions in the codebase echo "Searching for card number masking implementations..." rg -t ts "mask.*card" echo "Checking for any potential card number leaks..." ast-grep --pattern 'card_number'Length of output: 1112
Script:
#!/bin/bash # Let's check the component's TypeScript file for masking logic echo "Checking the component's TypeScript implementation..." rg -p "bank-account-card.component.ts" -A 10 -B 10 # Let's also check if there's a dedicated card number component being used echo "Checking card number component implementation..." cat src/app/shared/components/card-number/card-number.component.ts # Check for any masking pipes in the shared module echo "Checking for masking pipes..." fd "mask.*pipe.ts"Length of output: 824
Script:
#!/bin/bash # Let's check the masking pipe implementation echo "Checking the masking pipe implementation..." cat src/app/shared/pipes/mask-number.pipe.ts # Let's also check if the card-number component's template has proper masking echo "Checking card number component's template..." cat src/app/shared/components/card-number/card-number.component.html # Check if the bank-account-card component uses any masking pipes echo "Checking bank account card component's module imports..." rg -p "bank-account-card.module.ts"Length of output: 1521
Script:
#!/bin/bash # Let's check if the bank-account-card component imports the masking pipe echo "Checking bank account card component's declarations..." rg -p "bank-account-card" -g "*.module.ts" -B 5 -A 10 # Let's also check the bank account card component's typescript file echo "Checking bank account card component implementation..." rg -p "bank-account-card.component.ts" -g "*.ts" -B 5 -A 10Length of output: 2422
| personalCardsService.isMfaEnabled.and.returnValue(of(false)); | ||
| component.loadData$ = new BehaviorSubject({ | ||
| pageNumber: 1, | ||
| }); | ||
| component.loadCardData$ = new BehaviorSubject({}); | ||
| component.linkedAccountsCount$ = of(1); | ||
| component.isConnected$ = of(true); | ||
| component.linkedAccounts$ = of(linkedAccountsRes); | ||
| component.linkedAccounts$ = of(linkedAccounts); |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
You’re rocking it, buddy!
Exposing linkedAccounts$ as an observable is a good pattern—very maintainable. Check if you need advanced test coverage for various data states (like an empty array).
* delete personal_card.model.ts and places where it was used * fix: remove model PersonalCardTxnExpenseSuggestion (#3399) * fix: remove model PersonalCardTxnExpenseSuggestion * fix: transform personal card public queryParams to platform (#3400) * fix: transform personal card public queryParams to platform * fix: remove use of old PersonalCardTxn model (#3401) * remove use of old PersonalCardTxn model * delete public PersonalCardTxn model and corresponding ldFlag (#3402)
sumrender
merged commit dbdf5f0
into
remove-old-code-ld-flag-for-personal-cards
There was a problem hiding this comment.
Actionable comments posted: 15
🔭 Outside diff range comments (5)
src/app/fyle/personal-cards/personal-cards.page.ts (2)
Line range hint
747-782: The logic is solid, but let's add some style!Like how I split my action sequences into perfect shots, let's split this method for better readability!
Extract navigation logic into separate methods:
private navigateToAddEditExpense(txnDetails: PlatformPersonalCardTxn): void { this.router.navigate([ '/', 'enterprise', 'add_edit_expense', { personalCardTxn: JSON.stringify(txnDetails), navigate_back: true }, ]); } private navigateToMatchedExpenses( txnDetails: PlatformPersonalCardTxn, expenseSuggestions: Expense[] ): void { this.router.navigate(['/', 'enterprise', 'personal_cards_matched_expenses'], { state: { personalCard: this.selectedAccount, txnDetails, expenseSuggestions, }, }); }
Line range hint
495-501: Handle errors with style, like how I handle villains!API calls need proper error handling to show user-friendly messages when things go wrong.
Add error handling to API calls:
this.personalCardsService .getMatchedExpensesSuggestions(txnDetails.amount, txnDate) .pipe( catchError((error) => { this.matSnackBar.openFromComponent(ToastMessageComponent, { ...this.snackbarProperties.setSnackbarProperties('error', { message: 'Failed to fetch expense suggestions. Please try again.', }), panelClass: ['msb-error'], }); return EMPTY; }), finalize(() => { this.loadingMatchedExpenseCount = false; this.loadingTxnId = null; }) ) .subscribe(/* ... */);Also applies to: 759-782
src/app/fyle/add-edit-expense/add-edit-expense-3.spec.ts (3)
Line range hint
4-9: Enhance test organization with describe blocks, macha!The test suite could benefit from better organization. Consider grouping related test cases using nested describe blocks for better readability and maintenance.
-describe('AddEditExpensePage-3', () => { +describe('AddEditExpensePage', () => { + describe('Personal Card Transaction Tests', () => { + // Personal card related test cases + }); + + describe('File Attachment Tests', () => { + // File attachment related test cases + });
Line range hint
1-3: Add test coverage for error scenarios, thalaiva!The test file focuses mainly on happy path scenarios. We should also test error handling and edge cases.
Add test cases for:
- Network failures during file uploads
- Invalid personal card data
- API error responses
- Race conditions in concurrent operations
Line range hint
1494-1523: Add JSDoc comments for better test documentation, boss!The test cases would benefit from proper JSDoc documentation explaining the test scenarios and expectations.
+/** + * Test suite for personal card transaction handling + * Verifies the behavior when creating expenses from personal card transactions + */ it('should get new expense observable from personal card txn and home currency does not match extracted data', (done) => {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (32)
src/app/core/mock-data/get-tasks-query-params-with-filters.data.ts(2 hunks)src/app/core/mock-data/personal-card-txn-expense-suggestions.data.ts(5 hunks)src/app/core/mock-data/personal-card-txns.data.ts(3 hunks)src/app/core/mock-data/personal-cards.data.ts(1 hunks)src/app/core/mock-data/unflattened-txn.data.ts(1 hunks)src/app/core/models/personal-card-txn-expense-suggestion.model.ts(0 hunks)src/app/core/models/personal_card.model.ts(0 hunks)src/app/core/models/personal_card_txn.model.ts(0 hunks)src/app/core/models/platform/platform-personal-card-filter-params.model.ts(1 hunks)src/app/core/models/platform/platform-personal-card-query-params.model.ts(1 hunks)src/app/core/models/platform/platform-personal-card-txn.model.ts(1 hunks)src/app/core/services/personal-cards.service.spec.ts(25 hunks)src/app/core/services/personal-cards.service.ts(13 hunks)src/app/fyle/add-edit-expense/add-edit-expense-3.spec.ts(2 hunks)src/app/fyle/add-edit-expense/add-edit-expense-4.spec.ts(11 hunks)src/app/fyle/add-edit-expense/add-edit-expense.page.ts(4 hunks)src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.spec.ts(1 hunks)src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.ts(3 hunks)src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.html(1 hunks)src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.spec.ts(6 hunks)src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.ts(3 hunks)src/app/fyle/personal-cards/personal-cards.page.html(1 hunks)src/app/fyle/personal-cards/personal-cards.page.spec.ts(27 hunks)src/app/fyle/personal-cards/personal-cards.page.ts(25 hunks)src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.spec.ts(5 hunks)src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.ts(5 hunks)src/app/shared/components/expense-card-lite/expense-card-lite.component.html(1 hunks)src/app/shared/components/expense-card-lite/expense-card-lite.component.spec.ts(3 hunks)src/app/shared/components/expense-card-lite/expense-card-lite.component.ts(1 hunks)src/app/shared/components/personal-card-transaction/personal-card-transaction.component.html(2 hunks)src/app/shared/components/personal-card-transaction/personal-card-transaction.component.spec.ts(5 hunks)src/app/shared/components/personal-card-transaction/personal-card-transaction.component.ts(2 hunks)
💤 Files with no reviewable changes (3)
- src/app/core/models/personal_card.model.ts
- src/app/core/models/personal-card-txn-expense-suggestion.model.ts
- src/app/core/models/personal_card_txn.model.ts
🧰 Additional context used
🪛 Biome (1.9.4)
src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.ts
[error] 65-65: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
src/app/fyle/personal-cards/personal-cards.page.ts
[error] 81-81: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
[error] 233-233: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gitStream.cm
🔇 Additional comments (186)
src/app/shared/components/expense-card-lite/expense-card-lite.component.ts (2)
10-10: Aiyoo, this change from partial to full type is mighty bold!
Thalaiva, converting@Input()from a partial type toExpenseis a big move. Make sure every place usingexpenseis ready to handle all the fields, da!
19-19: One punch, no fetch calls needed!
Superstar logic, man! Checkingfile_ids?.lengthdirectly saves us from extra service calls. Perfect for ensuring the receipts are recognized with lightning speed.src/app/shared/components/personal-card-transaction/personal-card-transaction.component.html (2)
3-3: Mind it! The date binding looks perfect, macha!The change to use
transaction.spent_atwith the dateFormat pipe maintains clean date formatting while aligning with the new transaction model. Style ah iruku! 🕶️
23-23: Superstar style description binding, thalaiva! 🌟Clean and simple change to use
transaction.description. When you do something, do it with style!src/app/shared/components/expense-card-lite/expense-card-lite.component.html (2)
28-28: Mind it! The vendor property transformation looks perfect!The change from vendor to merchant property is exactly what a superstar would do - clean, direct, and powerful! This aligns perfectly with our platform API migration, just like how I align my sunglasses before a dramatic entry!
32-32: Kabali da! The date property change is spot on!The transformation from txn_dt to spent_at is as smooth as my signature walk. The date pipe format remains unchanged, maintaining the style while upgrading the substance - that's how we roll in the platform API world!
But remember one thing: When you change properties like this, you better make sure your TypeScript component is ready for it!
✅ Verification successful
Mind it! Your TypeScript is as solid as my style!
The component is perfectly aligned with the platform API changes. It's importing and using the correct Expense interface from the v1 models, which includes the spent_at property. The force is strong with this one!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the component's property usage ast-grep --pattern 'class ExpenseCardLiteComponent { $$$ expense: $_ { $$$ spent_at: $_ $$$ } $$$ }'Length of output: 122
Script:
#!/bin/bash # Find the component file fd -e ts expense-card-lite.component # Search for the component class and expense-related definitions rg -l "class ExpenseCardLiteComponent" # Look for expense type/interface definitions rg "(?:interface|type) .*Expense.*\{" -A 10Length of output: 36222
Script:
#!/bin/bash # Check the component file content rg -A 10 "class ExpenseCardLiteComponent" src/app/shared/components/expense-card-lite/expense-card-lite.component.ts # Look for expense property definition rg "expense\s*:\s*" src/app/shared/components/expense-card-lite/expense-card-lite.component.ts # Check imports rg "^import.*Expense" src/app/shared/components/expense-card-lite/expense-card-lite.component.tsLength of output: 669
src/app/core/mock-data/unflattened-txn.data.ts (1)
1376-1376: Superb change, boss!
Settingpurposeto'mocha'is neat and doesn't break anything.src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.ts (5)
2-2: Sensational import, da!
Aiyoo, I see you're now importingPlatformPersonalCardinstead of the old model. This is a neat shift to the new structure.
20-20: Rocking new Input property!
Changing@Input() accountDetailstoPlatformPersonalCardpaves the way for the new data model. Perfect synergy!
36-36: Injecting dateService like a superstar!
This injection will help manage date conversions properly. Good job, da!
40-43: Aiyoo, local date conversion, superb!
This block checksyodlee_last_synced_atand converts it to local time. Great clarity for user display.
83-83: Card number reference, semma correct!
Usingcard_numberinstead ofaccount_numberin the confirmation message is more relevant for a personal card.src/app/core/mock-data/personal-card-txn-expense-suggestions.data.ts (7)
2-6: Machi, robust typing, la!
ImportingPlatformApiResponse,Expense,ApprovalState,AccountType, andExpenseStatedemonstrates strong code clarity.
8-8: Encouraging typed response, superb!
ExportingplatformPersonalCardTxnExpenseSuggestionsResasPlatformApiResponse<Expense[]>is a neat type-safety step.
25-25: Pending approval label, well done!
UsingApprovalState.APPROVAL_PENDINGis more descriptive than plain strings, oh yes!
49-49: Date object usage, super star coding!
Explicitly instantiatingDateprevents confusion with date strings. Good for maintaining consistent date handling.
167-167: Enum usage for account type, mass!
AccountType.PERSONAL_CASH_ACCOUNTfosters readability and reduces potential literal typos.
174-174: State asExpenseState.DRAFT
Aiyoo, your use of enum-based expense states is dynamic and clean.
183-183: Updated date for a modern code base!
new Date(...)keeps it consistent with the rest of the typed approach.src/app/core/mock-data/personal-card-txns.data.ts (6)
10-11: Start with a count of 3, nice!
DeclaringplatformPersonalCardTxnswithcount: 3clarifies how many transactions we handle, da!
14-47: Hefty object, well-structured boss!
Defining fields liketransactionType,matched_expenses, andspent_atas typed data is grand for consistency.
50-50: Still on debit track, super!
Your second transaction object also uses'debit', matching the scenario.
59-59: Unique ID, btxndbZdAth0x5
Neat to keep IDs distinct for each transaction illustration.
81-84: MATCHED state, abso-fantastic!
Marking the transaction asPlatformPersonalCardTxnState.MATCHEDensures we reflect actual matched status.
85-102: Third transaction, splendid structure!
Another well-populated object with no matched expenses, good for negative testing scenarios.src/app/core/services/personal-cards.service.spec.ts (52)
8-8: Machi, grouping imports is good!
IncludingdeletePersonalCardPlatformResandplatformApiLinkedAccRessets the stage for thorough service tests.
25-25: Res for suggestions cleverly imported
Bringing inplatformPersonalCardTxnExpenseSuggestionsResensures wide coverage.
78-80: Transaction type addition is a breeze
The test verifiesaddTransactionTypeToTxnsseamlessly, well done!
89-90: Platform approach test, mass!
CheckinggetPersonalCards()with the new platform route is thorough and matches real usage.
102-102: Counting personal cards, semma logic
Ensures the platform count fetch is tested properly.
122-122: Deleting account test, bold move
Confirms the service calls the correct platform endpoint.
135-136: Fetching bank transactions
Uses platform route with correct params, verifying the reactive approach.
151-153: Transaction count check
You revolve around the same platform service for count—makes sense, boss!
163-163: Linking personal cards
Your test forpostBankAccounts()on platform is well-structured.
204-205: Credit param test
Ensures'transactionType'is respected in query.
212-212: Generate credit params
Essential check for the function’s logic branching.
214-215: Asserting combined filters
State'in.(INITIALIZED)'and'personal_card_id'– good coverage!
222-223: Another param scenario
Testing'Debit'transaction type ensures robust branching, la!
231-231: Refined param generation
Ensuring no conflict with existing states.
233-234: GTE for debit
Condition ensures transactions are not missed.
241-242: Still verifying param structure
Clarity in code coverage.
249-250: Follow-up state check
Ensuring repeated calls can handle repeated states.
256-257: No transactionType scenario
Nice for testing fallback logic.
262-263: Params remain untouched
Ensuring we do not override if filters are empty.
270-273: Fine date creation
Starting date param usage for custom range.
285-285: Query param name
const queryType = 'created_at'is easy to track.
290-291: Thorough or-check
Double-checking logical and/or combos.
293-294: One more state + personal_card_id
Ensures everything is consistent, boss.
299-302: Another custom date scenario
We are verifying code for partial date usage.
313-313: Still referencing created_at
Short but important coverage.
318-319: Appending more or filters
No collisions—very nice.
321-322: Ensuring param merges
Nalla choice.
327-330: Checking end date usage
Completes the custom range scenario.
341-341: Still setting queryType
No confusion with the code.
346-347: Double-lining created_at
Never skipping lines for coverage.
349-350: All states intact
We remain consistent in param usage.
615-615: Date param generation test
“Aiyoo, good test for generateDateParams sake!”
625-628: This Month scenario
Focus on'spent_at'date range—makes sense!
635-637: Final check on or
Ensuring'spent_at'filters remain correct.
652-655: Last month scenario
Working for'spent_at'date with bounding times.
662-664: Asserting date range
We confirm those filter combos again.
680-683: Last 30 days
Again verifying param generation.
690-692: Check or-lists
No overlap or missing date constraints.
707-710: Last 60 days
Ensuring multiple day offsets work.
717-719: All time scenario
Using 90 days as a fallback is nice.
734-737: Custom range usage
Focus on user-specified start/end date.
744-746: Pipes in action
We confirm the subscription’s result.
759-762: Another date param
(and(spent_at.gte...,spent_at.lt...))tested well.
769-771: Chaining the or
We do not hamper the rest.
793-796: This week check
Kitchen-sink for date range coverage.
799-799: loadData$.next usage
Observables synergy—nice job!
805-806: No conflict
Ensuring final param merges.
875-875: Selected account
Makes sure future calls know which card is in use.
903-903: Posting accounts
Matches previously tested.
999-999: transactions$ with data
Confirms we handle the correct live array.
1008-1008: MFA enabled scenario
We ensureisMfaEnabledtest coverage, la!
1017-1017: Line 1017
Confirms we pass correct data param for MFA check.src/app/fyle/personal-cards/personal-cards.page.spec.ts (36)
25-26: Smart new imports
platformPersonalCardTxnsandlinkedAccountsattest to the platform’s new wave.
44-45: Bringing suggestions and states
These new imports ensure test coverage formatchedExpenseSuggestionsandPlatformPersonalCardTxnState.
208-208:getPersonalCardsupdated
Yes, returninglinkedAccountsis in line with the new data model.
216-216:linkedAccounts$replaced
Switching to real data from mock for further test consistency.
218-218: transactions$ sample
Using[platformPersonalCardTxns.data[0]]helps ensure correct test coverage.
277-281: Post accounts, exact test
Macha, returninglinkedAccountsis sharper. Also ensures proper creation.
286-286: Verifying post call
We see correct usage param—no old references.
308-312: Multiple card scenario
Again checks if we handle more cards returned.
317-317: One more post check
Yes, final assertion for call count.
343-346: Card changed
Applying'personal_card_id'for'baccY70V3Mz048'is spot on.
358-361: No pre-existing params
We see the test logic handles uninitialized queryParams gracefully.
Line range hint
414-432: Segment switch
SettingINITIALIZEDensures coverage of default state.
473-475: Sync new transactions
SelectinglinkedAccounts[0]then callingsyncTransactions. Perfect chaining, boss!
Line range hint
615-615: No direct snippet shown
But referencing that line indicates an approach to handle date filters.
Line range hint
627-628: Even more query param
Ensuring'state': 'in.(INITIALIZED)','personal_card_id':'eq.baccLesaRlyvLY'.
Line range hint
635-635: Transaction counting
Confirms base limit.
Line range hint
652-655: Again verifying flow
Test ensures repeated call is consistent.
Line range hint
662-662: Filtering synergy
Focus on merging user filters with default.
Line range hint
680-683: Ensuring last 30 days
Same approach, less duplication.
690-690: Another or extension
We keep adding coverage.
707-710: Another date verify
Yes,range: 'All Time'or'Custom Range'shaped here.
717-723: Navigate matched expenses
Ensures we route user topersonal_cards_matched_expenseswith correct data.
732-732: Exit creation
If page is loading or in selection mode, we skip.
742-744: Expense already matched
We open expense preview instead of re-creating.
Line range hint
749-766:openExpensePreviewusage
Forming correct modal with ID from matched expense. Semma clarity, boss!
793-796: DateRangeModal checks
We ensure correct flow for custom date selection.
Line range hint
799-806: Assigning or filter
Then callingloadData$.nextis a typical pattern for reactive updates.
838-839: Adding new filters
ConfirmstransactionTypeand ID are used.
969-974: Re-loading linked accounts
Observing new data fromgetPersonalCards()—top class.
995-995: Checking infinite scroll
Ensures we only show it if transactions exceed the limit.
Line range hint
999-999: transactions$ assigned
We test final assignment for scrolled items.
Line range hint
1008-1008: TestingisMfaEnabled
Returning a boolean for the card ID. Superstar logic.
Line range hint
1017-1017: MFA param
Ensures we post correct ID to the platform endpoint.
1025-1028: loadPersonalTxns with params
Demonstrates call order for filters andextendQueryParamsForTextSearch.
1040-1040: No bank transactions
We verify we skip service calls if count is zero.
1057-1057: Lifecycle test
Ensures we doloadAccountCount,loadLinkedAccounts, andloadPersonalTxnson init.src/app/core/models/platform/platform-personal-card-query-params.model.ts (1)
2-6: Making all these fields mandatory, huh? Super star says: “Verify the callers!”
Aiyoo, since you have removed the question marks, remember that every place that constructsPlatformPersonalCardQueryParamsmust now supply these properties. If any part of the code used to rely on the optional nature, it may cause runtime errors, thalaiva!src/app/core/models/platform/platform-personal-card-filter-params.model.ts (2)
1-1: New import statement from the top? Rajini salutes your foresight!
Good job importingPlatformPersonalCardQueryParams. This sets you up for flexible usage of the new interface everywhere else, pa!
3-7: New interface creation looks stylish, macha!
‘PlatformPersonalCardFilterParams’ is introduced withpageNumber,searchString, and partialqueryParams. This is neat for paging and filtering, but be mindful to test differently typed data forsearchStringand partial properties inqueryParamsto avoid runtime drama, thambi!src/app/shared/components/personal-card-transaction/personal-card-transaction.component.ts (7)
3-3: Rolling out the big star import, la?
ImportingPlatformPersonalCardTxnis a grand way to unify the data model. No more scattered definitions—this is the real mass entry!
13-13: Lucky number 13, new propertypreviousTxnDate, tha?
Make sure you elegantly handle potential null values or date mismatches for older transactions, especially if the user leaps around like a Rajini fight scene.
23-23:setMultiselectModeevent emitter? Semma style!
You’re elegantly bridging your component to the outside world. Keep that output well-named and tested, boss!
29-29: New propertycurrency? Nalla velai!
You’re storing the currency symbol separate from the raw data. This is good for display logic. Keep verifying this matches the actual transaction currency.
33-34:isSelectedgetter: crisp as a Rajini punchline!
Usingsome()is a good approach, da! Confirm performance with large arrays, though it's typically fine.
38-38: Dynamite styling, match currency and date elegantly.
getCurrencySymbol(...)is brilliant, thambi. Also a neat way to show/hide the date if it matches the previous one. Superb logic.
50-52:onTapTransaction: smooth as a Rajini bullet dodge!
Similarly, you emittransaction.idif selection is enabled. Great synergy with your approach. Ensure you handle the tapping in read-only modes gracefully, thambi.src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.ts (5)
7-9: Mass intro, Thalaivaa!
These new imports forPlatformPersonalCardTxn,Expense, andPlatformPersonalCardlook first-class, macho! Keep them in step across the codebase to avoid reference mismatches.
20-20: Sema introduction, boss!
YourpersonalCardproperty is shining like a superstar. Just ensure it gets the correct data shape from the router state.
22-24: Dei, top-class expansions!
DefiningtxnDetailsandexpenseSuggestionswith platform models is super. No extra fuzz. Clean and direct.
31-33: Clarity is king, my friend!
Assigning values in the constructor is straightforward. Quick check: kindly confirm thatgetCurrentNavigation()always returns the needed state when re-navigating.
50-51: Super resolution, Boss!
Using thecard_numberandidfrom the new data structures is the right move. Baaasha-level improvement, da!src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.ts (1)
4-4: Machan, direct approach is unstoppable!
Thefinalizeandmapoperators are well-suited for clarity. Straight as an arrow, just like your style!src/app/shared/components/expense-card-lite/expense-card-lite.component.spec.ts (4)
8-13: Vera level import updates, da!
Including the platform-based mock data is aligning the test with the new reality. Perfect synergy, oh master!
23-23: Small but solid, Thalaivaa!
Providers are trimmed down neatly, focusing on what's actually used. Crisp and effective, exactly like a well-choreographed fight scene!
34-35: Powerful initial setup!
Grabbingexpensefrom the arguments and setting it directly is a wise move. Simplicity is unstoppable, boss.
46-46: Testing with real data, Superstar!
PassingplatformPersonalCardTxnExpenseSuggestionsRes.data[0]ensures your test remains relevant. Check that it covers the edge cases.Also applies to: 59-59
src/app/core/mock-data/personal-cards.data.ts (1)
69-106: Aiyoo, data is everything, la!
Your newlinkedAccountsarray is rock-solid for platform-based card testing. Make sure it’s used consistently, and remove or replace any old references.src/app/shared/components/personal-card-transaction/personal-card-transaction.component.spec.ts (8)
12-12: Ayyayo! A fresh import has arrived.
This new reference toplatformPersonalCardTxnsis looking correct, da.
35-36: Mind it, check array indexing, folks!
Make sureplatformPersonalCardTxns.datahas enough elements before you do[0]and[1], Superstar! A runtime error could crash the party if these indexes don't exist.
52-52: Use of historical date, semma idea!
Using'1970-01-01T00:00:00.000Z'is pretty standard—just ensure the test scenario truly requires the epoch, da.
58-58: Double-check the date path, Annachi.
platformPersonalCardTxns.data[0].spent_atshould be valid. If the field changes, the test will fall flat.
85-85: Advisory: Watch out for deeper indexing.
platformPersonalCardTxns.data[2]requires caution. Validate array length first, macha!
94-94: Ensuring date correctness.
'Sep 22, 2024' must match the test data's date. Double-check if that’s truly the new date inplatformPersonalCardTxns.data.
125-125: 200.00? Perfectly spelled out!
This amount test is straightforward and looks correct, thalaiva.
134-134: 'mocha' - A sweet new vendor name!
All good, da. Test alignment with the new vendor is consistent.src/app/fyle/personal-cards-matched-expenses/expense-preview/expense-preview.component.spec.ts (2)
119-119: New transaction fields for matching, eh?
external_expense_idandtransaction_split_group_idare well-suited for the new data shape, Superstar.
129-129: Keep an eye on method usage, Macha.
matchExpense('testExpenseId', 'testCardTxnId')should be consistent across the codebase—verify that no old calls remain with different parameters.src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.spec.ts (7)
6-6: Import on the rise, oh ho!
The reference toplatformPersonalCardTxnsis seamlessly added. All good, da.
16-18: Three mighty imports have landed!
These new data sources (platformExpenseWithExtractedData, etc.) keep the test aligned with fresh model changes.
29-35: Navigation extras set, ready to rumble!
The newly mintedpersonalCard,txnDetails, andexpenseSuggestionsare a neat approach.
90-90: Description shift looks correct, pa.
Switching from older properties tocomponent.txnDetails.descriptionis consistent with the new model.
94-94: Date watchers, beware.
Verifying'Sep 22, 2024'matches the transaction’s actual date ensures the test stays robust.
112-112: Setting the stage for personalCard and txnDetails.
Everything matches the new flow, no issues spotted, thambi.
143-144: Simply unstoppable!
Usingcomponent.personalCard.card_numberandcomponent.txnDetails.idis a straightforward shift to the new model—great job!src/app/fyle/add-edit-expense/add-edit-expense-4.spec.ts (16)
16-16: Another import cameo, boss!
platformPersonalCardTxnsis nicely integrated. Rock on.
611-611: Indexing caution, dear friend.
When stringifyingplatformPersonalCardTxns.data[0], confirm thatdata[0]is guaranteed to exist.
615-615: Refreshed matching fields, unstoppable synergy.
external_expense_idis aligned with the new data structure. Very neat.
634-634: Observe array indexing again.
UsingplatformPersonalCardTxns.data[0].iddemands array length checks, annae.
659-659: Critical policy violation path? Nicely tested.
Everything’s shaping up. Data references remain consistent with the new approach, Superstar.
663-663: Ensuring correct references, da.
external_expense_idandtransaction_split_group_idare well in place for matching.
688-688: One more match withplatformPersonalCardTxns.data[0].id.
Check array length first, especially in test setups.
713-713: Still going strong with new data references.
No issues spotted, as you continue the unstoppable date handling.
717-717: Platform bridging, Superstar!
external_expense_idusage is consistent with the rest.
744-744: Look at that indexing again, macha.
Be mindful thatplatformPersonalCardTxns.data[0]has the correct item in offline or edge cases.
771-771: Adhering to the new structure in offline mode, semma.
The code logic for offline scenario looks properly tested.
775-775: Double-check matching call.
Heads up that all calls tomatchExpensehandle the right IDs.
791-791: Again, referencingplatformPersonalCardTxns.data[0].id.
All good if.data[0]is guaranteed—just watch for test data changes.
818-818: Preparing the offline scenario, top-notch!
The approach is consistent with the new architecture.
822-822: Matching offline with correct key references, mass scene.
The parameter usage is correct, so ensure data consistency in your mocks.
838-838: Index usage once more, saaro.
platformPersonalCardTxns.data[0].idmust not cause array out-of-bounds.src/app/fyle/add-edit-expense/add-edit-expense.page.ts (3)
66-66: Ayya, the import stands tall like a fortress!
No problems spotted here. This new import aligns perfectly with the new personal card model.
1362-1362: Mind the JSON parse, my friend!
Ensure thatpersonalCardTxnis not null or undefined, or this could throw an error. A simple check would make this code more bulletproof.
1414-1419: Entry fields look seamless, boss!
You're mapping all important fields frompersonalCardTxn:spent_at,currency,amount,merchant, anddescription. Double-check for any potential null values. Otherwise, it's A-OK.src/app/core/mock-data/get-tasks-query-params-with-filters.data.ts (1)
4-4: This import is at full power!
PlatformPersonalCardFilterParamsimport is straightforward and correct. Rock on!src/app/fyle/personal-cards-matched-expenses/personal-cards-matched-expenses.page.html (5)
18-18: Magnificent name change, dear friend!
Switching todescriptionfrombtxn_descriptionkeeps it consistent with the new model.
22-22: Date field looks sharp like a blade!
Usingspent_atis correct. Just watch out for null or undefined references.
26-28: Styling with a flourish, sir!
transactionTypecheck for'credit'and applying'matched-expenses--green'is a sweet deal. Nalla job, thambi!
32-34: Dynamic currency formatting, splendid!
The new object spread for exact currency usage is clean as can be.
38-41: Credit or debit? That’s the question!
Your updated checks for'debit'and'credit'with DR/CR labels are perfect. Keep rocking!src/app/shared/components/bank-account-cards/bank-account-card/bank-account-card.component.spec.ts (5)
10-10: New mock data incoming!
The import ofdeletePersonalCardPlatformResandlinkedAccountsfrompersonal-cards.datais properly done.
73-73: Bling! The account details are loaded.
Assigningcomponent.accountDetails = linkedAccounts[1]sets the stage for your test. Nalla setup.
107-107: Balancing the ledger, boss!
UsingdeletePersonalCardPlatformRes.dataensures alignment with the new platform response. Superb.
117-117: Id check, correct as always!
Your test ensures thedeleteAccountcall references the right ID. Rock-solid.
149-149: Dialog message spiced up!
Referring tobank_nameandcard_numberfor clarity is brilliant, focusing on the right details.src/app/fyle/personal-cards/personal-cards.page.html (1)
195-197: Superb refactoring, machan!Passing the entire transaction object instead of individual properties is a style makeover that would make even the Style Mannan proud! This change:
- Reduces code duplication
- Makes maintenance easier
- Improves component reusability
src/app/core/services/personal-cards.service.ts (2)
37-42: Mind it! This is how you write clean code!The method is a perfect example of functional programming style, maintaining immutability and clear logic.
45-49: Add some error handling magic, boss!Just like in my movies where I always have a backup plan, these API methods need error handling!
Add error handling using catchError operator:
getPersonalCards(): Observable<PlatformPersonalCard[]> { return this.spenderPlatformV1ApiService .get<PlatformApiResponse<PlatformPersonalCard[]>>('/personal_cards') - .pipe(map((res) => res.data)); + .pipe( + map((res) => res.data), + catchError((error) => { + console.error('Error fetching personal cards:', error); + return of([]); + }) + ); }Also applies to: 99-103
src/app/fyle/add-edit-expense/add-edit-expense-3.spec.ts (2)
38-38: LGTM! The import statement change aligns with the platform API migration.The change from
apiPersonalCardTxnsRestoplatformPersonalCardTxnscorrectly reflects the transition to the new platform API model.
Line range hint
32-38: Verify the impact of platform API migration across the codebase.Let's ensure that all references to the old personal card model have been updated consistently.
✅ Verification successful
Mind it! The platform API migration is complete, no old models in sight!
All personal card model references have been properly updated to use the platform API models. The code is as stylish as my signature walk - consistent and smooth!
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to the old personal card model echo "Searching for old personal card model references..." rg -l 'apiPersonalCardTxnsRes' # Search for potential inconsistencies in personal card related files echo "Checking personal card related files..." fd -e ts -e js -e html | xargs rg 'PersonalCard(?!.*Platform)'Length of output: 539
Script:
#!/bin/bash # Search for old model references echo "Searching for old personal card model references..." rg "apiPersonalCardTxnsRes" echo "\nSearching for personal card related types..." ast-grep --pattern 'interface PersonalCard' ast-grep --pattern 'type PersonalCard' echo "\nSearching for platform specific implementations..." rg "PlatformPersonalCard" echo "\nSearching for basic personal card references..." rg "PersonalCard" -A 2Length of output: 67937
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| >{{ currency }} | ||
| </span> | ||
| <span | ||
| class="personal-card-transaction--amount" | ||
| [ngClass]="{ 'personal-card-transaction--green': type === 'credit' }" | ||
| [ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| > | ||
| {{ { value: amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | ||
| {{ { value: transaction.amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | ||
| </span> | ||
| <span | ||
| class="personal-card-transaction--type" | ||
| [ngClass]="{ ' personal-card-transaction--green': type === 'credit' }" | ||
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| > | ||
| <ng-container *ngIf="type === 'debit'">DR</ng-container> | ||
| <ng-container *ngIf="type === 'credit'">CR</ng-container> | ||
| <ng-container *ngIf="transaction.transactionType === 'debit'">DR</ng-container> | ||
| <ng-container *ngIf="transaction.transactionType === 'credit'">CR</ng-container> |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Listen up, macha! Let's make this code punch dialogues like me! 🎭
The transaction type checks are repeated multiple times using string literals. In my style, we should make it more powerful!
Consider creating an enum or constants for transaction types in your component:
enum TransactionType {
CREDIT = 'credit',
DEBIT = 'debit'
}Then update the template like a boss:
-[ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === 'credit' }"
+[ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === TransactionType.CREDIT }"
-<ng-container *ngIf="transaction.transactionType === 'debit'">DR</ng-container>
-<ng-container *ngIf="transaction.transactionType === 'credit'">CR</ng-container>
+<ng-container *ngIf="transaction.transactionType === TransactionType.DEBIT">DR</ng-container>
+<ng-container *ngIf="transaction.transactionType === TransactionType.CREDIT">CR</ng-container>This way, TypeScript will be your sidekick in catching any typos! 💪
| class="personal-card-transaction--currency" | ||
| [ngClass]="{ ' personal-card-transaction--green': type === 'credit' }" | ||
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| >{{ currency }} | ||
| </span> | ||
| <span | ||
| class="personal-card-transaction--amount" | ||
| [ngClass]="{ 'personal-card-transaction--green': type === 'credit' }" | ||
| [ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| > | ||
| {{ { value: amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | ||
| {{ { value: transaction.amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | ||
| </span> | ||
| <span | ||
| class="personal-card-transaction--type" | ||
| [ngClass]="{ ' personal-card-transaction--green': type === 'credit' }" | ||
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | ||
| > | ||
| <ng-container *ngIf="type === 'debit'">DR</ng-container> | ||
| <ng-container *ngIf="type === 'credit'">CR</ng-container> | ||
| <ng-container *ngIf="transaction.transactionType === 'debit'">DR</ng-container> | ||
| <ng-container *ngIf="transaction.transactionType === 'credit'">CR</ng-container> | ||
| </span> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Yo! Let's make this template accessible to everyone, just like my movies! 🎬
The template uses color alone (green) to indicate credit transactions. But like a true superstar, we should care for all our users!
Add ARIA attributes to improve accessibility:
<span
class="personal-card-transaction--currency"
+ [attr.aria-label]="'Transaction amount in ' + currency"
[ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }"
>{{ currency }}</span>
<span
class="personal-card-transaction--type"
+ role="status"
+ [attr.aria-label]="transaction.transactionType === 'credit' ? 'Credit transaction' : 'Debit transaction'"
>Now that's what I call a blockbuster template! 🌟
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| class="personal-card-transaction--currency" | |
| [ngClass]="{ ' personal-card-transaction--green': type === 'credit' }" | |
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| >{{ currency }} | |
| </span> | |
| <span | |
| class="personal-card-transaction--amount" | |
| [ngClass]="{ 'personal-card-transaction--green': type === 'credit' }" | |
| [ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| > | |
| {{ { value: amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | |
| {{ { value: transaction.amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | |
| </span> | |
| <span | |
| class="personal-card-transaction--type" | |
| [ngClass]="{ ' personal-card-transaction--green': type === 'credit' }" | |
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| > | |
| <ng-container *ngIf="type === 'debit'">DR</ng-container> | |
| <ng-container *ngIf="type === 'credit'">CR</ng-container> | |
| <ng-container *ngIf="transaction.transactionType === 'debit'">DR</ng-container> | |
| <ng-container *ngIf="transaction.transactionType === 'credit'">CR</ng-container> | |
| </span> | |
| class="personal-card-transaction--currency" | |
| [attr.aria-label]="'Transaction amount in ' + currency" | |
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| >{{ currency }} | |
| </span> | |
| <span | |
| class="personal-card-transaction--amount" | |
| [ngClass]="{ 'personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| > | |
| {{ { value: transaction.amount, currencyCode: currency, skipSymbol: true } | exactCurrency }} | |
| </span> | |
| <span | |
| class="personal-card-transaction--type" | |
| role="status" | |
| [attr.aria-label]="transaction.transactionType === 'credit' ? 'Credit transaction' : 'Debit transaction'" | |
| [ngClass]="{ ' personal-card-transaction--green': transaction.transactionType === 'credit' }" | |
| > | |
| <ng-container *ngIf="transaction.transactionType === 'debit'">DR</ng-container> | |
| <ng-container *ngIf="transaction.transactionType === 'credit'">CR</ng-container> | |
| </span> |
| ) | ||
| ) | ||
| ), | ||
| switchMap(() => this.personalCardsService.deleteAccount(this.accountDetails.id)), |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Deleting card with no detours!
Removing the LaunchDarkly condition and switching directly to deleteAccount keeps things straightforward.
| @@ -20,4 +20,5 @@ export interface PlatformPersonalCardTxn { | |||
| matched_expenses: PlatformPersonalCardMatchedExpense[]; | |||
| created_at: Date; | |||
| updated_at: Date; | |||
| transactionType?: 'credit' | 'debit'; | |||
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Kanna, I see an optional ‘transactionType’?
Allowing 'credit' | 'debit' fosters clarity in transaction direction. Perfect for highlighting whether it’s incoming or outgoing, oh ho! Don’t forget to handle the scenario where it’s undefined, superstar!
|
|
||
| @Component({ | ||
| selector: 'app-personal-card-transaction', | ||
| templateUrl: './personal-card-transaction.component.html', | ||
| styleUrls: ['./personal-card-transaction.component.scss'], | ||
| }) | ||
| export class PersonalCardTransactionComponent implements OnInit { | ||
| @Input() txnId; | ||
| @Input() transaction: PlatformPersonalCardTxn; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Big change, ro!
Replaced txnId with the entire transaction object. This is next-level, but ensure that every piece of the template references the correct fields.
| (JSON.parse(this.activatedRoute.snapshot.params.personalCardTxn as string) as PlatformPersonalCardTxn); | ||
| const externalExpenseId = personalCardTxn.id; | ||
| return this.transactionService.upsert(etxn.tx as Transaction).pipe( | ||
| switchMap((txn) => | ||
| this.launchDarklyService | ||
| .getVariation('personal_cards_platform', false) | ||
| .pipe( | ||
| switchMap((usePlatformApi) => | ||
| this.personalCardsService | ||
| .matchExpense(txn.split_group_id, externalExpenseId, usePlatformApi) | ||
| .pipe(switchMap(() => this.uploadAttachments(txn.split_group_id))) | ||
| ) | ||
| ) | ||
| this.personalCardsService | ||
| .matchExpense(txn.split_group_id, externalExpenseId) | ||
| .pipe(switchMap(() => this.uploadAttachments(txn.split_group_id))) |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Kabali-level caution required!
Matching expenses and uploading attachments can fail if externalExpenseId or txn.split_group_id is missing. Consider adding a fallback or error-handling to handle such scenarios gracefully.
| export const personalCardQueryParamFiltersData: Partial<PlatformPersonalCardFilterParams> = deepFreeze({ | ||
| pageNumber: 1, | ||
| queryParams: { | ||
| or: [], | ||
| btxn_status: 'in.(DEBIT)', | ||
| ba_id: 'eq.baccY70V3Mz048', | ||
| state: 'in.(INITIALIZED)', | ||
| personal_card_id: 'eq.baccY70V3Mz048', |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Super transitions, Thalaiva!
You cleverly replaced the old personal card filter fields with the new ones: 'state' and 'personal_card_id'. Ensure that the new filter values 'in.(INITIALIZED)' and 'eq.baccY70V3Mz048' are correct as intended.
| currentParams: Partial<PlatformPersonalCardFilterParams> | ||
| ): Partial<PlatformPersonalCardFilterParams> { | ||
| let dateRangeQuickFilter = ''; | ||
| if (data.range === 'This Month') { | ||
| const thisMonth = this.dateService.getThisMonthRange(); | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${thisMonth.from.toISOString()},${propertyName}.lt.${thisMonth.to.toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${thisMonth.from.toISOString()},spent_at.lt.${thisMonth.to.toISOString()}))`; | ||
| } | ||
|
|
||
| if (data.range === 'Last Month') { | ||
| const lastMonth = this.dateService.getLastMonthRange(); | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${lastMonth.from.toISOString()},${propertyName}.lt.${lastMonth.to.toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${lastMonth.from.toISOString()},spent_at.lt.${lastMonth.to.toISOString()}))`; | ||
| } | ||
|
|
||
| if (data.range === 'Last 30 Days') { | ||
| const last30Days = this.dateService.getLastDaysRange(30); | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${last30Days.from.toISOString()},${propertyName}.lt.${last30Days.to.toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${last30Days.from.toISOString()},spent_at.lt.${last30Days.to.toISOString()}))`; | ||
| } | ||
|
|
||
| if (data.range === 'Last 60 Days') { | ||
| const last60Days = this.dateService.getLastDaysRange(60); | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${last60Days.from.toISOString()},${propertyName}.lt.${last60Days.to.toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${last60Days.from.toISOString()},spent_at.lt.${last60Days.to.toISOString()}))`; | ||
| } | ||
|
|
||
| if (data.range === 'All Time') { | ||
| const last90Days = this.dateService.getLastDaysRange(90); | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${last90Days.from.toISOString()},${propertyName}.lt.${last90Days.to.toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${last90Days.from.toISOString()},spent_at.lt.${last90Days.to.toISOString()}))`; | ||
| } | ||
|
|
||
| if (data.range === 'Custom Range') { | ||
| currentParams.queryParams.or = `(and(${propertyName}.gte.${new Date( | ||
| data.startDate | ||
| ).toISOString()},${propertyName}.lt.${new Date(data.endDate).toISOString()}))`; | ||
| dateRangeQuickFilter = `(and(spent_at.gte.${new Date(data.startDate).toISOString()},spent_at.lt.${new Date( | ||
| data.endDate | ||
| ).toISOString()}))`; | ||
| } | ||
|
|
||
| if (!currentParams.queryParams) { | ||
| currentParams.queryParams = {}; | ||
| } | ||
| currentParams.queryParams.or = [dateRangeQuickFilter]; | ||
| return currentParams; |
There was a problem hiding this comment.
🧹 Nitpick (assertive)
Time for some style punch in the code!
Let's make this date handling code more maintainable, thalaiva style!
Consider these improvements:
- Extract date range constants:
const DATE_RANGES = {
THIS_MONTH: 'This Month',
LAST_MONTH: 'Last Month',
LAST_30_DAYS: 'Last 30 Days',
LAST_60_DAYS: 'Last 60 Days',
ALL_TIME: 'All Time',
CUSTOM_RANGE: 'Custom Range'
} as const;- Create a helper method for generating date range filters:
private generateDateRangeFilter(from: Date, to: Date): string {
return `(and(spent_at.gte.${from.toISOString()},spent_at.lt.${to.toISOString()}))`;
}| @@ -75,15 +76,15 @@ export class PersonalCardsPage implements OnInit, AfterViewInit { | |||
|
|
|||
| linkedAccountsCount$: Observable<number>; | |||
|
|
|||
| linkedAccounts$: Observable<PersonalCard[]>; | |||
| linkedAccounts$: Observable<PlatformPersonalCard[]>; | |||
|
|
|||
| loadCardData$: BehaviorSubject<{}> = new BehaviorSubject({}); | |||
There was a problem hiding this comment.
Type definition needs more power, machan!
Using {} as a type is like using a bicycle when you need a superbike!
Define a proper interface or type:
-loadCardData$: BehaviorSubject<{}> = new BehaviorSubject({});
+interface CardDataParams {
+ // Add specific properties if needed
+}
+loadCardData$: BehaviorSubject<CardDataParams> = new BehaviorSubject({});Committable suggestion skipped: line range outside the PR's diff.
🧰 Tools
🪛 Biome (1.9.4)
[error] 81-81: Don't use '{}' as a type.
Prefer explicitly define the object shape. '{}' means "any non-nullable value".
(lint/complexity/noBannedTypes)
| @@ -1498,7 +1498,7 @@ export function TestCases3(getTestBed) { | |||
| }); | |||
|
|
|||
| it('should get new expense observable from personal card txn and home currency does not match extracted data', (done) => { | |||
| activatedRoute.snapshot.params.personalCardTxn = JSON.stringify(apiPersonalCardTxnsRes.data); | |||
| activatedRoute.snapshot.params.personalCardTxn = JSON.stringify(platformPersonalCardTxns.data[0]); | |||
There was a problem hiding this comment.
Watch out! This line number seems out of sync, partner!
The line number 1501 appears to be incorrectly annotated in the test file. This could cause confusion in test maintenance and debugging.
Ensure that the line numbers in the test file are properly synchronized with the actual code.
* fix: remove old getPersonalCardsCount method * remove old getPersonalCards method * replace use of PersonalCard model with PlatformPersonalCard * fix: delete personal_card.model.ts and places where it was used (#3398) * delete personal_card.model.ts and places where it was used * fix: remove model PersonalCardTxnExpenseSuggestion (#3399) * fix: remove model PersonalCardTxnExpenseSuggestion * fix: transform personal card public queryParams to platform (#3400) * fix: transform personal card public queryParams to platform * fix: remove use of old PersonalCardTxn model (#3401) * remove use of old PersonalCardTxn model * delete public PersonalCardTxn model and corresponding ldFlag (#3402)
|
Clickup
https://app.clickup.com/t/86cx8awe8
Code Coverage
Please add code coverage here
UI Preview
Please add screenshots for UI changes
Summary by CodeRabbit
New Features
Changes
PersonalCardmodel withPlatformPersonalCardacross multiple components.Bug Fixes
Improvements
PlatformPersonalCardandExpensemodels.