-
Notifications
You must be signed in to change notification settings - Fork 3k
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
[PAYMENT MAY 29][$250] Split - Expense text is missing from the preview tile when creating split expense #41010
Comments
Triggered auto assignment to @slafortune ( |
ProposalPlease re-state the problem that we are trying to solve in this issue.Expense text is missing from the preview tile when creating split expense. It's visible if you use the quick action button to do the same split What is the root cause of that problem?The For quick action, we skip the confirmation screen and pass
while in confirmation screen (split without quick action), For the first case, Line 3140 in 2b0b12c
That's why splitting through QAB has What changes do you think we should make in order to solve the problem?There are 2 options:
I think in general we fallback to Line 3140 in 2b0b12c
const isMerchantEmpty = !merchant || merchant === CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT
...
merchant: isMerchantEmpty ? Localize.translateLocal('iou.expense') : merchant
That means we fallback to merchant: merchant || CONST.TRANSACTION.PARTIAL_TRANSACTION_MERCHANT What alternative solutions did you explore? (Optional)NA |
@slafortune Eep! 4 days overdue now. Issues have feelings too... |
Job added to Upwork: https://www.upwork.com/jobs/~01cb074043a26909ea |
Triggered auto assignment to Contributor-plus team member for initial proposal review - @s77rt ( |
@gijoe0295 Thanks for the proposal. Your RCA makes sense. Let's align on the expected behavior and discuss the solution. 🎀 👀 🎀 The expected behavior is not clear, when creating a split should we:
|
Triggered auto assignment to @thienlnam, see https://stackoverflow.com/c/expensify/questions/7972 for more details. |
@thienlnam Could you check for assignment please? |
@slafortune, @s77rt, @thienlnam Eep! 4 days overdue now. Issues have feelings too... |
@thienlnam Can you please check #41010 (comment) |
@slafortune @s77rt @thienlnam 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! |
@thienlnam when you get a chance can you review the assignment? |
@slafortune, @s77rt, @thienlnam Uh oh! This issue is overdue by 2 days. Don't forget to update your issues! |
Ah yes, thanks for the bump - was OOO last week |
We would want to keep the merchant empty |
@gijoe0295 Let's just change the merchant value to
|
This was merged yesterday - @s77rt - C+ Role ~ will be due $250 via Upworks I'll be on vacation until June 4th so adding a BZ in the meantime. |
Triggered auto assignment to @garrettmknight ( |
Payment Summary: |
@s77rt can you complete the checklist when you get a chance? |
Regression Test Proposal
|
@garrettmknight Done! ^ |
Legend, thank you! |
If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!
Version Number: 1.4.66.0
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team
Issue found when executing PR #40961
Action Performed:
Expected Result:
Expense text should be visible
Actual Result:
Expense text is missing from the preview tile when creating split expense. It's visible if you use the quick action button to do the same split
Workaround:
Unknown
Platforms:
Which of our officially supported platforms is this issue occurring on?
Screenshots/Videos
Add any screenshot/video evidence
View all open jobs on GitHub
Upwork Automation - Do Not Edit
Issue Owner
Current Issue Owner: @garrettmknightThe text was updated successfully, but these errors were encountered: