[DISTANCE] MEDIUM: [$500] Only allow admins to manually edit distance request amount and currency #30654

m-natarajan · 2023-10-31T20:08:34Z

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.3.93-1
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
Expensify/Expensify Issue URL:
Issue reported by: @esh-g
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1698001650923669

Action Performed:

Make a new distance request.
Go to the request.
After it has loaded, go offline.
Change the currency in the amount.
Go back online.

Expected Result:

The currency should be changed

Actual Result:

The currency is not changed.

Workaround:

unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

MacOS: Chrome / Safari

Screen.Recording.2023-10-23.at.12.21.12.AM.mov

Recording.188.mp4

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~01f9c85c43ba1d6c35
Upwork Job ID: 1719835802236735488
Last Price Increase: 2023-11-01
Automatic offers:

alitoshmatov | Reviewer | 27772651
DylanDylann | Contributor | 27772653
esh-g | Reporter | 27772658

Issue Owner

Current Issue Owner: @Gonals

DylanDylann · 2023-10-31T20:08:35Z

Proposal

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

Unable to change currency when offline in distance request

What is the root cause of that problem?

Currently, when calling UpdateDistanceRequest with currency data, BE does not update the currency, it still returns the original currency.
When the user is in offline mode, and updates currency from "A" to "B", optimisticData is update to B. Then turn on the network, API will be called and updated to currency to A. That leads to the current behavior.

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

Firstly, please check this discussion: https://expensify.slack.com/archives/C01GTK53T8Q/p1698949355446899?thread_ts=1697575566.549679&cid=C01GTK53T8Q

We should not allow editing the amount or currency when creating distance request or editing distance request.
We only allow the admin of the workspace to edit the amount or currency when editing distance request.

We need to disable the amount field when creating distance request
In here

App/src/components/MoneyRequestConfirmationList.js

Line 593 in cd1c2bd

interactive={!props.isReadOnly}

we should update like this

interactive={!props.isReadOnly && !props.isDistanceRequest}

We only allow admin of workspace to edit the amount field when editing distance request
In here

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 185 to 186 in cd1c2bd

interactive={canEdit && !isSettled}

shouldShowRightIcon={canEdit && !isSettled}

Update like this

interactive={canEdit && !isSettled && (isDistanceRequest ? isAdmin : true)}
shouldShowRightIcon={canEdit && !isSettled && (isDistanceRequest ? isAdmin : true)}

Prevent user (exclude admin) from going to edit amount page when editing distance request by deeplink
In here

App/src/pages/EditRequestPage.js

Line 167 in 11a7170

if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT) {

update like this

    if (fieldToEdit === CONST.EDIT_REQUEST_FIELD.AMOUNT && (TransactionUtils.isDistanceRequest(transaction) ? isAdmin : true)) {

OR, we can dismiss modal if the user goes to the edit page by deep link

Prevent the user from going to amount page when creating distance request by deeplink
In here

App/src/pages/iou/steps/NewRequestAmountPage.js

Line 167 in 11a7170

<FullPageNotFoundView shouldShow={!IOUUtils.isValidMoneyRequestType(iouType)}>

update like this

                <FullPageNotFoundView shouldShow={!IOUUtils.isValidMoneyRequestType(iouType) || isDistanceRequestTab}>

OR, we can dismiss modal if the user goes to the edit page by deep link

We also need update from BE follow this requirement:
We should not allow editing the amount or currency when creating distance request or editing distance request.
We only allow the admin of the workspace to edit the amount or currency when editing distance request.

melvin-bot · 2023-10-31T20:08:40Z

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

melvin-bot · 2023-10-31T20:08:42Z

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 · 2023-11-01T21:56:25Z

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

melvin-bot · 2023-11-01T21:56:30Z

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

samilabud · 2023-11-02T03:02:10Z

Proposal

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

Unable to change currency when offline in distance request

What is the root cause of that problem?

When we update the currency using 'distance request' we use a method called IOU.updateDistanceRequest, this method doesn't update the currency (this behavior occurs online - if you update the page you can see the currency is the first one you selected- and offline).

App/src/pages/EditRequestPage.js

Line 128 in 5d45f0f

    
           IOU.updateDistanceRequest(transaction.transactionID, report.reportID, transactionChanges);

These are the API request and response of IOU.updateDistanceRequest function:

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

I think the simplest change is to call the function (used in the manual request) IOU.editMoneyRequest method when using 'distance request' as shown below :

App/src/pages/EditRequestPage.js

Line 126 in 5d45f0f

function editMoneyRequest(transactionChanges) {

function editMoneyRequest(transactionChanges) {
        if (TransactionUtils.isDistanceRequest(transaction)) {
            IOU.updateDistanceRequest(transaction.transactionID, report.reportID, transactionChanges);
            IOU.editMoneyRequest(transaction.transactionID, report.reportID, transactionChanges);
        } else {
            IOU.editMoneyRequest(transaction.transactionID, report.reportID, transactionChanges);
        }
        Navigation.dismissModal(report.reportID);
    }

This is the result:

Fixed.Unable.to.change.currency.when.offline.in.distance.request.test.mp4

bernhardoj · 2023-11-02T09:33:03Z

Proposal

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

Changing the distance request currency will revert back to the previous currency.

What is the root cause of that problem?

When we change the currency of a distance request, we will optimistically update it, but the BE actually doesn't allow it, so the value/currency will be reverted.

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

Currently, BE doesn't allow us to change the distance request currency, but allows us to change the distance request amount. I'm pretty sure we want to disallow both because on the distance request confirm page, the amount button is completely disabled. This is because the amount is calculated from the distance.

So, my suggestion is to also disable the amount button on the report/chat page if it's a distance request.

App/src/components/ReportActionItem/MoneyRequestView.js

Lines 179 to 190 in 3db5ff4

    
           <MenuItemWithTopDescription 
        
               title={formattedTransactionAmount ? formattedTransactionAmount.toString() : ''} 
        
               shouldShowTitleIcon={isSettled} 
        
               titleIcon={Expensicons.Checkmark} 
        
               description={amountDescription} 
        
               titleStyle={styles.newKansasLarge} 
        
               interactive={canEdit && !isSettled} 
        
               shouldShowRightIcon={canEdit && !isSettled} 
        
               onPress={() => Navigation.navigate(ROUTES.EDIT_REQUEST.getRoute(report.reportID, CONST.EDIT_REQUEST_FIELD.AMOUNT))} 
        
               brickRoadIndicator={hasErrors && transactionAmount === 0 ? CONST.BRICK_ROAD_INDICATOR_STATUS.ERROR : ''} 
        
               error={hasErrors && transactionAmount === 0 ? translate('common.error.enterAmount') : ''} 
        
           />

interactive={canEdit && !isSettled && !isDistanceRequest}
shouldShowRightIcon={canEdit && !isSettled && !isDistanceRequest}

melvin-bot · 2023-11-07T13:53:29Z

@twisterdotcom, @alitoshmatov Eep! 4 days overdue now. Issues have feelings too...

twisterdotcom · 2023-11-07T14:59:11Z

Some internal discussions actually ongoing now about whether to even allow changing the currency of a distance request. Please HOLD. https://expensify.slack.com/archives/C03SSAQ7P/p1698248408690509

cc @neil-marcellini

twisterdotcom · 2023-11-09T21:48:25Z

Okay, @neil-marcellini am I right now that we don't want to allow for changing the currency at all now? Once it's created, the currency is set by the workspace it's created on.

alitoshmatov · 2023-11-13T09:31:07Z

Reviewing

alitoshmatov · 2023-11-13T16:32:47Z

@DylanDylann proposal looks good and tests well, also it covers all cases where page is accessed through deeplink.

C+ reviewed 🎀 👀 🎀

melvin-bot · 2023-11-13T16:32:54Z

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

alitoshmatov · 2023-11-13T16:33:44Z

@samilabud Your proposal misses a point which we are not allowing changing currency

@bernhardoj your proposal is similar to @DylanDylann 's proposal, also it does not cover deeplink access

twisterdotcom · 2023-11-13T21:12:08Z

@neil-marcellini would love your technical input here. We won't allow currency changes at all now, we will just eventually allow moving different currency distance expenses between different currency workspaces. If so, I think we should close this, as offline is working fine and we would need to change online.

neil-marcellini · 2023-12-13T19:04:17Z

The Auth PR has changes requested, because we switched to a new command while I was working on this, so I need to move the changes over there before putting this up for review again.

melvin-bot · 2023-12-19T11:07:11Z

@twisterdotcom, @neil-marcellini, @alitoshmatov, @DylanDylann Eep! 4 days overdue now. Issues have feelings too...

neil-marcellini · 2023-12-19T15:57:07Z

I'll try to squeeze in some updates to my backend PR tomorrow.

neil-marcellini · 2023-12-20T17:32:45Z

I got the backend PR up for review again

neil-marcellini · 2023-12-26T16:40:34Z

The backend PR was finally merged and should be deployed today. @DylanDylann you can probably finish off the frontend PR soon.

neil-marcellini · 2023-12-27T17:48:34Z

The Auth PR was deployed

DylanDylann · 2023-12-28T03:27:35Z

Thanks @neil-marcellini

neil-marcellini · 2023-12-28T20:31:00Z

❗ 🔥 Heads up, I'm going to be OOO working 0% from 1/1-1/12/24. I'll be back on 1/15/24. I'm planning to work 50% tomorrow 12/29. Please feel free to un-assign, re-assign, whatever. When I return I'll pick back up whatever I can and do my best to GSD in order of priority. I will also post all of my assigned issues in #engineering-chat in Slack to try to recruit volunteers.

alitoshmatov · 2024-01-04T13:35:27Z

@twisterdotcom PR is ready for a review by internal engineer, can you reassign another internal for engineer since @neil-marcellini is OOO till 15th January

twisterdotcom · 2024-01-04T14:51:33Z

@alitoshmatov could you link the PR? I'll find an engineer.

melvin-bot · 2024-01-04T14:51:48Z

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

twisterdotcom · 2024-01-04T14:52:07Z

Eugh, it will do the same for Neil. Let me see who's on the Wave 5 team.

alitoshmatov · 2024-01-04T14:55:52Z

The PR is here #31778

alitoshmatov · 2024-01-18T12:47:48Z

The PR was deployed to product in 9th of january. I think this one is ready for payment

cc: @twisterdotcom

twisterdotcom · 2024-01-18T17:39:27Z

Weird we didn't get the automation. Okay, gonna send offers to you all.

twisterdotcom · 2024-01-18T17:49:46Z

Oh wait, the offers are there. Weird. Sorry for the duplicate offers - I withdrew the ones I just made and paid the ones we automated last year. Okay:

Payment Summary:

@alitoshmatov paid $500 here (C+)
@DylanDylann paid $500 here (Contributor)
@esh-g paid $50 here (Reporter)

twisterdotcom · 2024-01-18T17:50:05Z

Gonna close, no need for regression.

m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 31, 2023

melvin-bot bot assigned twisterdotcom Oct 31, 2023

twisterdotcom added the External Added to denote the issue can be worked on by a contributor label Nov 1, 2023

melvin-bot bot changed the title ~~Unable to change currency when offline in distance request~~ [$500] Unable to change currency when offline in distance request Nov 1, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 1, 2023

melvin-bot bot assigned alitoshmatov Nov 1, 2023

melvin-bot bot added the Overdue label Nov 6, 2023

melvin-bot bot removed the Overdue label Nov 7, 2023

twisterdotcom changed the title ~~[$500] Unable to change currency when offline in distance request~~ [HOLD] [$500] Unable to change currency when offline in distance request Nov 7, 2023

melvin-bot bot added the Overdue label Nov 9, 2023

melvin-bot bot added Overdue and removed Overdue labels Nov 9, 2023

melvin-bot bot removed the Overdue label Nov 13, 2023

melvin-bot bot assigned MonilBhavsar Nov 13, 2023

neil-marcellini added the Reviewing Has a PR in review label Dec 9, 2023

neil-marcellini removed the Reviewing Has a PR in review label Dec 13, 2023

melvin-bot bot added the Overdue label Dec 18, 2023

melvin-bot bot removed the Overdue label Dec 19, 2023

neil-marcellini added the Reviewing Has a PR in review label Dec 20, 2023

melvin-bot bot added Weekly KSv2 and removed Daily KSv2 labels Jan 2, 2024

twisterdotcom added Internal Requires API changes or must be handled by Expensify staff and removed Internal Requires API changes or must be handled by Expensify staff labels Jan 4, 2024

Gonals assigned Gonals and neil-marcellini and unassigned neil-marcellini Jan 4, 2024

twisterdotcom closed this as completed Jan 18, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[DISTANCE] MEDIUM: [$500] Only allow admins to manually edit distance request amount and currency #30654

[DISTANCE] MEDIUM: [$500] Only allow admins to manually edit distance request amount and currency #30654

m-natarajan commented Oct 31, 2023 •

edited by Gonals

Loading

DylanDylann commented Oct 31, 2023 •

edited

Loading

melvin-bot bot commented Oct 31, 2023

melvin-bot bot commented Oct 31, 2023

melvin-bot bot commented Nov 1, 2023

melvin-bot bot commented Nov 1, 2023

samilabud commented Nov 2, 2023

bernhardoj commented Nov 2, 2023

melvin-bot bot commented Nov 7, 2023

twisterdotcom commented Nov 7, 2023

twisterdotcom commented Nov 9, 2023

alitoshmatov commented Nov 13, 2023

alitoshmatov commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

alitoshmatov commented Nov 13, 2023 •

edited

Loading

twisterdotcom commented Nov 13, 2023

neil-marcellini commented Dec 13, 2023

melvin-bot bot commented Dec 19, 2023

neil-marcellini commented Dec 19, 2023

neil-marcellini commented Dec 20, 2023

neil-marcellini commented Dec 26, 2023

neil-marcellini commented Dec 27, 2023

DylanDylann commented Dec 28, 2023

neil-marcellini commented Dec 28, 2023

alitoshmatov commented Jan 4, 2024

twisterdotcom commented Jan 4, 2024

melvin-bot bot commented Jan 4, 2024

twisterdotcom commented Jan 4, 2024

alitoshmatov commented Jan 4, 2024

alitoshmatov commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

[DISTANCE] MEDIUM: [$500] Only allow admins to manually edit distance request amount and currency #30654

[DISTANCE] MEDIUM: [$500] Only allow admins to manually edit distance request amount and currency #30654

Comments

m-natarajan commented Oct 31, 2023 • edited by Gonals Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

DylanDylann commented Oct 31, 2023 • 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?

melvin-bot bot commented Oct 31, 2023

melvin-bot bot commented Oct 31, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Nov 1, 2023

melvin-bot bot commented Nov 1, 2023

samilabud commented Nov 2, 2023

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?

bernhardoj commented Nov 2, 2023

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?

melvin-bot bot commented Nov 7, 2023

twisterdotcom commented Nov 7, 2023

twisterdotcom commented Nov 9, 2023

alitoshmatov commented Nov 13, 2023

alitoshmatov commented Nov 13, 2023

melvin-bot bot commented Nov 13, 2023

alitoshmatov commented Nov 13, 2023 • edited Loading

twisterdotcom commented Nov 13, 2023

neil-marcellini commented Dec 13, 2023

melvin-bot bot commented Dec 19, 2023

neil-marcellini commented Dec 19, 2023

neil-marcellini commented Dec 20, 2023

neil-marcellini commented Dec 26, 2023

neil-marcellini commented Dec 27, 2023

DylanDylann commented Dec 28, 2023

neil-marcellini commented Dec 28, 2023

alitoshmatov commented Jan 4, 2024

twisterdotcom commented Jan 4, 2024

melvin-bot bot commented Jan 4, 2024

twisterdotcom commented Jan 4, 2024

alitoshmatov commented Jan 4, 2024

alitoshmatov commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

twisterdotcom commented Jan 18, 2024

m-natarajan commented Oct 31, 2023 •

edited by Gonals

Loading

DylanDylann commented Oct 31, 2023 •

edited

Loading

alitoshmatov commented Nov 13, 2023 •

edited

Loading