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

[$250] Remove Add receipt from the three dot overflow menu #37060

Closed
JmillsExpensify opened this issue Feb 22, 2024 · 36 comments
Closed

[$250] Remove Add receipt from the three dot overflow menu #37060

JmillsExpensify opened this issue Feb 22, 2024 · 36 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 Engineering External Added to denote the issue can be worked on by a contributor

Comments

@JmillsExpensify
Copy link

JmillsExpensify commented Feb 22, 2024

Problem

We've recently added a receipt empty state in conjunction with the on-going violations project, though we also have an existing Add receipt option in the three dot overflow menu. Example below.

Screenshot 2024-02-21 at 17 00 02

Accordingly, we effectively have two options to do the same thing, within the same screen. That's confusing and unnecessary.

Solution

Let's remove the Add receipt option in the three dot overflow menu, such that the receipt empty state is the only/main flow for adding receipts.

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01462cd7dc3ef5fb51
  • Upwork Job ID: 1760460411785555968
  • Last Price Increase: 2024-02-22
  • Automatic offers:
    • s77rt | Reviewer | 0
    • Krishna2323 | Contributor | 0
@JmillsExpensify JmillsExpensify added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Feb 22, 2024
@JmillsExpensify JmillsExpensify self-assigned this Feb 22, 2024
Copy link

melvin-bot bot commented Feb 22, 2024

Current assignee @JmillsExpensify is eligible for the Bug assigner, not assigning anyone new.

@JmillsExpensify JmillsExpensify added 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 Feb 22, 2024
@melvin-bot melvin-bot bot changed the title Remove Add receipt from the three dot overflow menu [$500] Remove Add receipt from the three dot overflow menu Feb 22, 2024
Copy link

melvin-bot bot commented Feb 22, 2024

Job added to Upwork: https://www.upwork.com/jobs/~01462cd7dc3ef5fb51

Copy link

melvin-bot bot commented Feb 22, 2024

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

@JmillsExpensify JmillsExpensify changed the title [$500] Remove Add receipt from the three dot overflow menu [$250] Remove Add receipt from the three dot overflow menu Feb 22, 2024
Copy link

melvin-bot bot commented Feb 22, 2024

Upwork job price has been updated to $250

@JmillsExpensify
Copy link
Author

Super straightforward issue so updating the price accordingly.

@neonbhai
Copy link
Contributor

neonbhai commented Feb 22, 2024

Proposal

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

Remove Add receipt from the three dot overflow menu

What is the root cause of that problem?

New Feature

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

We will remove the option form MoneyRequestHeader here

We will also remove the beta violation check for receipt empty state here:

{!hasReceipt && canEditReceipt && canUseViolations && (
<ReceiptEmptyState

@Krishna2323
Copy link
Contributor

Proposal

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

Remove Add receipt from the three dot overflow menu

What is the root cause of that problem?

new feature

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

Need to remove the code below:

if (canModifyRequest) {
if (!TransactionUtils.hasReceipt(transaction)) {
threeDotsMenuItems.push({
icon: Expensicons.Receipt,
text: translate('receipt.addReceipt'),
onSelected: () =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
CONST.IOU.ACTION.EDIT,
CONST.IOU.TYPE.REQUEST,
transaction?.transactionID ?? '',
report.reportID,
Navigation.getActiveRouteWithoutParams(),
),
),
});
}

Result

@Krishna2323
Copy link
Contributor

Hey @neonbhai, you updated your proposal after mine, initially you were just pointing out this issue as reference which I could have also done before your proposal without checking the codebase and getting the correct reference. I hope that makes sense.

cc: @s77rt

@nexarvo
Copy link
Contributor

nexarvo commented Feb 22, 2024

Proposal

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

Remove Add receipt from the three dot overflow menu

What is the root cause of that problem?

New Request

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

We are pushing Add receipt to three dot menu. We need to remove code where it is being pushed to array, like this:

if (!TransactionUtils.hasReceipt(transaction)) {
threeDotsMenuItems.push({
icon: Expensicons.Receipt,
text: translate('receipt.addReceipt'),
onSelected: () =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
CONST.IOU.ACTION.EDIT,
CONST.IOU.TYPE.REQUEST,
transaction?.transactionID ?? '',
report.reportID,
Navigation.getActiveRouteWithoutParams(),
),
),
});
}

Copy link

melvin-bot bot commented Feb 22, 2024

📣 @usman-ghani564! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

  1. Make sure you've read and understood the contributing guidelines.
  2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
  3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
  4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
    Screen Shot 2022-11-16 at 4 42 54 PM
    Format:
Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

@nexarvo
Copy link
Contributor

nexarvo commented Feb 22, 2024

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01b5c4ee99eace8e60

Copy link

melvin-bot bot commented Feb 22, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@askavyblr
Copy link

Proposal

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

Remove Add receipt from the three dot overflow menu

What is the root cause of that problem?

New Feature

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

Need to remove Below Lines

if (!TransactionUtils.hasReceipt(transaction)) {
threeDotsMenuItems.push({
icon: Expensicons.Receipt,
text: translate('receipt.addReceipt'),
onSelected: () =>
Navigation.navigate(
ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
CONST.IOU.ACTION.EDIT,
CONST.IOU.TYPE.REQUEST,
transaction?.transactionID ?? '',
report.reportID,
Navigation.getActiveRouteWithoutParams(),
),
),
});

What alternative solutions did you explore? (Optional)

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@JmillsExpensify Currently the receipt empty state is only available for users with violations beta enabled. Is this still the case? Can we remove that restriction? cc @cead22 #30663

With beta Without beta
Screenshot 2024-02-22 at 6 13 40 AM Screenshot 2024-02-22 at 6 11 24 AM

@handipriyono
Copy link

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01d78cbc4edbcfa8e7

Copy link

melvin-bot bot commented Feb 22, 2024

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

@handipriyono
Copy link

Proposal

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

  • Remove Add receipt option in the three dot overflow menu

What is the root cause of that problem?

  • eliminate `Add receipt' from three dot menu ( currently still showing)

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

  • remove lines of code when "add receipt" menu being pushed to threeDotsMenuItems array. In this specific line.
    if (!TransactionUtils.hasReceipt(transaction)) {
    threeDotsMenuItems.push({
    icon: Expensicons.Receipt,
    text: translate('receipt.addReceipt'),
    onSelected: () =>
    Navigation.navigate(
    ROUTES.MONEY_REQUEST_STEP_SCAN.getRoute(
    CONST.IOU.ACTION.EDIT,
    CONST.IOU.TYPE.REQUEST,
    transaction?.transactionID ?? '',
    report.reportID,
    Navigation.getActiveRouteWithoutParams(),
    ),
    ),
    });
    }
Screenshot 2024-02-22 at 8 14 36 PM Screenshot 2024-02-22 at 8 17 37 PM

What alternative solutions did you explore? (Optional)

@JmillsExpensify
Copy link
Author

@JmillsExpensify Currently the receipt empty state is only available for users with violations beta enabled. Is this still the case? Can we remove that restriction? cc @cead22 #30663

With beta Without beta
Screenshot 2024-02-22 at 6 13 40 AM Screenshot 2024-02-22 at 6 11 24 AM

Yes, violations are still being implemented and under a beta. In fact, this is what has created the duplicate flows to do the same thing. So we want to remove Add receipt from the overflow menu as we're cleaning up this overall flow.

P.S. I also reached out to Carlos via DM to confirm he's aligned.

@cead22
Copy link
Contributor

cead22 commented Feb 22, 2024

That sounds good to me, and we can remove the beta check for violations that we currently have for display the receipt empty state

{!hasReceipt && canEditReceipt && canUseViolations && (
<ReceiptEmptyState

@neonbhai
Copy link
Contributor

Proposal

Updated

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@neonbhai Thanks for the proposal. Based on the edit timeline, @Krishna2323 was the first in proposing the appropriate changes.

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@Krishna2323 Thanks for the proposal. The suggested changes looks good to me.

🎀 👀 🎀 C+ reviewed
Link to proposal

Copy link

melvin-bot bot commented Feb 22, 2024

Triggered auto assignment to @hayata-suenaga, see https://stackoverflow.com/c/expensify/questions/7972 for more details.

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@usman-ghani564 Thanks for the proposal. Unfortunately your proposal is a dupe

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@askavyblr Thanks for the proposal. Your proposal is also a dupe.

@s77rt
Copy link
Contributor

s77rt commented Feb 22, 2024

@handipriyono Thanks for the proposal. This one is also a dupe.

@melvin-bot melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Feb 23, 2024
Copy link

melvin-bot bot commented Feb 23, 2024

📣 @s77rt 🎉 An offer has been automatically sent to your Upwork account for the Reviewer role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job

Copy link

melvin-bot bot commented Feb 23, 2024

📣 @Krishna2323 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link
Upwork job
Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Feb 25, 2024
@Krishna2323
Copy link
Contributor

@s77rt PR ready for review.

@Krishna2323
Copy link
Contributor

@JmillsExpensify friendly bump for payments here :)

@hayata-suenaga hayata-suenaga removed the Reviewing Has a PR in review label Mar 8, 2024
@Krishna2323
Copy link
Contributor

@JmillsExpensify bump for payments process 🙇🏻

Copy link

melvin-bot bot commented Mar 14, 2024

@JmillsExpensify @s77rt @Krishna2323 @hayata-suenaga 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 melvin-bot bot added Engineering Daily KSv2 and removed Weekly KSv2 labels Mar 14, 2024
@hayata-suenaga
Copy link
Contributor

waiting for payment

@melvin-bot melvin-bot bot added the Overdue label Mar 18, 2024
@Krishna2323
Copy link
Contributor

waiting for payment process by @JmillsExpensify

@melvin-bot melvin-bot bot removed the Overdue label Mar 18, 2024
@JmillsExpensify
Copy link
Author

Not sure why the automation failed, though please reach out to me via DM for these cases. I'm on hundreds of issues so it's hard to keep track.

@JmillsExpensify
Copy link
Author

Payment summary:

All contracts paid. Given that we are removing functionality, I don't think we need to create a regression test for this case.

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 External Added to denote the issue can be worked on by a contributor
Projects
None yet
Development

No branches or pull requests

9 participants