-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Show 'Report physical card loss/damage' option for physical cards, regardless of activation status #55767
Conversation
…gardless of activation status
@paultsimura 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] |
Reviewer Checklist
Screenshots/VideosAndroid: NativeAndroid.webmAndroid: mWeb Chromechrome.webmiOS: NativeSimulator.Screen.Recording.-.iPhone.15.Pro.-.2025-01-26.at.16.05.13.mp4iOS: mWeb SafariSimulator.Screen.Recording.-.iPhone.15.Pro.-.2025-01-26.at.16.05.36.mp4MacOS: Chrome / SafariScreen.Recording.2025-01-26.at.15.59.35.movMacOS: DesktopScreen.Recording.2025-01-26.at.16.08.12.mov |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That is a nice, clean solution, thanks 👍
Also, good job differentiating Tests and QA Steps in this case.
Please make the QA steps more straightforward for QA flow, like this (because I doubt there is a clear way to reproduce all different card statuses during QA):
- Join a workspace with the Expensify Card enabled
- As a workspace member, get assigned a physical Expensify Card
- Open the Wallet page
- In the Assigned Cards section, click the newly assigned card
- Verify the "Report physical card loss/damage" option is shown while the card is still in state:
NOT_ACTIVATED
Thanks! Updated the QA steps as suggested. |
@paultsimura Just to confirm the expected behaviour, with the new changes, the Report physical card loss/damage option is shown if the card state is STATE_SUSPENDED. Is it the same in OD? Is this correct? |
@twisterdotcom maybe you could also help as you reported the issue. |
I see in the OldDot front-end code that we have the following states:
The user can click the Request New Card button on a physical card if that card has one of the So I would imagine that we want to show the report option in NewDot when the card is in cc @twisterdotcom and maybe @joekaufmanexpensify for confirmation 🙏 |
@cristipaval that is exactly correct. I believe we only show it now for the It should not show for |
Nice. @Shahidullah-Muffakir could you please update accordingly? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing approval for now.
Thank you for confirming, I'll add the changes. |
…state is either open or not_activated
@paultsimura I've added the changes, thank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ✔️
✋ 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/cristipaval in version: 9.0.93-0 🚀
|
Why were so many of the deploys canceled? |
🚀 Deployed to production by https://github.com/roryabraham in version: 9.0.93-3 🚀
|
Do we care that this failed to deploy to some platforms? |
AFAIK, this PR was reverted: https://expensify.slack.com/archives/C01GTK53T8Q/p1738321390817309 |
Explanation of Change
Previously, the option Report physical card loss/damage was only shown if the card's status was "open." With this change, the option will now be displayed for any physical card that is shown, regardless of its activation status. The only time the option won't appear is if the card's state is
STATE_NOT_ISSUED.
Fixed Issues
$ #55189
PROPOSAL: #55189 (comment)
Tests
Run this in the console(One by one for each status)
STATE_NOT_ISSUED
status.Offline tests
STATE_NOT_ISSUED
status.QA Steps
PR Author Checklist
### Fixed Issues
section aboveTests
sectionOffline steps
sectionQA steps
sectiontoggleReport
and notonIconClick
)myBool && <MyComponent />
.src/languages/*
files and using the translation methodSTYLE.md
) were followedAvatar
, I verified the components usingAvatar
are working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG)
)Avatar
is modified, I verified thatAvatar
is working as expected in all cases)Design
label and/or tagged@Expensify/design
so the design team can review the changes.ScrollView
component to make it scrollable when more elements are added to the page.main
branch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTest
steps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
ios.20native-4.mp4
iOS: mWeb Safari
MacOS: Chrome / Safari
maco.20safari.mp4
MacOS: Desktop
desktoop.mp4