[NO QA]: Company card details#48491
Conversation
|
@allroundexperts Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
# Conflicts: # src/components/Icon/Illustrations.ts
|
Let us know when you got some screenshots and we'll do a design review 😄 |
|
@dubielzyk-expensify here we go: Seems like we do not have correct icon for Unassign user - could you please provide it. Thanks! |
|
Yup, agree with Danny here! |
|
What Danny said and otherwise this is looking good to me 👍 |
Reviewer Checklist
Screenshots/VideosAndroid: NativeScreen.Recording.2024-09-09.at.11.08.44.PM.movAndroid: mWeb ChromeScreen.Recording.2024-09-09.at.11.07.00.PM.moviOS: NativeScreen.Recording.2024-09-09.at.11.03.02.PM.moviOS: mWeb SafariScreen.Recording.2024-09-09.at.11.04.49.PM.movMacOS: Chrome / SafariScreen.Recording.2024-09-09.at.10.53.55.PM.movMacOS: DesktopScreen.Recording.2024-09-09.at.11.00.49.PM.mov |
|
@narefyev91 Can you please finish the checklist and make sure that the lint errors are resolved? |
# Conflicts: # src/ROUTES.ts # src/libs/Navigation/linkingConfig/FULL_SCREEN_TO_RHP_MAPPING.ts # src/libs/actions/Policy/Policy.ts
|
@narefyev91 Can I please have testing steps? |
|
@narefyev91 On the following screen size, the buttons are stacked too close to each other. The settings button also seems to be clipped.
|
Yeah - just pushing latest changes for offline support - which was approved yesterday |
# Conflicts: # src/libs/API/parameters/index.ts # src/libs/API/types.ts # src/pages/workspace/upgrade/WorkspaceUpgradePage.tsx # src/types/form/index.ts
|
@allroundexperts Hey! Can you please review again? Thanks! |
|
Sure. Will review again today! |
|
Friendly bump on this one @yuwenmemon, Thanks! |
|
|
|
✋ 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/yuwenmemon in version: 9.0.35-0 🚀
|
|
🚀 Deployed to production by https://github.com/grgia in version: 9.0.35-7 🚀
|
|
Just saw a log error making the call to SetCardExportAccount fail:
How are we sending |
| @@ -0,0 +1,6 @@ | |||
| type UpdateCompanyCard = { | |||
| authToken?: string | null; | |||
| cardID: string; | |||
There was a problem hiding this comment.
cardID was supposed to be of type number, this caused #51895
| ); | ||
|
|
||
| return ( | ||
| <SelectionScreen |
There was a problem hiding this comment.
We didn't not include a search option like we do in other places in the app when the number of options is more than 8, this caused #51884
| /> | ||
| <MenuItemWithTopDescription | ||
| description={translate('workspace.moreFeatures.companyCards.transactionStartDate')} | ||
| title={card?.startDate ? format(card.startDate, CONST.DATE.FNS_FORMAT_STRING) : ''} |
| /> | ||
| <MenuItemWithTopDescription | ||
| description={translate('workspace.moreFeatures.companyCards.cardNumber')} | ||
| title={CardUtils.maskCard(card?.lastFourPAN)} |
There was a problem hiding this comment.
It caused this issue, we need to mask the card name to display 6 first numbers and 4 last numbers
| API.write(WRITE_COMMANDS.DELETE_COMPANY_CARD_FEED, parameters, onyxData); | ||
| } | ||
|
|
||
| function unassignWorkspaceCompanyCard(workspaceAccountID: number, cardID: string, bankName: string) { |
There was a problem hiding this comment.
We missed to update the optimistic data to apply online pattern B on this action
| const connectedIntegration = getConnectedIntegration(policy, accountingIntegrations) ?? connectionSyncProgress?.connectionName; | ||
|
|
||
| const [personalDetails] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST); | ||
| const [allBankCards] = useOnyx(`${ONYXKEYS.COLLECTION.WORKSPACE_CARDS_LIST}${workspaceAccountID}_${bank}`); |
There was a problem hiding this comment.
Coming from this issue #58005, we were using the bank value directly from the route params, which may cause an error(wrong data) when the page is refreshed. When the page is refreshed, the value becomes encoded. To fix this, we need todecodeURIComponent bank before using it, more context: #58005 (comment)
| > | ||
| <HeaderWithBackButton | ||
| title={translate('workspace.moreFeatures.companyCards.cardName')} | ||
| onBackButtonPress={() => Navigation.goBack(ROUTES.WORKSPACE_COMPANY_CARD_DETAILS.getRoute(policyID, cardID, bank))} |
There was a problem hiding this comment.
Looks like not passing {compareParams: false} here to navigation back led to this issue:
|
I am not able to comment on the changes for some reason so posting here. This logic does not compare params while going back which results in new route being set when we open the card details from member profile. It caused #62197 |














Details
Company Card details page
Fixed Issues
$ #47377, #48425, #47379
PROPOSAL:
Preconditions:
We need to add mock information for card in Onyx - i added them inside src/pages/workspace/companyCards/WorkspaceCompanyCardsPage.tsx - just uncomment useEffect
We adding as well some mock data to test export cards account - i used quickbooks online for mock data
Tests
Offline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)myBool && <MyComponent />.src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))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
android1.mov
android2.mov
Android: mWeb Chrome
android-web.mov
iOS: Native
Screen.Recording.2024-09-10.at.16.04.33.mov
iOS: mWeb Safari
ios-web.mov
MacOS: Chrome / Safari
web1.mov
web2.mov
MacOS: Desktop
desktop.mov