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

[$2000] Android -Scan- Parent message not automatically updated to [Deleted] when deleted #27058

Closed
6 tasks done
kbecciv opened this issue Sep 8, 2023 · 51 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering Internal Requires API changes or must be handled by Expensify staff

Comments

@kbecciv
Copy link

kbecciv commented Sep 8, 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. Go to + > Request money > Scan > Take a pic of the receipt
  2. Once uploaded, add an amount. (very important)
  3. Verify you receive amount changed message on the bottom of the page
  4. Click on the 3 dots and attempt to delete it
  5. Verify that the parent message does not automatically update to [Deleted]

Expected Result:

When the parent message is deleted, it should automatically update to [Deleted]

Actual Result:

Parent message is not automatically updated to [Deleted]

Workaround:

Unknown

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.66.3
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
Notes/Photos/Videos: Any additional supporting documentation

Upload-unable.to.Delete.mp4
Screen_Recording_20230908_161343_New.Expensify.mp4

Expensify/Expensify Issue URL:
Issue reported by: @avi-shek-jha
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1693868591788509

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01a605ed983b7c2031
  • Upwork Job ID: 1700244152320290816
  • Last Price Increase: 2023-10-02
@kbecciv kbecciv added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Sep 8, 2023
@melvin-bot melvin-bot bot changed the title Android -Scan- System generated messages not deleted [$500] Android -Scan- System generated messages not deleted Sep 8, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

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

melvin-bot bot commented Sep 8, 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

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

Triggered auto assignment to @bfitzexpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

@melvin-bot
Copy link

melvin-bot bot commented Sep 8, 2023

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

@bfitzexpensify
Copy link
Contributor

Agreed this should be external. Not sure why melv assigned both you and I, @CortneyOfstad, but I'm happy to take this one

@parasharrajat
Copy link
Member

The Expected behavior is not how our thread reports work. @bfitzexpensify I feel this is something that we should clarify first before we look for proposals.

@bfitzexpensify
Copy link
Contributor

@parasharrajat do you have a little more context on that? Is the expected behaviour that we don't remove historical messages upon deletion? If so, I think we would classify this as not being a bug and close the issue.

@melvin-bot melvin-bot bot removed the Overdue label Sep 14, 2023
@melvin-bot
Copy link

melvin-bot bot commented Sep 15, 2023

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

@parasharrajat
Copy link
Member

My thought was that even if the parent message of the thread is deleted, it does not delete child messages in the thread.

IMO, the same thing applies in this case. This is how it will be shown in the parent IOU report chat.
Screenshot 2023-09-15 at 9 39 28 PM


But I noticed something the chat title does not change to [Deleted message] immediately. It is changed only after the message is posted in the same thread.


Thus I asked you to clarify the expected behaviour. Because the mentioned expected behavior is wrong.

@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2023
@parasharrajat
Copy link
Member

@bfitzexpensify Thoughts.

@melvin-bot melvin-bot bot removed the Overdue label Sep 18, 2023
@bfitzexpensify
Copy link
Contributor

Thanks for the clarification. You're right in that the expected behaviour doesn't match how other deleted parent messages work. It does still seem like there might be something to fix here (ensuring the parent message is updated to [Deleted]).

So, how does this sound:

We update the action performed to this:

  1. Go to + > Request money > Scan > Take a pic of the receipt
  2. Once uploaded, add an amount. (very important)
  3. Verify you receive amount changed message on the bottom of the page
  4. Click on the 3 dots and attempt to delete it
  5. Verify that the parent message does not automatically update to [Deleted]

With an expected behaviour of:

When the parent message is deleted, it should automatically update to [Deleted]

@parasharrajat
Copy link
Member

parasharrajat commented Sep 18, 2023

Yeah, That sounds good. It will be better to change the title and actual result as well.

@bfitzexpensify bfitzexpensify changed the title [$500] Android -Scan- System generated messages not deleted [$500] Android -Scan- Parent message not automatically updated to [Deleted] when deleted Sep 18, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

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

  • Decide whether any proposals currently meet our guidelines and can be approved as-is today
  • If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
  • If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

@melvin-bot melvin-bot bot added Internal Requires API changes or must be handled by Expensify staff and removed 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 labels Oct 6, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 6, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Oct 9, 2023

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

@bfitzexpensify
Copy link
Contributor

@parasharrajat have you had a chance to review this?

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 9, 2023
@bfitzexpensify
Copy link
Contributor

friendly bump @parasharrajat

@melvin-bot melvin-bot bot removed the Overdue label Oct 12, 2023
@parasharrajat
Copy link
Member

Missed it. I will get it tomorrow.

@parasharrajat
Copy link
Member

Okay, both proposals are very similar. Now the first question would be why these internal messages are not counted in visibleChilds. Maybe because these are only visible to the owner. Let me find more info around this.

@shubham1206agra
Copy link
Contributor

@parasharrajat Maybe this

But when we change anything for the money request, such as description, amount, etc., it does not increase the amount of childVisibleActionCount due to the fact that someone created a money request by mistake and wants to delete it. After deletion, it should not show the request in the favour that the user just changed the parameter of the request, not anything useful is inside it.

@shubham1206agra
Copy link
Contributor

@parasharrajat I think this has been taken care of already in #28214

@melvin-bot melvin-bot bot added the Overdue label Oct 15, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 16, 2023

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

@parasharrajat
Copy link
Member

Testing.

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Oct 16, 2023
@melvin-bot
Copy link

melvin-bot bot commented Oct 20, 2023

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

@bfitzexpensify
Copy link
Contributor

How did your testing go @parasharrajat?

@melvin-bot melvin-bot bot removed the Overdue label Oct 20, 2023
@parasharrajat
Copy link
Member

Yup. This is gone. @bfitzexpensify

@s-alves10
Copy link
Contributor

@parasharrajat

It looks like the title in LHN and the header of the transaction thread were fixed already
Here is my question:
After deleting the iou request, LHN title and the header are changed to [Deleted request], but iou report shows nothing though there exists the transaction thread in the LHN.
image

I thought we should show [Deleted request] in the iou report as well like
image

Let me know what the expected behavior is
Thank you

@parasharrajat
Copy link
Member

That is a different issue and being fixed ATM.

@bfitzexpensify
Copy link
Contributor

Yup. This is gone. @bfitzexpensify

Great, thanks @parasharrajat. Closing this out.

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 Engineering Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

6 participants