Add getCardAssignmentDateOption util method#82707
Conversation
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
@MelvinBot fix the TS issues |
…signmentDateOption The dateOption field in AssignCardData is typed as string, so the function parameter needs to accept string instead of the narrow ValueOf type. Use explicit equality checks to return the properly typed result. Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
Fixed the typecheck failure. The issue was that Fix: Changed the parameter type to |
Code Reviewsrc/libs/CardUtils.ts — Incomplete refactoringLine 863 ( src/libs/CardUtils.ts — Verbose JSDocLines 844-854: The JSDoc is overly verbose for a small utility function. The function name and parameter names already communicate the intent clearly. Consider trimming to just the summary line, or removing the JSDoc entirely — the code is self-explanatory. |
|
@MelvinBot can you implement the improvements here: #82707 (comment) |
- Add getCardAssignmentStartDate to eliminate duplicated startDate ternary in AssigneeStep.tsx - Trim verbose JSDoc on getCardAssignmentDateOption to a single summary line - Remove unused date-fns format import from AssigneeStep.tsx Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com>
|
Implemented both improvements from the review:
|
PR #82707 Review – Expensify/AppSummaryThe PR moves card-assignment date logic into shared helpers in What’s good
Suggestions1. Use
|
|
No product review needed |
|
@carlosmiceli conflict and if you want to do te suggestion from #82707 (comment) |
|
@MelvinBot can you merge main and fix conflicts please? |
…d-util Co-authored-by: Carlos Miceli <carlosmiceli@users.noreply.github.com> # Conflicts: # src/libs/CardUtils.ts # src/pages/workspace/companyCards/assignCard/AssigneeStep.tsx # src/pages/workspace/companyCards/assignCard/InviteNewMemberStep.tsx
|
Merged main and resolved conflicts in 3 files:
|
|
@nkuoch ok, I think this is ready for another review :) |
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppAndroid: mWeb ChromeiOS: HybridAppiOS: mWeb SafariMacOS: Chrome / Safari |
|
🚧 @nkuoch has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging by https://github.com/nkuoch in version: 9.3.26-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.3.26-8 🚀
|
Explanation of Change
Fixed Issues
$
PROPOSAL:
Tests
Offline tests
QA Steps
// TODO: These must be filled out, or the issue title must include "[No QA]."
Navigate to Workspace → Company Cards → Assign Card
Select an employee as cardholder
Select a card to assign
Set transaction start date and complete assignment
Verify card assigned successfully with correct date
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectioncanBeMissingparam foruseOnyxtoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari