Skip to content
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 App#18604] [$1000] Deleting a request money message shows a empty last message #18861

Closed
1 of 6 tasks
kavimuru opened this issue May 12, 2023 · 23 comments
Closed
1 of 6 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors

Comments

@kavimuru
Copy link

kavimuru commented May 12, 2023

If you haven’t already, check out our contributing guidelines for onboarding and email contributors@expensify.com to request to join our Slack channel!


Action Performed:

  1. Open chat > Request money
  2. After requesting delete it

Expected Result:

Deleting a request money message should not display a empty last message.

Actual Result:

Deleting a request money message shows a empty last message.

Workaround:

Can the user still use Expensify without this being fixed? Have you informed them of the workaround?

Platforms:

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

  • Android / native
  • Android / Chrome
  • iOS / native
  • iOS / Safari
  • MacOS / Chrome / Safari
  • MacOS / Desktop

Version Number: 1.3.13.1
Reproducible in staging?: y
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
Notes/Photos/Videos: Any additional supporting documentation

2023-05-12.20-50-13.mp4
Recording.584.mp4

Expensify/Expensify Issue URL:
Issue reported by: @ayazhussain79
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1683906942740509

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b71f19dcccb67b54
  • Upwork Job ID: 1657138400751517696
  • Last Price Increase: 2023-05-12
@kavimuru kavimuru added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

Bug0 Triage Checklist (Main S/O)

  • This "bug" occurs on a supported platform (ensure Platforms in OP are ✅)
  • This bug is not a duplicate report (check E/App issues and #expensify-bugs)
    • If it is, comment with a link to the original report, close the issue and add any novel details to the original issue instead
  • This bug is reproducible using the reproduction steps in the OP. S/O
    • If the reproduction steps are clear and you're unable to reproduce the bug, check with the reporter and QA first, then close the issue.
    • If the reproduction steps aren't clear and you determine the correct steps, please update the OP.
  • This issue is filled out as thoroughly and clearly as possible
    • Pay special attention to the title, results, platforms where the bug occurs, and if the bug happens on staging/production.
  • I have reviewed and subscribed to the linked Slack conversation to ensure Slack/Github stay in sync

@Julesssss Julesssss self-assigned this May 12, 2023
@kavimuru
Copy link
Author

@trjExpensify @JmillsExpensify @luacmartins @mountiny @Julesssss @cristipaval tagging as per this comment in the slack

@joekaufmanexpensify
Copy link
Contributor

Reproduced on staging.

2023-05-12_15-43-16.mp4

@joekaufmanexpensify
Copy link
Contributor

@Julesssss LMK if your plan is to keep this internally, and I'll add that label to the issue.

@trjExpensify
Copy link
Contributor

This can go external 👍

@trjExpensify trjExpensify added the External Added to denote the issue can be worked on by a contributor label May 12, 2023
@melvin-bot melvin-bot bot changed the title Deleting a request money message shows a empty last message [$1000] Deleting a request money message shows a empty last message May 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

Current assignee @joekaufmanexpensify is eligible for the External assigner, not assigning anyone new.

@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

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

@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label May 12, 2023
@melvin-bot
Copy link

melvin-bot bot commented May 12, 2023

Current assignee @Julesssss is eligible for the External assigner, not assigning anyone new.

@s-alves10
Copy link
Contributor

Proposal

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

Empty row is added when delete a request money message.

What is the root cause of that problem?

Now we don't show Reqeust bill actions with total = 0 as you can see below. We only show the latest action as shown in the description video(settled up).

if (props.iouReport.total === 0) {
return null;
}

This works fine but the main reason for this issue is here.

if (!this.props.displayAsGroup) {
return (
<ReportActionItemSingle
action={this.props.action}
showHeader={!this.props.draftMessage}
wrapperStyles={[styles.chatItem, isWhisper ? styles.pt1 : {}]}
shouldShowSubscriptAvatar={this.props.shouldShowSubscriptAvatar}
report={this.props.report}
>
{content}
</ReportActionItemSingle>
);
}

The content is empty, but we have its style styles.chatItem. Its value is:

App/src/styles/styles.js

Lines 1415 to 1422 in 293f23b

chatItem: {
display: 'flex',
flexDirection: 'row',
paddingTop: 8,
paddingBottom: 8,
paddingLeft: 20,
paddingRight: 20,
},

The padding style makes the empty row visible.

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

  1. Add isIOUActionNotShown function to ReportActionsUtils.js. This function would return true if the given action should be hidden.
  2. In ReportActionItem.js, if the action should be hidden, the render function have to return null.
  if (ReportActionUtils.isIOUActionNotShown(this.props.report.reportID, this.props.reportActions, this.props.action)) {
     return null;
  }
  1. These fixes would affect the group displaying. We need to fix the displayAsGroup props of ReportActionItem.
    displayAsGroup={ReportActionsUtils.isConsecutiveActionMadeByPreviousActor(sortedReportActions, index)}

What alternative solutions did you explore? (Optional)

@mountiny
Copy link
Contributor

I am not sure if this is best solution @s-alves10 because we will move the requests to the iou/expense reports with this PR

I will put this on hold for that PR so we can see if its reproducible there

@mountiny mountiny changed the title [$1000] Deleting a request money message shows a empty last message [HOLD App#18604] [$1000] Deleting a request money message shows a empty last message May 13, 2023
@melvin-bot melvin-bot bot added the Overdue label May 15, 2023
@Julesssss
Copy link
Contributor

Agree with holding.

@melvin-bot melvin-bot bot removed the Overdue label May 15, 2023
@luacmartins
Copy link
Contributor

I think this was solved here

@joekaufmanexpensify
Copy link
Contributor

@luacmartins Awesome, does that mean we can close this?

@luacmartins
Copy link
Contributor

luacmartins commented May 16, 2023

@joekaufmanexpensify can we check if this is still reproducible before closing that PR should be in staging now?

@trjExpensify
Copy link
Contributor

How can you delete @luacmartins when we don't have the transaction view sorted yet, it's just empty?

@luacmartins
Copy link
Contributor

You can hover the IOUPreview in the IOU/Expense report and the context menu should show the delete option.

@trjExpensify
Copy link
Contributor

Nada on staging:

image

@trjExpensify
Copy link
Contributor

Okay, gave it a refresh and I can see the trash can now. Tested in a workspaceChat and a DM, still something wonky going on:

s6pcaW1SIj.mp4

@joekaufmanexpensify
Copy link
Contributor

I agree it's still a bit wonky, but the bug reported in this issue is fixed, no? There is no longer an empty last message.

The settled up 0.00 alongside showing the cash request for 30.00 as completed feels a bit odd, but it feels like that's a different bug, if it is one.

@trjExpensify
Copy link
Contributor

Correct, there is no longer an empty last message.

@luacmartins
Copy link
Contributor

Yea, we can close this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 External Added to denote the issue can be worked on by a contributor Help Wanted Apply this label when an issue is open to proposals by contributors
Projects
None yet
Development

No branches or pull requests

8 participants