-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
[HOLD for payment 2024-08-07] [$250] [MEDIUM] ReportPreview is showing the incorrect title on a travel booking #45344
Comments
Triggered auto assignment to @slafortune ( |
Job added to Upwork: https://www.upwork.com/jobs/~01a1f88ea93705aa2a |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @eVoloshchak ( |
Proposal: Fix Incorrect Title on ReportPreview for Travel Bookings Problem Statement: The ReportPreview feature is displaying an incorrect title for travel bookings, which can lead to confusion and errors for users. The root cause of this problem is an incorrect implementation of the title logic in the ReportPreview component. Root Cause: The current implementation of the title logic in the ReportPreview component is not correctly handling travel booking types, resulting in an incorrect title being displayed. Proposed Solution: To resolve this issue, we recommend the following optimized solution: Simplify the title logic: Refactor the title logic to use a more concise and efficient approach, reducing the complexity of the code. import React from 'eact'; const ReportPreview = ({ travelBooking }) => { return ( {title}); }; export default ReportPreview; We explored the following alternative solutions: Using a template engine: Utilize a template engine such as Handlebars to generate the title for travel bookings. However, this approach would require additional dependencies and complexity. Please let us know if you would like to discuss this proposal further or proceed with implementation |
ProposalPlease re-state the problem that we are trying to solve in this issue.The report name shows the approver and submitter's names, like an IOU. What is the root cause of that problem?In
person associated with the report, there's no logic to customize for trips.
What changes do you think we should make in order to solve the problem?In
tripData?.tripID of the report), we'll put the tripName to ReportActionItemFragment in order to display it. We can add a custom case TRIP for the trip header fragment like in this
The back-end needs to provide the What alternative solutions did you explore? (Optional)Optionally we can show the trip icon as the left hand icon of the report action item, in place of the participants' icons as it is now. |
How can I book a travel? |
@rushatgabhane has been helping implement travel, so I am going to assign him and @shubham1206agra who has been reviewing his travel issues! At some point soon, travel will be something anybody can help with but for now as we're racing for a deadline, I'm going to prioritise them. Appreciate the excitement for it though folks! |
📣 @shubham1206agra 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app! |
@twisterdotcom we can assign @tienifr for their proposal #45344 (comment) 🎀👀🎀 |
Triggered auto assignment to @grgia, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@rushatgabhane PR is here #46266. |
good to merge! @stitesExpensify |
on staging |
|
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.14-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 2024-08-07. 🎊 For reference, here are some details about the assignees on this issue:
|
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@rushatgabhane @shubham1206agra can you please complete the checklist? are you both due payment for the C+role? |
@slafortune I am doing the checklist. |
BugZero Checklist: The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed:
|
@slafortune i don't know know about that. i had reviewed the proposal and the PR so I'm definitely due for payment 😄 |
Yes, we both reviewed the PR. And I kind of monitored this issue through. |
@shubham1206agra should there be any regression tests? |
@slafortune No, since this was a simple edge case for design, which got missed. |
Upworks NewDot |
$250 approved for @rushatgabhane |
If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!
In the trip room, the ReportPreview is showing, but it shows as if the person who should pay the report is the approver, and not the workspace (which is what we show in other places)
Version Number:
Reproducible in staging?:
Reproducible in production?:
If this was caught during regression testing, add the test name, ID and link from TestRail:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by:
Slack conversation:
Action Performed:
Expected Result:
The trip report name should show the Trip Name in the reportPreview.
Actual Result:
The report name shows the approver and submitter's names, like an IOU.
Workaround:
Can the user still use Expensify without this being fixed? Have you informed them of the workaround?
Screenshots/Videos
Slack threads
Example 1
Example 2
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @slafortuneThe text was updated successfully, but these errors were encountered: