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

[ON HOLD Payment 4/3/24] [Violations] [$500] Don't show RBR on the LHN for transaction threads with violations if the report has already been reimbursed/settled #34548

Closed
1 of 6 tasks
cead22 opened this issue Jan 15, 2024 · 45 comments
Assignees
Labels
Awaiting Payment Auto-added when associated PR is deployed to production Bug Something is broken. Auto assigns a BugZero manager. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@cead22
Copy link
Contributor

cead22 commented Jan 15, 2024

  • Slack discussion (internal)
  • Transaction threads for transactions that have violations show up on the LHN with a RBR for the use that's requesting money
  • The idea behind RBR is to show the user a path to take an action to fix a potential issue
  • This is reasonable for the user requesting money because their requests/transactions may contain violations that can prevent them from getting approved and paid
  • Once the requests are paid, we agreed to continue showing violations in order to show that the request may not be in compliance with the current workspace rules, which is useful for the users requesting money, and also users paying those requests
  • However, the RBR specifically on the LHN isn't as relevant because the main idea behind that is to show a path to fix an issue, but in this case, the "issue" is the request has violations, but since they've been paid/settled, this isn't important for the requestor to do

Given that, let's not show RBR on the LHN for transaction threads with violations, if the transactions/money requests have been paid/settled

@trevor-coleman @lindboe can you please comment here so I can assign you? Thanks


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: v1.4.43-14
Reproducible in staging?: Presumably
Reproducible in production?: Presumably
Issue reported by: @situchan #35449 (comment)

Action Performed:

  • Make sure you're on the violations beta
  • Request money without adding a category from workspace that requires category (it should have a missing category violation)
  • Submit the request if you need to
  • Approve the request as a workspace admin
  • Open the transaction thread as the requester. You may need to navigate to another chat, and then back to the transaction thread

Expected Result:

  • RBR should not be shown on the LHN for the submitter

Actual Result:

  • RBR is shown on the LHN for the submitter

Platforms:

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

  • Android: Native
  • Android: mWeb Chrome
  • iOS: Native
  • iOS: mWeb Safari
  • MacOS: Chrome / Safari
  • MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence
image

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01e69e67564d338f8f
  • Upwork Job ID: 1761101186954891264
  • Last Price Increase: 2024-02-23
  • Automatic offers:
    • situchan | Reviewer | 0
    • FitseTLT | Contributor | 0
@cead22 cead22 added Engineering Daily KSv2 Improvement Item broken or needs improvement. labels Jan 15, 2024
@trevor-coleman
Copy link
Contributor

Commenting for assignment

@cead22
Copy link
Contributor Author

cead22 commented Jan 19, 2024

ReportUtils PR was just merged, so we're starting to work on other issues, but that one was higher priority

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 19, 2024
@cead22
Copy link
Contributor Author

cead22 commented Jan 24, 2024

Still a priority but other issues are being worked on first

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jan 24, 2024
Copy link

melvin-bot bot commented Jan 29, 2024

@cead22, @trevor-coleman Huh... This is 4 days overdue. Who can take care of this?

@lindboe
Copy link
Contributor

lindboe commented Jan 29, 2024

@cdanwards this is the ticket you were pulling into optimistic IDs, right? since we're already making RBR changes there?

@cead22
Copy link
Contributor Author

cead22 commented Jan 31, 2024

WIP

@melvin-bot melvin-bot bot added Overdue Reviewing Has a PR in review Weekly KSv2 and removed Overdue Daily KSv2 labels Jan 31, 2024
@situchan
Copy link
Contributor

Happy to take this as C+ since I already have context

@cead22 cead22 removed the Reviewing Has a PR in review label Feb 23, 2024
@cead22 cead22 added Improvement Item broken or needs improvement. Bug Something is broken. Auto assigns a BugZero manager. and removed Improvement Item broken or needs improvement. labels Feb 23, 2024
@melvin-bot melvin-bot bot removed the Overdue label Mar 4, 2024
@zanyrenney
Copy link
Contributor

nice, thank you @FitseTLT

@JmillsExpensify
Copy link

Following on that last comment, we're getting lots of reports of this issue in the app, so it would be super ideal if we can get this PR raised and merged this week.

@FitseTLT
Copy link
Contributor

FitseTLT commented Mar 5, 2024

PR is on review stage 👍

@JmillsExpensify JmillsExpensify changed the title [$500] Don't show RBR on the LHN for transaction threads with violations if the report has already been reimbursed/settled [Violations] [$500] Don't show RBR on the LHN for transaction threads with violations if the report has already been reimbursed/settled Mar 12, 2024
@JmillsExpensify
Copy link

Good movement on the linked PR. Ideally we can even get it merged before the week closes out.

@cead22
Copy link
Contributor Author

cead22 commented Mar 21, 2024

PR is merged

@sakluger
Copy link
Contributor

FYI this still seems to be broken for me. I left a more detailed comment in the PR: #37767 (comment).

@cead22
Copy link
Contributor Author

cead22 commented Mar 27, 2024

I'm pretty sure that error is unrelated, and this is happening because OpenReport is failing with a JSON error
image

It looks like this started on Friday
image

@cead22
Copy link
Contributor Author

cead22 commented Mar 27, 2024

I found the issue and submitted a PR

@cead22
Copy link
Contributor Author

cead22 commented Mar 27, 2024

That said, the problem is unrelated to this GH issue, so I'm going to close this one since it's done

@cead22 cead22 closed this as completed Mar 27, 2024
@FitseTLT
Copy link
Contributor

@cead22 What about Payments?? 😆

@cead22
Copy link
Contributor Author

cead22 commented Mar 27, 2024

Argh, they always get me. I see the upwork jobs were created, but not sure what else we need for payments -- @CortneyOfstad @zanyrenney can you help with this please?

@FitseTLT
Copy link
Contributor

It was just deployed today. I am only claiming to reopen it 👍

@cead22 cead22 reopened this Mar 27, 2024
@CortneyOfstad CortneyOfstad added the Awaiting Payment Auto-added when associated PR is deployed to production label Mar 27, 2024
@CortneyOfstad
Copy link
Contributor

CortneyOfstad commented Mar 27, 2024

No worries — since it was just deployed today, we will need to wait the 7-day period, so going to adjust the title to reflect the payment date if there are no regressions 👍

PR link here ¸— #37767

@CortneyOfstad CortneyOfstad changed the title [Violations] [$500] Don't show RBR on the LHN for transaction threads with violations if the report has already been reimbursed/settled [ON HOLD Payment 4/3/24] [Violations] [$500] Don't show RBR on the LHN for transaction threads with violations if the report has already been reimbursed/settled Mar 27, 2024
@CortneyOfstad
Copy link
Contributor

Payment has been completed! Will post payment summary below.

@situchan does this need a regression test? For some reason the checklist is not populating, so just wanted to confirm. Thanks!

@CortneyOfstad
Copy link
Contributor

Payment Summary

@FitseTLT (Contributor) — paid $500 via Upwork
@situchan (C+) — paid $500 via Upwork

@zanyrenney
Copy link
Contributor

Seeing as payment is complete, we can close this one out. Thanks for managing the payment @CortneyOfstad ❤️

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. Engineering External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: Done
Development

No branches or pull requests

10 participants