[Due for payment 2025-02-18] [$250] Per diem - Time summary on the confirmation page is incorrect #55449
Comments
|
Triggered auto assignment to @RachCHopkins ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.When a user creates a per diem expense for the same day (like 8:00 AM to 10:00 AM), the app shows wrong time calculations. Instead of showing 2 hours, it shows "First day: 16.00 hours", "Trip: 1 full day", and "Last day: 10.00 hours". What is the root cause of that problem?The time calculation code doesn't check if the start and end dates are on the same day. It always tries to split the time into first day, trip days, and last day, even when everything happens on one day. This causes wrong calculations for same-day expenses. What changes do you think we should make in order to solve the problem?Add a check at the start of the calculation to see if start and end dates are on the same day App/src/libs/PerDiemRequestUtils.ts Line 257 in 64ed6a5 modify it as: function getTimeDifferenceIntervals(transaction: OnyxEntry<Transaction>) {
const customUnitRateDate = transaction?.comment?.customUnit?.attributes?.dates ?? {start: '', end: ''};
const startDate = new Date(customUnitRateDate.start);
const endDate = new Date(customUnitRateDate.end);
+ // Check if start and end dates are on the same day
+ const isSameDay = startOfDay(startDate).getTime() === startOfDay(endDate).getTime();
+
+ if (isSameDay) {
+ // If same day, just calculate the direct hour difference
+ const hourDiff = (differenceInMinutes(endDate, startDate) / 60).toFixed(2);
+ return {
+ firstDay: hourDiff,
+ tripDays: 0,
+ lastDay: undefined,
+ };
+ }
// Original logic for multi-day calculations
const firstDayDiff = differenceInMinutes(startOfDay(addDays(startDate, 1)), startDate);
const tripDaysDiff = differenceInDays(startOfDay(endDate), startOfDay(addDays(startDate, 1)));
const lastDayDiff = differenceInMinutes(endDate, startOfDay(endDate));
return {
firstDay: firstDayDiff === 1440 ? undefined : (firstDayDiff / 60).toFixed(2),
tripDays: firstDayDiff === 1440 ? tripDaysDiff + 1 : tripDaysDiff,
lastDay: lastDayDiff === 0 ? undefined : (lastDayDiff / 60).toFixed(2),
};
}What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?NA What alternative solutions did you explore? (Optional)NA Screen.Recording.2025-01-18.at.8.18.54.PM.mov |
ProposalPlease re-state the problem that we are trying to solve in this issue.After turning on the Make or track payments,the BA flow opens on the page where it was left What is the root cause of that problem?The function assumes startDate and endDate always span multiple days. App/src/libs/PerDiemRequestUtils.ts Lines 257 to 269 in 850c339 Example: Given startDate = '2025-01-16T08:00:00' and endDate = '2025-01-16T18:00:00' const firstDayDiff = differenceInMinutes(startOfDay(addDays(startDate, 1)), startDate); // Incorrectly calculates next day
const tripDaysDiff = differenceInDays(startOfDay(endDate), startOfDay(addDays(startDate, 1))); // Returns negative or invalid
const lastDayDiff = differenceInMinutes(endDate, startOfDay(endDate)); // Processes last day even for same-day tripsWhat changes do you think we should make in order to solve the problem?To fix this issue, we need to check if startDate and endDate are on the same day at the start of the function. If they are, directly calculate the total time difference as firstDay and set tripDays and lastDay to undefined. import {..., isSameDay, ...} from 'date-fns';function getTimeDifferenceIntervals(transaction: OnyxEntry<Transaction>) {
const customUnitRateDate = transaction?.comment?.customUnit?.attributes?.dates ?? {start: '', end: ''};
const startDate = new Date(customUnitRateDate.start);
const endDate = new Date(customUnitRateDate.end);
// Check if startDate and endDate are on the same day
if (isSameDay(startDate, endDate)) { // add this line
const totalMinutes = differenceInMinutes(endDate, startDate); // add this line
return { // add this line
firstDay: totalMinutes / 60, // add this line
tripDays: undefined, // add this line
lastDay: undefined, // add this line
}; // add this line
} // add this line
// If not on the same day, calculate intervals
const firstDayDiff = differenceInMinutes(startOfDay(addDays(startDate, 1)), startDate);
const tripDaysDiff = differenceInDays(startOfDay(endDate), startOfDay(addDays(startDate, 1)));
const lastDayDiff = differenceInMinutes(endDate, startOfDay(endDate));
return {
firstDay: firstDayDiff === 1440 ? undefined : firstDayDiff / 60,
tripDays: firstDayDiff === 1440 ? tripDaysDiff + 1 : tripDaysDiff,
lastDay: lastDayDiff === 0 ? undefined : lastDayDiff / 60,
};
}POCWhat specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?Create unit test for case same day. What alternative solutions did you explore? (Optional)Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job. |
|
I don't appear to have Per Diems. I will have to assume they exist because we have two people above who can repro this. Update: Per Diem is behind a beta. |
|
Job added to Upwork: https://www.upwork.com/jobs/~021881825734850372345 |
|
Triggered auto assignment to Contributor-plus team member for initial proposal review - @c3024 ( |
|
@c3024, when you get a chance, could you please review the proposals? Thank you |
|
Thanks for the bump. Checking! |
|
@Shahidullah-Muffakir 's RCA and suggested fix in his proposal here looks good to me! 🎀 👀 🎀 C+ reviewed! |
|
Triggered auto assignment to @lakchote, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
|
@lakchote, @RachCHopkins, @c3024 Whoops! This issue is 2 days overdue. Let's get this updated quick! |
|
@lakchote a little bump on this:
Thanks. |
|
@lakchote, @RachCHopkins, @c3024 Eep! 4 days overdue now. Issues have feelings too... |
|
@lakchote @RachCHopkins @c3024 this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks! |
|
@lakchote, @RachCHopkins, @c3024 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it! |
|
@Shahidullah-Muffakir's proposal LGTM. |
|
📣 @Shahidullah-Muffakir You have been assigned to this job! |
|
|
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.95-6 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue: If no regressions arise, payment will be issued on 2025-02-18. 🎊 For reference, here are some details about the assignees on this issue:
|
|
@c3024 @RachCHopkins @c3024 The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@Shahidullah-Muffakir do you have the ability to apply to the upwork job? (I'm just trying to work out if I need to create a manual offer or not) |
I've just applied. |
|
Sorry @Shahidullah-Muffakir I have no idea where that went, so I just sent you an offer instead! Let me know when it's accepted! |
|
Thanks @RachCHopkins, I’ve accepted the offer. |
|
Cool, we will be good to go next week when @c3024 has done the checklist! |
|
@lakchote @RachCHopkins @c3024 @Shahidullah-Muffakir this issue is now 4 weeks old, please consider:
Thanks! |
BugZero Checklist:
Bug classificationSource of bug:
Where bug was reported:
Who reported the bug:
Regression Test Proposal Template
Regression Test ProposalPrecondition:
Test:
Do we agree 👍 or 👎 |
|
Payment Summary:
Upwork job here |
|
Contributor has been paid, the contract has been completed, and the Upwork post has been closed. |
|
$250 approved for @c3024 |
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.87-0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
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+28930@applause.expensifail.com
Issue reported by: Applause - Internal Team
Action Performed:
Precondition:
Expected Result:
The time summary should be correct on the confirmation page
It should show "First Day: 2.00 hours" because the configured duration is only 2 hours (the same on OD)
Actual Result:
The time summary is wrong on the confirmation page
It shows "First day: 16.00 hours", "Trip: 1 full day", "Last day: 10.00 hours" when the configured duration is only 2 hours
On OD, it shows the correct summary which is "First Day: 2.00 hours"
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
Bug6717376_1737187618758.bandicam_2025-01-18_15-35-42-986.mp4
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @RachCHopkinsThe text was updated successfully, but these errors were encountered: