create pay and downgrade rhp#59070
Conversation
|
@jayeshmangwani 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] |
|
Thanks for the PR @dukenv0307 and If you can add the Tests to the PR description, that would be helpful. Also, I just want to confirm—do we need to wait for testing until @blimpich comes online? Their ngrok setup might not be working right now, but I’m not entirely sure. |
|
@jayeshmangwani yeah the ngrok setup doesn't work unless I'm online sadly. Let me know if that doesn't work with your schedule and we can find another C+ 👍 |
|
@jayeshmangwani I updated the steps. Can you please test it? Thanks |
@blimpich I’m fine with the schedule and will be online during this timeline. I’m starting the review now. |
| discounts: BillingItem[]; | ||
|
|
||
| /** A list of sales tax items */ | ||
| salesTax: BillingItem[]; |
There was a problem hiding this comment.
The salesTax value from billingDetails is currently an object, also pressing 'Delete' from the overview causes the app to crash.
{
"description": "Sales Tax ( - 0%)",
"amount": 104,
"formattedAmount": "$1.04",
"type": "",
"percentage": 0
}
|
@dukenv0307 Pressing 'Delete' causes the app to crash crash-on-delete.mov |
|
@jayeshmangwani That's weird. Based on the data mentioned here, the salesTax should be an array, and when I use staging server, it's also an array cc @blimpich |
|
Then , there might be a backend issue. I’m currently testing the pay-and-downgrade page directly. @blimpich can check this API response later Just a note—I’m testing this with the ngrok .env, in case it helps with debugging the issue. |
src/hooks/usePayAndDowngrade.ts
Outdated
| const [shouldBillWhenDowngrading] = useOnyx(ONYXKEYS.SHOULD_BILL_WHEN_DOWNGRADING); | ||
| const isDeletingPaidWorkspaceRef = useRef(false); | ||
|
|
||
| const setIsDelingPaidWorkspace = (value: boolean) => { |
|
Strange, I'm pretty sure I copied that example of billingReceiptData straight from the dev tools so it should've been an object, but for some reason it wasn't. It should be an object, not an array. Here is another example of |
|
@jayeshmangwani Can you please check again? BTW, I'm asking for the translation here: https://expensify.slack.com/archives/C01GTK53T8Q/p1743100833103959 |
|
Thanks, Checking in 30 minutes. |
|
@dukenv0307, can you please confirm on your side if the changes work well on Safari and mWeb Safari? I’m not sure why, but the safari.-shouldCalculateBillNewDot.mov |
|
Not a major issue, but upon refreshing, Not Found page briefly appears. We could add a loader while the data loads. on-refresh-bug.mov |
|
@blimpich @dukenv0307, what do you think about adding a visual loader or some kind of representation to indicate that we’re waiting for the no-visual-apear.mov |
|
Yeah we should have some form of loading indicator, good point! I talked with design about this, we want the menu item popover to stay open after you click "delete" and for the trash can icon to change to a loading spinner while we wait for |
|
And echoing that point, for the Delete button from the Workspace Overview, let's use our loading spinner pattern on the button after you press delete. |
|
Thankyou! Let's go ahead with these ideas #59070 (comment) And #59070 (comment) |
|
Big agree! Those will be great improvements here. |
|
Agree with Shawn about the spinner 👍 |
|
@jayeshmangwani I updated the PR |
|
@dukenv0307 have you tested this PR on safari and mobile? |
|
@jayeshmangwani It works well on my side. Did you use the same account as Chrome? Screen.Recording.2025-03-31.at.00.02.50.mov |
|
@jayeshmangwani I resolved all your comments above Screen.Recording.2025-04-14.at.21.36.11.mov |
|
Thank you for making the changes! @blimpich All yours. |
|
Will review sometime today, nice work 👍 |
blimpich
left a comment
There was a problem hiding this comment.
Looks very good overall. Couple minor comments. Once those are resolved I think we're good to merge 👍
|
@blimpich Thanks for your suggestions. I reoslved all of them |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
Note to QA: this probably can't be QA'ed since it requires having an account with at least 3 purchases. I don't think this is possible to test in QA. |
|
🚀 Deployed to staging by https://github.com/blimpich in version: 9.1.29-0 🚀
|
|
@dukenv0307 @blimpich Our QA team does not have an account with this condition:
Could this be verified internally? |
|
On staging, not really. This is unfortunately very difficult to test. So lets just skip QA for now. We'll deal with issues with this as they come up 👍 |
|
🚀 Deployed to production by https://github.com/marcaaron in version: 9.1.29-10 🚀
|
| } | ||
|
|
||
| if (!shouldBillWhenDowngrading) { | ||
| setIsDeleteModalOpen(true); |
Explanation of Change
Fixed Issues
$ #58603
PROPOSAL:
Tests
Pay & downgradepage is shownPay & downgradebuttonOffline tests
QA Steps
Same as above
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)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
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop