[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link #42925

lanitochka17 · 2024-05-31T14:12:30Z

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: 1.4.78-2
Reproducible in staging?: Y
Reproducible in production?: Y
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Issue reported by: Applause - Internal Team

Issue found when executing PR #42192

Action Performed:

Pre-requisite:

Account should be integrated with a xero account which already had configured some taxes
(Use these guides to do that: https://central.xero.com/s/article/Add-or-edit-tax-rates-GL#:~:text=a%20tax%20rate-,In%20the%20Accounting%20menu%2C%20select%20Advanced%2C%20then%20click%20Tax%20rates,Click%20Save.
)

Navigate to staging.new.expensify.com
Import taxes on Accounted page if not already imported
Go to taxes page and deep link to add new tax rate page (i.e change the url to look like this: https://staging.new.expensify.com/settings/workspaces/<WORKSPACE_ID>/taxes/new

Expected Result:

Not found page shows up

Actual Result:

User is able to add a new tax rate

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Add any screenshot/video evidence

Bug6497169_1717144406016.bandicam_2024-05-31_11-28-11-821.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~012da44be2618c21c5
Upwork Job ID: 1797644044811587584
Last Price Increase: 2024-06-03
Automatic offers:

tienifr | Contributor | 102596329

Issue Owner

Current Issue Owner: @puneetlath

melvin-bot · 2024-05-31T14:12:42Z

Triggered auto assignment to @puneetlath (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details. Please add this bug to a GH project, as outlined in the SO.

lanitochka17 · 2024-05-31T14:13:25Z

@puneetlath FYI I haven't added the External label as I wasn't 100% sure about this issue. Please take a look and add the label if you agree it's a bug and can be handled by external contributors

lanitochka17 · 2024-05-31T14:13:43Z

We think that this bug might be related to #wave-collect - Release 1

ShridharGoel · 2024-05-31T14:34:08Z

Proposal

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

Taxes - User is able to add new tax rate using deep link.

What is the root cause of that problem?

In WorkspaceCreateTaxPage we are not blocking access if hasAccountingConnections is true.

App/src/pages/workspace/taxes/WorkspaceCreateTaxPage.tsx

Lines 64 to 68 in 81fedce

    
           <AccessOrNotFoundWrapper 
        
               accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]} 
        
               policyID={policyID} 
        
               featureName={CONST.POLICY.MORE_FEATURES.ARE_TAXES_ENABLED} 
        
           >

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

Add shouldOpenProfileOnBackPress as a prop in AccessOrNotFoundWrapper:

Update onBackButtonPress:

onBackButtonPress={shouldOpenProfileOnBackPress ? () => Navigation.goBack(policyID ? ROUTES.WORKSPACE_PROFILE.getRoute(policyID) : ROUTES.HOME) : () => Navigation.goBack()}

We also need to discuss if we want to keep shouldOpenProfileOnBackPress as true by default or should we keep it as false. If we want to keep the existing flow in other places, we will need to make it's default value as true.

function AccessOrNotFoundWrapper({accessVariants = [], fullPageNotFoundViewProps, shouldBeBlocked, shouldOpenProfileOnBackPress = true, ...props}: AccessOrNotFoundWrapperProps)

shouldOpenProfileOnBackPress will be passed to PageNotFoundFallback.

Add the below in AccessOrNotFoundWrapper:

shouldBeBlocked={PolicyUtils.hasAccountingConnections(policy)}

It will look like this:

<AccessOrNotFoundWrapper
    accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
    policyID={policyID}
    featureName={CONST.POLICY.MORE_FEATURES.ARE_TAXES_ENABLED}
    shouldBeBlocked={PolicyUtils.hasAccountingConnections(policy)}
    shouldOpenProfileOnBackPress={false}
>

Same needs to be done for categories also:

Get policy using:

const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${route.params.policyID}`);

Then update like below:

<AccessOrNotFoundWrapper
    accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
    policyID={route.params.policyID}
    featureName={CONST.POLICY.MORE_FEATURES.ARE_CATEGORIES_ENABLED}
    shouldBeBlocked={PolicyUtils.hasAccountingConnections(policy)}
    shouldOpenProfileOnBackPress={false}
>

We need to do the same for tag page as well. Also, check if same needs to be done at some other place.

const [policy] = useOnyx(`${ONYXKEYS.COLLECTION.POLICY}${route.params.policyID}`);

<AccessOrNotFoundWrapper
    accessVariants={[CONST.POLICY.ACCESS_VARIANTS.ADMIN, CONST.POLICY.ACCESS_VARIANTS.PAID]}
    policyID={route.params.policyID}
    featureName={CONST.POLICY.MORE_FEATURES.ARE_TAGS_ENABLED}
    shouldBeBlocked={PolicyUtils.hasAccountingConnections(policy) || PolicyUtils.isMultiLevelTags(policyTags)}
    shouldOpenProfileOnBackPress={false}
>

tienifr · 2024-06-01T18:37:56Z

Proposal

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

User is able to add a new tax rate

What is the root cause of that problem?

In WorkspaceCreateTaxPage, we do not display the not found page if PolicyUtils.hasAccountingConnections(policy) like we disable the "Add rate" button in here.

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

In WorkspaceCreateTaxPage, we need to display the not found page if PolicyUtils.hasAccountingConnections(policy). And then we need to make sure when user click on back button, the workspace taxes page is displayed.
To do it, we can user <FullPageNotFoundView shouldShow={PolicyUtils.hasAccountingConnections(policy)}> in here.
We can apply the same fix to other similar pages.

What alternative solutions did you explore? (Optional)

NA

melvin-bot · 2024-06-03T14:58:36Z

Job added to Upwork: https://www.upwork.com/jobs/~012da44be2618c21c5

melvin-bot · 2024-06-03T14:58:41Z

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

Ollyws · 2024-06-04T13:31:25Z

@ShridharGoel Thanks for the proposal but it will cause the back button to navigate to the wrong page.
@tienifr's proposal LGTM.
🎀👀🎀 C+ reviewed

melvin-bot · 2024-06-04T13:31:34Z

Current assignee @puneetlath is eligible for the choreEngineerContributorManagement assigner, not assigning anyone new.

ShridharGoel · 2024-06-04T14:06:51Z

Proposal

Updated to fix back button flow. Using the existing AccessOrNotFound is better than adding a new FullPageNotFound.

Also, the new prop would be useful in the future as well.

@Ollyws @puneetlath Kindly check.

ShridharGoel · 2024-06-04T14:10:28Z

Screen.Recording.2024-06-04.at.7.39.23.PM.mov

ShridharGoel · 2024-06-04T17:47:33Z

@Ollyws Bump on the above.

melvin-bot · 2024-06-04T19:48:05Z

📣 @tienifr 🎉 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 📖

puneetlath · 2024-06-04T19:59:21Z

@Ollyws do you want to take a look?

tienifr · 2024-06-05T04:09:36Z

@Ollyws PR #43087 is ready

ShridharGoel · 2024-06-08T10:18:41Z

Hello @Ollyws, my comments were ignored but at least some explanation could have been provided.

Ollyws · 2024-06-10T11:03:01Z

Ah sorry about that but your initial proposal would have caused a regression so I just went with the first working proposal, it was only modified after I'd reviewed it. But I think it was better to keep things simple here anyway.

melvin-bot · 2024-06-11T23:27:51Z

Reviewing label has been removed, please complete the "BugZero Checklist".

melvin-bot · 2024-06-11T23:27:54Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.81-11 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix User is able to add new tax rate using deep link #43087

If no regressions arise, payment will be issued on 2024-06-18. 🎊

For reference, here are some details about the assignees on this issue:

@Ollyws requires payment through NewDot Manual Requests
@tienifr requires payment automatic offer (Contributor)

melvin-bot · 2024-06-11T23:27:55Z

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:

[@Ollyws] The PR that introduced the bug has been identified. Link to the PR:
[@Ollyws] 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:
[@Ollyws] 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:
[@Ollyws] Determine if we should create a regression test for this bug.
[@Ollyws] 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.
[@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

melvin-bot · 2024-06-13T15:25:51Z

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.4.82-4 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

fix User is able to add new tax rate using deep link #43087

If no regressions arise, payment will be issued on 2024-06-20. 🎊

For reference, here are some details about the assignees on this issue:

@Ollyws requires payment through NewDot Manual Requests
@tienifr requires payment automatic offer (Contributor)

melvin-bot · 2024-06-13T15:25:52Z

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:

[@Ollyws] The PR that introduced the bug has been identified. Link to the PR:
[@Ollyws] 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:
[@Ollyws] 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:
[@Ollyws] Determine if we should create a regression test for this bug.
[@Ollyws] 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.
[@puneetlath] Link the GH issue for creating/updating the regression test once above steps have been agreed upon:

Ollyws · 2024-06-20T12:46:51Z

BugZero Checklist:

The PR that introduced the bug has been identified. Link to the PR:

#40598

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:

https://github.com/Expensify/App/pull/40598/files#r1647514401

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:

N/A

Determine if we should create a regression test for this bug.

Yes

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.

Regression Test Proposal

Pre-requisite: Account should be integrated with a xero account which already had configured some taxes

1. Open app
2. Import taxes on Accounted page if not already imported
3. Go to taxes page and deep link to add new tax rate page (i.e change the url to look like this: https://staging.new.expensify.com/settings/workspaces/<WORKSPACE_ID>/taxes/new)
4. Verify not found page shows up

Do we agree 👍 or 👎

Ollyws · 2024-06-20T12:49:34Z

Requested payment in ND.

JmillsExpensify · 2024-06-20T16:22:46Z

Will approve the payment once I have the payment summary!

puneetlath · 2024-06-20T18:10:03Z

Payment summary:

C - @tienifr - $250 - paid via Upwork
C+ - @Ollyws - $250 - to be paid via NewDot

Thanks everyone!

JmillsExpensify · 2024-06-21T18:03:54Z

$250 approved for @Ollyws

lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels May 31, 2024

melvin-bot bot assigned puneetlath May 31, 2024

melvin-bot bot added the Overdue label Jun 3, 2024

puneetlath added the External Added to denote the issue can be worked on by a contributor label Jun 3, 2024

melvin-bot bot changed the title ~~Taxes - User is able to add new tax rate using deep link~~ [$250] Taxes - User is able to add new tax rate using deep link Jun 3, 2024

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 3, 2024

melvin-bot bot assigned Ollyws Jun 3, 2024

melvin-bot bot removed the Overdue label Jun 3, 2024

puneetlath assigned tienifr Jun 4, 2024

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Jun 4, 2024

tienifr mentioned this issue Jun 5, 2024

fix User is able to add new tax rate using deep link #43087

Merged

50 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Jun 5, 2024

melvin-bot bot added Weekly KSv2 Awaiting Payment Auto-added when associated PR is deployed to production and removed Weekly KSv2 labels Jun 11, 2024

melvin-bot bot changed the title ~~[$250] Taxes - User is able to add new tax rate using deep link~~ [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link Jun 11, 2024

melvin-bot bot removed the Reviewing Has a PR in review label Jun 11, 2024

melvin-bot bot added Weekly KSv2 and removed Weekly KSv2 labels Jun 13, 2024

melvin-bot bot changed the title ~~[HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link~~ [HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link Jun 13, 2024

melvin-bot bot added Daily KSv2 and removed Weekly KSv2 labels Jun 18, 2024

Ollyws mentioned this issue Jun 20, 2024

[Simplified Collect][CLEAN UP][LOW] Standardize on Workspace wrapper #40598

Merged

50 tasks

puneetlath closed this as completed Jun 20, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link #42925

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link #42925

lanitochka17 commented May 31, 2024 •

edited by melvin-bot bot

Loading

melvin-bot bot commented May 31, 2024

lanitochka17 commented May 31, 2024

lanitochka17 commented May 31, 2024

ShridharGoel commented May 31, 2024 •

edited

Loading

tienifr commented Jun 1, 2024

melvin-bot bot commented Jun 3, 2024

melvin-bot bot commented Jun 3, 2024

Ollyws commented Jun 4, 2024

melvin-bot bot commented Jun 4, 2024

ShridharGoel commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024

melvin-bot bot commented Jun 4, 2024

puneetlath commented Jun 4, 2024

tienifr commented Jun 5, 2024

ShridharGoel commented Jun 8, 2024

Ollyws commented Jun 10, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 13, 2024

melvin-bot bot commented Jun 13, 2024

Ollyws commented Jun 20, 2024

Ollyws commented Jun 20, 2024

JmillsExpensify commented Jun 20, 2024

puneetlath commented Jun 20, 2024

JmillsExpensify commented Jun 21, 2024

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link #42925

[HOLD for payment 2024-06-20] [HOLD for payment 2024-06-18] [$250] Taxes - User is able to add new tax rate using deep link #42925

Comments

lanitochka17 commented May 31, 2024 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented May 31, 2024

lanitochka17 commented May 31, 2024

lanitochka17 commented May 31, 2024

ShridharGoel commented May 31, 2024 • edited Loading

Proposal

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

What is the root cause of that problem?

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

tienifr commented Jun 1, 2024

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

melvin-bot bot commented Jun 3, 2024

melvin-bot bot commented Jun 3, 2024

Ollyws commented Jun 4, 2024

melvin-bot bot commented Jun 4, 2024

ShridharGoel commented Jun 4, 2024 • edited Loading

Proposal

ShridharGoel commented Jun 4, 2024 • edited Loading

ShridharGoel commented Jun 4, 2024

melvin-bot bot commented Jun 4, 2024

puneetlath commented Jun 4, 2024

tienifr commented Jun 5, 2024

ShridharGoel commented Jun 8, 2024

Ollyws commented Jun 10, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 11, 2024

melvin-bot bot commented Jun 13, 2024

melvin-bot bot commented Jun 13, 2024

Ollyws commented Jun 20, 2024

Ollyws commented Jun 20, 2024

JmillsExpensify commented Jun 20, 2024

puneetlath commented Jun 20, 2024

JmillsExpensify commented Jun 21, 2024

lanitochka17 commented May 31, 2024 •

edited by melvin-bot bot

Loading

ShridharGoel commented May 31, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading

ShridharGoel commented Jun 4, 2024 •

edited

Loading