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 for payment 2023-09-04] Cannot delete or edit their own request #25664

Closed
6 tasks done
mountiny opened this issue Aug 22, 2023 · 34 comments
Closed
6 tasks done

[HOLD for payment 2023-09-04] Cannot delete or edit their own request #25664

mountiny opened this issue Aug 22, 2023 · 34 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff

Comments

@mountiny
Copy link
Contributor

mountiny commented Aug 22, 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:

Break down in numbered steps

  1. User A requests X from User B
  2. User B request the same X from User A
  3. User B cannot delete their request from the header
  4. When you navigate tot he IOU report
  5. Delete the money request from the popover menu

Expected Result:

Describe what you think should've happened

You should be able to delete requests you have made.

When deleting from the popover menu, the request is deleted

Actual Result:

Describe what actually happened

Unable to delete the request

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:
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
Expensify/Expensify Issue URL:
Issue reported by: @tranvantoan-qn
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1691836416831359

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0140d2e85164cfdf25
  • Upwork Job ID: 1694687743326535680
  • Last Price Increase: 2023-08-24
@mountiny mountiny added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Aug 22, 2023
@mountiny mountiny self-assigned this Aug 22, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 2023

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

@melvin-bot
Copy link

melvin-bot bot commented Aug 22, 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

@mountiny
Copy link
Contributor Author

cc @luacmartins if you are interested

@tranvantoan-qn
Copy link

I have attached a screen recording here to make it easier for you to follow.

Delete-Request-Money-Error.mp4

@spcheema
Copy link
Contributor

I got this response while deleting @mountiny

image

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Aug 22, 2023
@mountiny
Copy link
Contributor Author

The issue is interesting, for IOU reports the verification fails to confirm the user is owner of the transactions here becuase the cardID is linked to the user who is payer and thats because we link all the transactions on the IOU report to the payer (person who owes money) here.

This is really interesting issue @luacmartins

@luacmartins
Copy link
Contributor

hmm we might have to add some new logic to check for the cardID and signs of each transaction as well 🤦

@mountiny
Copy link
Contributor Author

I am really curious why we doing this and if we have to be doing this.

@mountiny mountiny removed the Reviewing Has a PR in review label Aug 24, 2023
@mountiny mountiny changed the title Cannot delete their own request Cannot delete or edit their own request Aug 24, 2023
@mountiny mountiny added the Internal Requires API changes or must be handled by Expensify staff label Aug 24, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

Job added to Upwork: https://www.upwork.com/jobs/~0140d2e85164cfdf25

@melvin-bot
Copy link

melvin-bot bot commented Aug 24, 2023

Triggered auto assignment to Contributor Plus for review of internal employee PR - @cubuspl42 (Internal)

@mountiny
Copy link
Contributor Author

@tranvantoan-qn Can you please confirm this works now?

@tranvantoan-qn
Copy link

tranvantoan-qn commented Aug 26, 2023

@mountiny
It worked, there is NO error when deleting the request and the Delete option is showing correctly for each sender

But I'm wondering how the navigation flow should work to avoid the Hm... It's not here page since the request/report is deleted - see video from 02:16

Issue-verification.mp4

@luacmartins
Copy link
Contributor

@tranvantoan-qn we're investigating that as part of #25698

@mountiny
Copy link
Contributor Author

Perfect so I think this is ready to pay out

@mountiny mountiny added Daily KSv2 Weekly KSv2 and removed Weekly KSv2 Daily KSv2 labels Aug 28, 2023
@melvin-bot
Copy link

melvin-bot bot commented Aug 28, 2023

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:

  • [@mananjadhav] The PR that introduced the bug has been identified. Link to the PR:
  • [@mananjadhav] The offending PR has been commented on, pointing out the bug it caused and why, so the author and reviewers can learn from the mistake. Link to comment:
  • [@mananjadhav] A discussion in #expensify-bugs has been started about whether any other steps should be taken (e.g. updating the PR review checklist) in order to catch this type of bug sooner. Link to discussion:
  • [@mananjadhav] Determine if we should create a regression test for this bug.
  • [@mananjadhav] If we decide to create a regression test for the bug, please propose the regression test steps to ensure the same bug will not reach production again.
  • [@anmurali] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

@mananjadhav
Copy link
Collaborator

@mountiny Is the payout date 2023-08-30 or 2023-09-04?

@luacmartins
Copy link
Contributor

Correct date should be 2023-09-04. This PR was deployed to production 3 days ago

@luacmartins luacmartins changed the title [HOLD for payment 2023-09-04] [HOLD for payment 2023-08-30] Cannot delete or edit their own request [HOLD for payment 2023-09-04] Cannot delete or edit their own request Aug 31, 2023
@mananjadhav
Copy link
Collaborator

mananjadhav commented Sep 4, 2023

Okay thanks. @mountiny Is it safe to consider this a feature request? Also do we need a regression test here? I don't think we do.

@mountiny
Copy link
Contributor Author

mountiny commented Sep 4, 2023

@mananjadhav This was a bug as there was backend issue with the permissions.

I think we need a regression test which will cover all the possible permission cases

@melvin-bot melvin-bot bot added Daily KSv2 and removed Weekly KSv2 Daily KSv2 labels Sep 4, 2023
@mananjadhav
Copy link
Collaborator

@mountiny Can we use the test steps from the PR?

@mountiny
Copy link
Contributor Author

mountiny commented Sep 8, 2023

you can probably use those from the issue here yeah

@mananjadhav
Copy link
Collaborator

Okay thanks. Can @anmurali help here to add the regression tests? or can you take care of that @mountiny? I don't have the access to create those.

@mountiny
Copy link
Contributor Author

mountiny commented Sep 8, 2023

Can you just write it down in the format that can be copy pasted and @anmurali will take care of that

@JmillsExpensify
Copy link

$1,000 payment approved via NewDot based on BZ summary.

@melvin-bot melvin-bot bot added the Overdue label Sep 10, 2023
@tranvantoan-qn
Copy link

tranvantoan-qn commented Sep 10, 2023

@JmillsExpensify Can you send me the offer for the reporter role via Upwork?

@mountiny
Copy link
Contributor Author

@JmillsExpensify @anmurali also reporting bonus $250 to @tranvantoan-qn please, thanks!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Sep 11, 2023
@melvin-bot melvin-bot bot removed the Overdue label Sep 14, 2023
@JmillsExpensify
Copy link

No worries, I'll take it.

@JmillsExpensify
Copy link

@tranvantoan-qn Upwork job is now public here: https://www.upwork.com/jobs/~0140d2e85164cfdf25. Can you apply for $250 and I'll issue payment?

@tranvantoan-qn
Copy link

i've applied for the job, thanks!

@mountiny
Copy link
Contributor Author

just waiting for the payment now

@JmillsExpensify
Copy link

Hired in Upwork.

@JmillsExpensify
Copy link

All paid out and contract closed. I think we're done here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Internal Requires API changes or must be handled by Expensify staff
Projects
None yet
Development

No branches or pull requests

8 participants