Conversation
|
Hey @getusha, can you merge main? |
|
Let's merge main and address the linting @getusha 😄 |
0a499a9 to
4edefdb
Compare
|
@hungvu193 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] |
src/types/onyx/LastPaymentMethod.ts
Outdated
| Iou: string; | ||
| iou: string; | ||
| /** The lastPaymentMethod of an Expense */ | ||
| Expense: string; | ||
| expense: string; | ||
| /** The lastPaymentMethod of an Invoice */ | ||
| Invoice: string; | ||
| invoice: string; |
There was a problem hiding this comment.
Oops, I thought I confirmed these property names with @grgia, may I know why you changed these property names? Is that something that's returned from our BE?
There was a problem hiding this comment.
It's to fix type error:
Not sure why we'd want them to be capitalized, aren't they keys?
|
Also can you provide the test steps and complete the checklist? Ty |
@hungvu193 thought i'll do that once the BE changes are deployed, so i can test it all together. wdyt? |
|
Got it. I posted on slack to move this forward |
|
Looks like we still have linting, can you fix it? @getusha |
|
Friendly reminder that we also have this PR which also changes the |
|
Still have linting, can you just merge your PR with mine? and we start from there? |
826b0ba to
2e0f923
Compare
|
@getusha What's being held on BE btw? |
|
the new endpoint to move iou to existing policy
…On Fri, 14 Mar 2025 at 6:09 PM Hans ***@***.***> wrote:
@getusha <https://github.com/getusha> What's being held on BE btw?
—
Reply to this email directly, view it on GitHub
<#56838 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR4OEV6CUSWCVBRU2J2DBRD2ULWI5AVCNFSM6AAAAABXCPXWL6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRUHE4DMNBZGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
[image: hungvu193]*hungvu193* left a comment (Expensify/App#56838)
<#56838 (comment)>
@getusha <https://github.com/getusha> What's being held on BE btw?
—
Reply to this email directly, view it on GitHub
<#56838 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AR4OEV6CUSWCVBRU2J2DBRD2ULWI5AVCNFSM6AAAAABXCPXWL6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDOMRUHE4DMNBZGI>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
|
Can you please also update on slack? |
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); | ||
| const policy = policies?.[policyID]; |
There was a problem hiding this comment.
OR somethings like this:
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];
hungvu193
left a comment
There was a problem hiding this comment.
I've just found another issue while working on invoice feature.
| const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${policyID}`); | ||
| const policy = policies?.[policyID]; |
There was a problem hiding this comment.
OR somethings like this:
const policy = policies?.[`${ONYXKEYS.COLLECTION.POLICY}${policyID}`];78ea10d to
44f1336
Compare
|
|
| // const isPolicyExpenseChat = !!reports.find((rp) => rp.policyID === transactionItem.policyID && rp.isPolicyExpenseChat); | ||
|
|
||
| // console.log({reports}) | ||
| // console.log({isPolicyExpenseChat}); |
There was a problem hiding this comment.
Why is all this code commented out? I see some more below here. We should not merge this kinda changes into main
There was a problem hiding this comment.
| const formattedAmount = convertToDisplayString(Math.abs(amount), currency) ?? ''; | ||
|
|
||
| // This will be fixed as part of https://github.com/Expensify/Expensify/issues/507850 | ||
| // eslint-disable-next-line deprecation/deprecation |
There was a problem hiding this comment.
Please do not add more usages of getPolicy(). It has been deprecated for a reason and adding more usages is just increasing the work that other people need to do.
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.1.72-0 🚀
|
1 similar comment
|
🚀 Deployed to staging by https://github.com/grgia in version: 9.1.72-0 🚀
|
|
🚀 Deployed to production by https://github.com/puneetlath in version: 9.1.72-10 🚀
|

Explanation of Change
Standardize pay button for IOUs and Expenses
Fixed Issues
$ #61744
$ #55874
#55875
PROPOSAL:
Tests
Paying IOU for the first time - preview
Pay %amount%buttonPay with personal account, Pay with business account, Mark as paidPay with personal accountPay %amount%buttonPay with business accountis presentPay with business accountPay via %workspace name%optionPaid IOU before - preview
Paying expense report for the first time - preview
Steps
Pay with business accountandMark as paid.Paying expense report for the first time - held expense
Steps
Paid expense report before - preview
Steps
Offline tests
N/a
QA Steps
Same as tests
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
Paying IOU for the first time - preview
Paid IOU before - preview
Paying expense report for the first time - preview
Paying expense report for the first time - held expense
Screen.Recording.2025-04-22.at.6.11.01.in.the.evening.mov
Paid expense report before - preview
MacOS: Desktop