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

[$500] [QAB] Shortcut - Request money shortcut for workspace remains after workspace is deleted #39051

Closed
6 tasks done
izarutskaya opened this issue Mar 27, 2024 · 30 comments
Closed
6 tasks done
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Engineering retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2

Comments

@izarutskaya
Copy link

izarutskaya commented Mar 27, 2024

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.57-0
Reproducible in staging?: Y
Reproducible in production?: N
Logs: https://stackoverflow.com/c/expensify/questions/4856
Issue reported by: Applause-Internal team

Action Performed:

  1. Go to staging.new.expensify.com.
  2. Go to workspace chat.
  3. Request money from the workspace chat.
  4. Delete the workspace.
  5. Click FAB.
  6. Click Request money shortcut.

Expected Result:

After deleting the workspace, Request money shortcut should lead user to select a new participant.

Actual Result:

After deleting the workspace, Request money shortcut for the deleted workspace still remains. Clicking on the shortcut results in not here page.

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

Bug6428615_1711540647403.shortcut_delete.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~016af2ad690a3296e0
  • Upwork Job ID: 1772979000228327424
  • Last Price Increase: 2024-03-28
@izarutskaya izarutskaya added DeployBlockerCash This issue or pull request should block deployment Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Mar 27, 2024
Copy link

melvin-bot bot commented Mar 27, 2024

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

@github-actions github-actions bot added Engineering Hourly KSv2 and removed Daily KSv2 labels Mar 27, 2024
Copy link
Contributor

👋 Friendly reminder that deploy blockers are time-sensitive ⏱ issues! Check out the open `StagingDeployCash` deploy checklist to see the list of PRs included in this release, then work quickly to do one of the following:

  1. Identify the pull request that introduced this issue and revert it.
  2. Find someone who can quickly fix the issue.
  3. Fix the issue yourself.

Copy link

melvin-bot bot commented Mar 27, 2024

Triggered auto assignment to @lakchote (Engineering), see https://stackoverflowteams.com/c/expensify/questions/9980/ for more details.

@izarutskaya
Copy link
Author

@mallenexpensify 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.

@izarutskaya
Copy link
Author

We think that this bug might be related to #vip-vsb.

@allgandalf
Copy link
Contributor

allgandalf commented Mar 27, 2024

Proposal

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

Request money shortcut for workspace remains after workspace is deleted

What is the root cause of that problem?

We don't check if workspace is archived before display the shortcut:

...(props.quickAction && props.quickAction.action
? [
{
icon: getQuickActionIcon(props.quickAction.action),
text: translate(getQuickActionTitle(props.quickAction.action)),
label: translate('quickAction.shortcut'),

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

Add isArchivedRoom check

App/src/libs/ReportUtils.ts

Lines 1106 to 1108 in cd18bef

function isArchivedRoom(report: OnyxEntry<Report> | EmptyObject): boolean {
return report?.statusNum === CONST.REPORT.STATUS_NUM.CLOSED && report?.stateNum === CONST.REPORT.STATE_NUM.APPROVED;
}

What alternative solutions did you explore? (Optional)

N/A

@lakchote lakchote added the Internal Requires API changes or must be handled by Expensify staff label Mar 27, 2024
Copy link

melvin-bot bot commented Mar 27, 2024

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

Copy link

melvin-bot bot commented Mar 27, 2024

Triggered auto assignment to Contributor Plus for review of internal employee PR - @eVoloshchak (Internal)

@lakchote
Copy link
Contributor

Related to #38669

cc @Gonals @parasharrajat

@parasharrajat
Copy link
Member

Not a regression from the PR but a new case to be handled. That PR was only about adding the UI for quick action.

@habasefa
Copy link

Proposal

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

Shortcut not being updated when a workspace related to the shortcut item is deleted

What is the root cause of that problem?

When a workspace is deleted, the shortcut(QuickAction) is not cleared.

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

When a workspace is deleted checking if it is in quick action, if it is resetting quick action
Adding a code like

  let policyInQuickAction: string | undefined;
    Onyx.connect(
       { key: ONYXKEYS.NVP_QUICK_ACTION_GLOBAL_CREATE,
        callback: (value)=> {
            policyInQuickAction = ReportUtils.getReportPolicyID(value?.chatReportID);
        },}
    )
    // Reset the quickAction policy
    if (policyID === policyInQuickAction){
        resetQuickAction();
    }

around here

return Object.values(policies ?? {}).some((policy) => policy && policy.type === CONST.POLICY.TYPE.FREE && policy.role === CONST.POLICY.ROLE.ADMIN);
}
/**

and a code like this to clear the current quick action

/**
 * Reset quick action
 */
function resetQuickAction(){
    Onyx.set(ONYXKEYS.NVP_QUICK_ACTION_GLOBAL_CREATE, null);
}

What alternative solutions did you explore? (Optional)

@lakchote lakchote added External Added to denote the issue can be worked on by a contributor and removed DeployBlockerCash This issue or pull request should block deployment Hourly KSv2 Internal Requires API changes or must be handled by Expensify staff labels Mar 28, 2024
@melvin-bot melvin-bot bot changed the title Shortcut - Request money shortcut for workspace remains after workspace is deleted [$500] Shortcut - Request money shortcut for workspace remains after workspace is deleted Mar 28, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Mar 28, 2024
Copy link

melvin-bot bot commented Mar 28, 2024

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

@parasharrajat
Copy link
Member

So Technically both proposals are a way forward.

  1. We can either control showing the quick action when it is vague.
  2. Or we can clear the quick action when the related report is archived.

Let me get my hands on design doc to understand the better approach.

@nkdengineer
Copy link
Contributor

nkdengineer commented Mar 28, 2024

Proposal

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

After deleting the workspace, Request money shortcut for the deleted workspace still remains. Clicking on the shortcut results in not here page.

What is the root cause of that problem?

We don't clear or hide the quick action when the report is archived or we cannot access to this report anymore

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

  1. In FloatingActionButtonAndPopover, we should subscribe the report of quickAction with id is quickAction.chatReportID
quickActionReport: {
    key: ({quickAction}) => `${ONYXKEYS.COLLECTION.REPORT}${lodashGet(quickAction, 'chatReportID', '0')}`
}
  1. Add a useEffect that will clear the quickAction if the report is archived or it become undefined or we cannot access to this report anymore, we will clear the quickAction. That can cover all cases like we remove the user from workspace or we delete the workspace on another device, ....
useEffect(() => {
    if (!props.quickAction || !props.quickAction.chatReportID) {
        return;
    }

    if (!props.quickActionReport || ReportUtils.isArchivedRoom(props.quickActionReport)) {
        ReportActionUtils.clearQuickAction();
    }
}, [props.quickAction, props.quickActionReport])
  1. Create a util function to clear the quickAction
function clearQuickAction() {
    Onyx.merge(ONYXKEYS.NVP_QUICK_ACTION_GLOBAL_CREATE, null);
}

What alternative solutions did you explore? (Optional)

For point 2, we also can create a state to control the quickAction should be displayed or not and update this accordingly in the useEffect that is mentioned at point 2.

@parasharrajat
Copy link
Member

parasharrajat commented Mar 28, 2024

@mallenexpensify
Copy link
Contributor

@parasharrajat can you share the Slack link plz?
I'm adding to #wave-collect cuz it's the earliest project that someone would encounter this bug.

@parasharrajat
Copy link
Member

Added.

@parasharrajat
Copy link
Member

parasharrajat commented Mar 29, 2024

Looks like the expected behaviour of this issue was incorrect which is updated. But we are also working on a follow-up issue/PR which should handle this issue as well. It is better to be held at #38051.

@mallenexpensify Could you please hold this issue on #38051

@mallenexpensify mallenexpensify changed the title [$500] Shortcut - Request money shortcut for workspace remains after workspace is deleted [HOLD #38051][$500] Shortcut - Request money shortcut for workspace remains after workspace is deleted Mar 29, 2024
@mallenexpensify mallenexpensify removed 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 Mar 29, 2024
@mallenexpensify
Copy link
Contributor

Thanks @parasharrajat , put on hold, removed Help Wanted and External

@mallenexpensify mallenexpensify added Weekly KSv2 and removed Daily KSv2 labels Mar 29, 2024
@trjExpensify trjExpensify changed the title [HOLD #38051][$500] Shortcut - Request money shortcut for workspace remains after workspace is deleted [HOLD #38051][$500] [QAB] Shortcut - Request money shortcut for workspace remains after workspace is deleted Apr 4, 2024
@trjExpensify
Copy link
Contributor

Did we confirm with @Gonals that #38051 will take care of it?

I'm adding to #wave-collect cuz it's the earliest project that someone would encounter this bug.

@mallenexpensify more to the point, it's related to the Quick Action Button (QAB) project.

@parasharrajat
Copy link
Member

@trjExpensify
Copy link
Contributor

Dope!

@melvin-bot melvin-bot bot added the Overdue label Apr 12, 2024
@lakchote
Copy link
Contributor

PR got approved, waiting for another reviewer's approval to be merged.

@melvin-bot melvin-bot bot removed the Overdue label Apr 15, 2024
@melvin-bot melvin-bot bot added the Overdue label Apr 23, 2024
@mallenexpensify
Copy link
Contributor

PR is on main
#39413

@melvin-bot melvin-bot bot removed the Overdue label Apr 23, 2024
@parasharrajat
Copy link
Member

@Expensify/applause Can we please retest it?

@mallenexpensify mallenexpensify added the retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause label Apr 26, 2024
@mallenexpensify mallenexpensify changed the title [HOLD #38051][$500] [QAB] Shortcut - Request money shortcut for workspace remains after workspace is deleted [$500] [QAB] Shortcut - Request money shortcut for workspace remains after workspace is deleted Apr 26, 2024
@mallenexpensify
Copy link
Contributor

Taking off hold, added retest-weekly label.

@trjExpensify
Copy link
Contributor

I think this is probably the same as: #41467

We have a PR up for that, so this can be closed. @Gonals can confirm though!

@Gonals
Copy link
Contributor

Gonals commented May 3, 2024

Yep! This should be handled by #41474

@Gonals Gonals closed this as completed May 3, 2024
@trjExpensify
Copy link
Contributor

Excellllent. Another one bites the dust!

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. Engineering retest-weekly Apply this label if you want this issue tested on a Weekly basis by Applause Weekly KSv2
Projects
No open projects
Archived in project
Development

No branches or pull requests

10 participants