Expensify / App

Welcome to New Expensify: a complete re-imagination of financial collaboration, centered around chat. Help us build the next generation of Expensify by sharing feedback and contributing to the code.
https://new.expensify.com
MIT License
3.56k stars 2.9k forks source link

[$500] IOU-In workspace transaction report page, split bill option is not displayed. #34423

Closed kavimuru closed 9 months ago

kavimuru commented 10 months ago

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: Reproducible in staging?: y Reproducible in production?: y 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: Applause - Internal Team Slack conversation:

Action Performed:

  1. Launch app
  2. Tap on a Workspace chat
  3. Tap plus icon near compose
  4. Note split bill option is displayed
  5. Tap on any expense report or create a new open and open it
  6. In transaction report page, tap plus icon near compose
  7. Note split bill option is missing

Expected Result:

In workspace transaction report page, split bill option must be shown.

Actual Result:

In workspace transaction report page, split bill option is not displayed.

Workaround:

unknown

Platforms:

Which of our officially supported platforms is this issue occurring on?

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/43996225/4933a068-3a7b-47d9-b84b-8d298efbd744

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a82e43fbd8ba4922
  • Upwork Job ID: 1745747139352756224
  • Last Price Increase: 2024-02-02
melvin-bot[bot] commented 10 months ago

Job added to Upwork: https://www.upwork.com/jobs/~01a82e43fbd8ba4922

melvin-bot[bot] commented 10 months ago

Triggered auto assignment to @johncschuster (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.

melvin-bot[bot] commented 10 months ago

Triggered auto assignment to Contributor-plus team member for initial proposal review - @mananjadhav (External)

FitseTLT commented 10 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

In workspace transaction report page, split bill option is not displayed.

What is the root cause of that problem?

We are only allowing split menu for group chat and workspace chat /room here https://github.com/Expensify/App/blob/55746c78a6ef59d9ad636eda8c610023c3cd62ac/src/libs/ReportUtils.ts#L3815-L3817

What changes do you think we should make in order to solve the problem?

We should allow workspace transaction report pages too

    if (
        (isChatRoom(report) && otherParticipants.length > 0) ||
        ((isDM(report) || isMoneyRequestReport(report)) && hasMultipleOtherParticipants) ||
        (isPolicyExpenseChat(report) && report?.isOwnPolicyExpenseChat)
    ) {

What alternative solutions did you explore? (Optional)

dukenv0307 commented 10 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

In workspace transaction report page, split bill option is not displayed.

What is the root cause of that problem?

We only display split option for policy expense chat or chat has multiple participant here.

https://github.com/Expensify/App/blob/b6a18b185f485cf17474c18add56359f4624b56a/src/libs/ReportUtils.ts#L3815

What changes do you think we should make in order to solve the problem?

We should add another case to check if the report is an expense report and the parent report has isOwnPolicyExpenseChat as true we will return the split bill option for this

if ((isChatRoom(report) && otherParticipants.length > 0) || (isDM(report) && hasMultipleOtherParticipants) || (isPolicyExpenseChat(report) && report?.isOwnPolicyExpenseChat) || (isExpenseReport(report) && parentReport?.isOwnPolicyExpenseChat)) {

What alternative solutions did you explore? (Optional)

NA

melvin-bot[bot] commented 10 months ago

@mananjadhav, @johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mananjadhav commented 10 months ago

I'll check and reproduce the issues. I can see code for both the proposals but not the explanation.

allgandalf commented 10 months ago

Proposal

Please re-state the problem that we are trying to solve in this issue.

The issue is that the split bill option is not displayed in the workspace transaction report page.

What is the root cause of that problem?

The split menu is currently only allowed for group chat and workspace chat/room and not for workspace transaction report pages in the code located in App/src/libs/ReportUtils.ts

https://github.com/Expensify/App/blob/b6a18b185f485cf17474c18add56359f4624b56a/src/libs/ReportUtils.ts#L3815

What changes do you think we should make in order to solve the problem?

The proposed changes aim to include the split menu for the workspace transaction report pages as well. The modified code looks like this:

if ((isChatRoom(report) && otherParticipants.length > 0) || 
    (isDM(report) && hasMultipleOtherParticipants) || 
    (isPolicyExpenseChat(report) && report?.isOwnPolicyExpenseChat) || 
    (isExpenseReport(report) && parentReport?.isOwnPolicyExpenseChat)) {

This part of the condition checks whether the current report is an expense report. If it is, the condition proceeds to the next part.

The second part of the condition verifies if the parent report of the expense report has the property isOwnPolicyExpenseChat set to true. This property indicates that the expense report is associated with a policy expense chat.

What alternative solutions did you explore? (Optional)

N/A

johncschuster commented 10 months ago

Still reviewing proposals

dukenv0307 commented 10 months ago

In the second thought I think it's not a bug because split bill option show only show for the main chat.

melvin-bot[bot] commented 10 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

mananjadhav commented 10 months ago

I have been trying to figure out if the Split bill is supposed to show. I can see this comment in the code.

// User created policy rooms and default rooms like #admins or #announce will always have the Split Bill option
// unless there are no other participants at all (e.g. #admins room for a policy with only 1 admin)
// DM chats will have the Split Bill option only when there are at least 2 other people in the chat.
// Your own workspace chats will have the split bill option.

Based @dukenv0307's previous comment, @johncschuster can you please confirm if this is a valid bug?

melvin-bot[bot] commented 9 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

mananjadhav commented 9 months ago

Quick bump @johncschuster.

melvin-bot[bot] commented 9 months ago

@mananjadhav, @johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!

johncschuster commented 9 months ago

Sorry I let this slip, @mananjadhav! I'm reviewing your comment above now!

melvin-bot[bot] commented 9 months ago

@mananjadhav @johncschuster this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

melvin-bot[bot] commented 9 months ago

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

melvin-bot[bot] commented 9 months ago

@mananjadhav, @johncschuster Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

mananjadhav commented 9 months ago

quick bump @johncschuster ^

melvin-bot[bot] commented 9 months ago

@mananjadhav @johncschuster this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

Thanks!

melvin-bot[bot] commented 9 months ago

Current assignee @mananjadhav is eligible for the Internal assigner, not assigning anyone new.

melvin-bot[bot] commented 9 months ago

@mananjadhav, @johncschuster Eep! 4 days overdue now. Issues have feelings too...

melvin-bot[bot] commented 9 months ago

@mananjadhav, @johncschuster Still overdue 6 days?! Let's take care of this!

johncschuster commented 9 months ago

Sorry for the delay here. I've had trouble finding the focus time to really dig into this. Yes, when I look a the comment from the code (from your comment above, @mananjadhav) and review the reproduction steps, I believe this behavior is expected.

johncschuster commented 9 months ago

Unless this behavior was produced in an #admins room for a policy with only 1 admin (as mentioned above), I think we can close this one out.