-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
[$250] [P2P Distance] Split - Participants amount displayed 0.00 briefly on confirmation screen #47100
Comments
👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:
|
Triggered auto assignment to @jasperhuangg ( |
We think that this bug might be related to #vip-split |
Not a blocker, it's behind the distance split beta. |
Job added to Upwork: https://www.upwork.com/jobs/~0170f1a1791408f62b |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat ( |
Triggered auto assignment to @garrettmknight ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Split distance - Participants amount displayed 0.00 briefly on confirmation screen What is the root cause of that problem?
What changes do you think we should make in order to solve the problem?
if (isDistanceRequest) {
let reportUpdated = report;
if (isPolicyExpenseChat && transaction?.participants?.[0]) {
reportUpdated = ReportUtils.getReport(transaction?.participants?.[0].reportID ?? '') ?? report;
}
const policyID = IOU.getIOURequestPolicyID(transaction, reportUpdated);
const policy = PolicyUtils.getPolicy(report?.policyID ?? policyID);
const policyCurrency = policy?.outputCurrency ?? PolicyUtils.getPersonalPolicy()?.outputCurrency ?? CONST.CURRENCY.USD;
const customUnitRateID = TransactionUtils.getRateID(transaction) ?? '-1';
const mileageRates = DistanceRequestUtils.getMileageRates(policy);
const defaultMileageRate = DistanceRequestUtils.getDefaultMileageRate(policy);
const mileageRate = TransactionUtils.isCustomUnitRateIDForP2P(transaction)
? DistanceRequestUtils.getRateForP2P(policyCurrency)
: mileageRates?.[customUnitRateID] ?? defaultMileageRate;
const {unit, rate} = mileageRate ?? {};
const distance = TransactionUtils.getDistanceInMeters(transaction, unit);
const currency = (mileageRate as MileageRate)?.currency ?? policyCurrency;
const amount = DistanceRequestUtils.getDistanceRequestAmount(distance, unit ?? CONST.CUSTOM_UNITS.DISTANCE_UNIT_MILES, rate ?? 0);
IOU.setMoneyRequestAmount(transactionID, amount, currency ?? '');
const participantsMap =
transaction?.participants?.map((participant) => {
if (participant.isSender && iouType === CONST.IOU.TYPE.INVOICE) {
return participant;
}
return participant.accountID ? OptionsListUtils.getParticipantsOption(participant, personalDetails) : OptionsListUtils.getReportOption(participant);
}) ?? [];
const participantAccountIDs: number[] = participantsMap.map((participant) => participant.accountID ?? -1);
if (isTypeSplit && !isPolicyExpenseChat && amount && transaction?.currency) {
IOU.setSplitShares(transaction, amount, currency, participantAccountIDs);
}
} Note The code above is not perfect, it is just pseudocode and can be improved. It might also have some flaws, but these can be addressed after thorough testing. What alternative solutions did you explore? (Optional) |
Edited by proposal-police: This proposal was edited at 2024-08-09 01:17:59 UTC. ProposalPlease re-state the problem that we are trying to solve in this issue.
What is the root cause of that problem?
the correct amount is displayed afterward. What changes do you think we should make in order to solve the problem?
will be: rightElement: transaction?.splitShares?.[participantOption.accountID ?? -1]?.amount !== undefined ? (
<MoneyRequestAmountInput />
) : (
<ActivityIndicator color={!true ? theme.textLight : theme.text} />
) |
@parasharrajat can you give these proposals a look when you get a chance? |
@garrettmknight, @parasharrajat, @jasperhuangg Whoops! This issue is 2 days overdue. Let's get this updated quick! |
📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app! Offer link |
@Krishna2323's approach seems to be the simplest approach to fixing the issue, assigning them! FYI I will be going OOO for the rest of the week, so we might need to bring in another internal engineer to help with PR reviews once the PR is done. cc @garrettmknight |
Sure, let's go with that approach. |
Draft PR is up, just need to test all cases and add recordings, will be completed by EOD. |
@parasharrajat PR ready for review. |
This issue has not been updated in over 15 days. @garrettmknight, @parasharrajat, @jasperhuangg, @Krishna2323 eroding to Monthly issue. P.S. Is everyone reading this sure this is really a near-term priority? Be brave: if you disagree, go ahead and close it out. If someone disagrees, they'll reopen it, and if they don't: one less thing to do! |
This comment was marked as outdated.
This comment was marked as outdated.
@jasperhuangg, The PR is still open, we were about to complete the testing but in this PR we removed the split expense option from global create button. I'm not completely sure what to do now but we are discussing in the PR. |
@Krishna2323 Got it, thanks for the update |
Since we've removed the global create, I've tried to repro using split in the group chat. I can't really tell if it happens in iOS, there's some glitchy behavior, but I can't see if it's starting with 0 because it's too fast. |
It's a lot clearer on Android. @Krishna2323 @parasharrajat is the fix you're working on still applicable to the behavior even if it doesn't come from Global Create? |
The approach is the same, but I had to reapply it to a different component (distance page) to make it work with the current split distance creation flow. @parasharrajat will review the updated code soon. |
@garrettmknight, @parasharrajat, @jasperhuangg, @Krishna2323 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
@garrettmknight, @parasharrajat, @jasperhuangg, @Krishna2323 6 days overdue. This is scarier than being forced to listen to Vogon poetry! |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
Version Number: 9.0.18.3
Reproducible in staging?: Y
Reproducible in production?: N
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): applausetester+0808pm@applause.expensifail.com
Issue reported by: Applause - Internal Team
Issue found when executing PR #42302
Action Performed:
Expected Result:
The correct participants amounts are displayed
Actual Result:
0.00 is briefly displayed instead of the correct amount
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6565894_1723139401565.video_2024-08-08_13-48-49.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @parasharrajatThe text was updated successfully, but these errors were encountered: