[HOLD for payment 2024-12-12] [$250] Support-Logged-In Agents Can Appear to Delete Workspaces in NewDot Without Actual Deletion or Error Message

[m-natarajan]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Version Number:
Reproducible in staging?: Needs Reproduction(No access to supportal)
Reproducible in production?: Needs Reproduction(No access to supportal)
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @RachCHopkins
Slack conversation (hyperlinked to channel name): ts_external_expensify_bugs

Action Performed:

1. Login in to a newdot account as support agent
2. Delete a workspace

Expected Result:

When a support agent attempts to delete a customer's workspace in NewDot while support-logged-in:
1.The UI should display an appropriate error message, similar to OldDot, indicating that the deletion is not allowed.
2.The workspace should remain visible and unchanged in the UI.
3.The UI should not give any misleading indication that the workspace has been successfully deleted.

Actual Result:

The UI appears to allow the deletion, showing no error message.
Even after refreshing, the workspace seems to be deleted from the NewDot
Upon returning to the main Supportal page, the workspace is still present, confirming it was not actually deleted.

Workaround:

Unknown

Platforms:

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

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

2024-11-20_08-50-52.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021860949442956139975
• Upwork Job ID: 1860949442956139975
• Last Price Increase: 2024-11-25
• Automatic offers:
• allgandalf | Reviewer | 105128268
• twilight2294 | Contributor | 105128269

Issue Owner

Current Issue Owner: @

Issue Owner

Current Issue Owner: @RachCHopkins

[melvin-bot]

Triggered auto assignment to @MitchExpensify (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.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[RachCHopkins]

@MitchExpensify I'm happy to manage this one, but would you mind confirming that you can repro from your own NewDot supportal?

[allgandalf]

please also assign me here with @RachCHopkins , we are going back and forth on this one on slack 👋

[MitchExpensify]

@MitchExpensify I'm happy to manage this one, but would you mind confirming that you can repro from your own NewDot supportal?

Confirmed I can reproduce

[melvin-bot]

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

[melvin-bot]

Current assignee @allgandalf is eligible for the External assigner, not assigning anyone new.

[allgandalf]

Not overdue, waiting for proposals / agency to pick up

[allgandalf]

@Expensify/design What should be the expected result here when a person supportal-logged in tries to delete a workspace ?

On OD we show them a modal saying they are not allowed to take the action:

If we want to show the same on ND, a mock would be great 🙏

[shawnborton]

We have a very simple modal alert pattern we can reuse here, something like this:

[twilight2294]

Proposal

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

Agents Can Appear to Delete Workspaces in NewDot Without Actual Deletion or Error Message

What is the root cause of that problem?

We do not have any check in places to see if the action is requested by support agent, so everytime we call the delete workspace, we directly show the modal to delete:

App/src/pages/workspace/WorkspacesListPage.tsx

Lines 170 to 175 in 967cc5c

	text: translate('workspace.common.delete'),
	onSelected: () => {
	setPolicyIDToDelete(item.policyID ?? '-1');
	setPolicyNameToDelete(item.title);
	setIsDeleteModalOpen(true);
	},

In old dot, we have a modal to show that this action is not allowed. Similarly in new dot, we will show similar modal if the request is from agent.

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

First we will check if the request is from support agent using the existing isSupportAuthToken check:

App/src/libs/Network/NetworkStore.ts

Lines 131 to 133 in 967cc5c

	function isSupportAuthToken(): boolean {
	return authTokenType === CONST.AUTH_TOKEN_TYPES.SUPPORT;
	}

In WorkspacesListPage:

    const isSupportalAction = isSupportAuthToken();

Then we will introduce a new state to show the modal of blocking and add hideSupportalModal funciton:

    const [isSupportalActionRestrictedModalOpen ,setIsSupportalActionRestrictedModalOpen] = useState(false);
        const hideSupportalModal = () => {
        setIsSupportalActionRestrictedModalOpen(false);
    };

Then below:

App/src/pages/workspace/WorkspacesListPage.tsx

Lines 170 to 175 in 967cc5c

	text: translate('workspace.common.delete'),
	onSelected: () => {
	setPolicyIDToDelete(item.policyID ?? '-1');
	setPolicyNameToDelete(item.title);
	setIsDeleteModalOpen(true);
	},

First we will check if the request is from agent and then show the blocking modal:

                    onSelected: () => {
                        if(isSupportalAction) {
                            isSupportalActionRestrictedModalOpen(true);
                            return;
                        }

                        setPolicyIDToDelete(item.policyID ?? '-1');
                        setPolicyNameToDelete(item.title);
                        setIsDeleteModalOpen(true);
                    },

Then add a modal to show that action is not allowed here:

            // Note that all these will be translated to their spanish equivalents as well and updated in es.ts file
            <ConfirmModal
                title={'Not so fast'}
                isVisible={isSupportalActionRestrictedModalOpen}
                onConfirm={hideSupportalModal}
                prompt={'You are not authorized to take this action when support logged in.'}
                confirmText={translate('common.buttonConfirm')}
                shouldShowCancelButton={false}
            />

Note that we can make a generic modal component for agent request blocking, we can also check all the places where we can delete the workspace and add this check there, also note that we can show this blocking view on the delete confirmation modal, that can be decided in the PR phase

What alternative solutions did you explore? (Optional)

[allgandalf]

Alright, @twilight2294 proposal makes sense to me, Their RCA is correct and solution would work in theory. Let's go with their solution.

Only caveat is that we would need to hardcode the value to true (While testing during PR phase) and then Internal QA it to test on either Ad-hoc (If possible) or directly onto staging.

The assigned internal engineer can help with that.

🎀👀🎀 C+ reviewed

[melvin-bot]

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

[melvin-bot]

📣 @allgandalf 🎉 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

[melvin-bot]

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

• [Internal QA]: Fix: Logged-In Agents Can Appear to Delete Workspaces in NewDot Without Actual Deletion or Error Message #53333

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

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

• @allgandalf requires payment automatic offer (Reviewer)
• @twilight2294 requires payment automatic offer (Contributor)

[melvin-bot]

@allgandalf @RachCHopkins @allgandalf The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]

[MitchExpensify]

BugZero Checklist:

• [Contributor] Classify the bug:

Bug classification

Source of bug:

• 1a. Result of the original design (eg. a case wasn't considered)
• 1b. Mistake during implementation
• 1c. Backend bug
• 1z. Other:

Where bug was reported:

• 2a. Reported on production (eg. bug slipped through the normal regression and PR testing process on staging)
• 2b. Reported on staging (eg. found during regression or PR testing)
• 2d. Reported on a PR
• 2z. Other:

Who reported the bug:

• 3a. Expensify user
• 3b. Expensify employee
• 3c. Contributor
• 3d. QA
• 3z. Other:

• [Contributor] 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:

• [Contributor] If the regression was CRITICAL (e.g. interrupts a core flow) A discussion in #expensify-open-source 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:

• [Contributor] If it was decided to create a regression test for the bug, please propose the regression test steps using the template below to ensure the same bug will not reach production again.

Regression Test Proposal Template

• [BugZero Assignee] Create a GH issue for creating/updating the regression test once above steps have been agreed upon.

  Link to issue:

Regression Test Proposal

Precondition:

Test:

Do we agree 👍 or 👎

[MitchExpensify]

cc on the BZ steps above @allgandalf

[allgandalf]

@MitchExpensify no regression test and checklist for this bug

• QA doesn't have access to supportal, so they would never be able to test

• We never considered implementing a blocking modal for supportal logged in users, so this is kind of a New Feature so no checklist should be applicable.

@Beamanator to confirm 🙇

[Beamanator]

Mmmmm true, I don't think supportal is available for applause so they can't really manually test that, right @MitchExpensify ?

[MitchExpensify]

That is true; I'm not sure how, or if, QA tests any Supportal centric bugs today. Maybe @mallenexpensify can help give us a bit of guidance on the BZ steps here 🙏

[RachCHopkins]

@Beamanator is it cool if I pay people even without this figured out?

[mallenexpensify]

Contributor: @twilight2294 paid $250 via Upwork
Contributor+: @allgandalf paid $250 via Upwork

Didn't wanna hold up payments over the weekend.

Let's leave this open through next week to see if we can figure out how we want to address Supportal issues (which... maybe we can just kick the can down the road til we see if we have many? I really don't know, kinda spitballing late on a Friday)

[Beamanator]

Thanks for paying out! Hmm yeah good call, maybe we can bring this up to the main people responsible for implementing supportal in NewDot recently?

[melvin-bot]

@Beamanator, @RachCHopkins, @allgandalf, @twilight2294 Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

[allgandalf]

patience melv!! not overdue

[melvin-bot]

@Beamanator, @RachCHopkins, @allgandalf, @twilight2294 Huh... This is 4 days overdue. Who can take care of this?

[RachCHopkins]

All paid out, waiting on QA steps.

[allgandalf]

No updates..... most probably we'll hear next year 😆

[melvin-bot]

@Beamanator, @RachCHopkins, @allgandalf, @twilight2294 Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@Beamanator, @RachCHopkins, @allgandalf, @twilight2294 Still overdue 6 days?! Let's take care of this!

[RachCHopkins]

Still waiting on QA steps.

[Beamanator]

Just to be clear, we're not waiting on me, right? 😅

[allgandalf]

[mallenexpensify]

I'm unsure of what QA steps would/should be for supportal related issues. Also unsure of who would/should come up with those. If we don't view the steps as a near-term priority, we can likely close this.... right?

[Beamanator]

Trueeee if QA doesn't actually test supportal stuff, it doesn't really make sense to need QA steps, does it? 🤣

[mallenexpensify]

Did a quick search in the #qa Slack room and didn't see much about Supportal, so I'm going to close for now. Comment/reopen if needed.