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] Unable to Act on Report Due to Deleted Workspace, Stuck with RBR in Focus Mode Without Edit Access #50573

Open
1 of 6 tasks
m-natarajan opened this issue Oct 10, 2024 · 25 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2

Comments

@m-natarajan
Copy link

m-natarajan commented Oct 10, 2024

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: 9.0.47-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:
Email or phone of affected tester (no customers):
Logs: https://stackoverflow.com/c/expensify/questions/4856
Expensify/Expensify Issue URL:
Issue reported by: @allgandalf
Slack conversation: https://expensify.slack.com/archives/C049HHMV9SM/p1728540959841099

Action Performed:

  1. Submit an expense with violation
  2. Select focus mode in preferences
  3. Delete the Workspace where the expenses was submitted
  4. Observe LHN

Expected Result:

Report in archived status

Actual Result:

See RBR in LHN with the report, but cannot act on it as don't have edit access anymore to that report, but also cannot do anything here, it stays in my LHN

Workaround:

Unknown

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

Screenshot 2024-10-10 at 11 45 49 AM

Recording.642.mp4

Onyx data:

{ "chatReportID": "4695747423350738", "chatType": "", "currency": "INR", "description": "", "errorFields": {}, "hasOutstandingChildRequest": false, "hasOutstandingChildTask": false, "hasParentAccess": true, "isCancelledIOU": false, "isOwnPolicyExpenseChat": false, "isPinned": false, "isWaitingOnBankAccount": false, "lastActionType": "CLOSED", "lastActorAccountID": "16559988", "lastMessageHtml": "closed this report", "lastMessageText": "closed this report", "lastReadSequenceNumber": 0, "lastReadTime": "2024-10-10 06:16:13.007", "lastVisibleActionCreated": "2024-04-08 08:54:26.839", "lastVisibleActionLastModified": "2024-04-08 08:54:26.839", "managerID": 0, "nonReimbursableTotal": 0, "notificationPreference": "hidden", "oldPolicyName": "Whogandalf's Workspace 27", "ownerAccountID": 16559988, "parentReportActionID": "6007246625411673721", "parentReportID": "4695747423350738", "participants": { "16559988": { "hidden": true, "notificationPreference": "hidden" } }, "permissions": [], "policyID": "3DEDDF79F054B00A", "private_isArchived": "2024-07-18 11:24:28.169", "reportID": "4817250283487348", "reportName": "Expense Report #5426340451518743", "stateNum": 2, "statusNum": 2, "total": 0, "type": "expense", "unheldTotal": 0, "welcomeMessage": "", "writeCapability": "all", "isLoadingPrivateNotes": false, "privateNotes": { "16559988": { "note": "" } } }

Add any screenshot/video evidence

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~021845769561131002585
  • Upwork Job ID: 1845769561131002585
  • Last Price Increase: 2024-10-21
  • Automatic offers:
    • allgandalf | Reviewer | 104576294
Issue OwnerCurrent Issue Owner: @
Issue OwnerCurrent Issue Owner: @allgandalf
@m-natarajan m-natarajan added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 10, 2024
Copy link

melvin-bot bot commented Oct 10, 2024

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

@allgandalf
Copy link
Contributor

Note

Can i be the C+ for this issue @sonialiap , I reported this bug and have prior context

@bernhardoj
Copy link
Contributor

bernhardoj commented Oct 10, 2024

Edited by proposal-police: This proposal was edited at 2024-10-17 14:42:26 UTC.

Proposal

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

The RBR still shows for deleted workspace chat and its expense report and thread.

What is the root cause of that problem?

There are 3 RBRs here. 1 in LHN,

function getAllReportActionsErrorsAndReportActionThatRequiresAttention(report: OnyxEntry<Report>, reportActions: OnyxEntry<ReportActions>): ReportErrorsAndReportActionThatRequiresAttention {
const reportActionsArray = Object.values(reportActions ?? {});
const reportActionErrors: OnyxCommon.ErrorFields = {};
let reportAction: OnyxEntry<ReportAction>;
for (const action of reportActionsArray) {
if (action && !isEmptyObject(action.errors)) {
Object.assign(reportActionErrors, action.errors);
if (!reportAction) {
reportAction = action;
}
}
}
const parentReportAction: OnyxEntry<ReportAction> =
!report?.parentReportID || !report?.parentReportActionID ? undefined : allReportActions?.[report.parentReportID ?? '-1']?.[report.parentReportActionID ?? '-1'];
if (ReportActionUtils.wasActionTakenByCurrentUser(parentReportAction) && ReportActionUtils.isTransactionThread(parentReportAction)) {
const transactionID = ReportActionUtils.isMoneyRequestAction(parentReportAction) ? ReportActionUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID : null;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !ReportUtils.isSettled(transaction?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = undefined;
}
} else if ((ReportUtils.isIOUReport(report) || ReportUtils.isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) {
if (ReportUtils.shouldShowRBRForMissingSmartscanFields(report?.reportID ?? '-1') && !ReportUtils.isSettled(report?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = ReportUtils.getReportActionWithMissingSmartscanFields(report?.reportID ?? '-1');
}
} else if (ReportUtils.hasSmartscanError(reportActionsArray)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = ReportUtils.getReportActionWithSmartscanError(reportActionsArray);
}
return {
errors: reportActionErrors,
reportAction,
};
}

1 in report preview,

const hasErrors =
(hasMissingSmartscanFields && !iouSettled) ||
// eslint-disable-next-line @typescript-eslint/prefer-nullish-coalescing
ReportUtils.hasViolations(iouReportID, transactionViolations) ||
ReportUtils.hasWarningTypeViolations(iouReportID, transactionViolations) ||
(ReportUtils.isReportOwner(iouReport) && ReportUtils.hasReportViolations(iouReportID)) ||
ReportUtils.hasActionsWithErrors(iouReportID);

and 1 in the expense/transaction preview.

const shouldShowRBR = hasNoticeTypeViolations || hasViolations || hasFieldErrors || (!isFullySettled && !isFullyApproved && isOnHold) || hasDuplicates;

None of the conditions check for the report archive status. The expense report will stay on the LHN because we always show it if there are errors other than failed receipt.

const hasErrorsOtherThanFailedReceipt =
doesTransactionThreadReportHasViolations ||
doesReportHaveViolations ||
Object.values(allReportErrors).some((error) => error?.[0] !== Localize.translateLocal('iou.error.genericSmartscanFailureMessage'));
const isReportInAccessible = report?.errorFields?.notFound;
if (ReportUtils.isOneTransactionThread(report.reportID, report.parentReportID ?? '0', parentReportAction)) {
return;
}
if (hasErrorsOtherThanFailedReceipt && !isReportInAccessible) {
reportsToDisplay.push({
...report,
hasErrorsOtherThanFailedReceipt: true,
});
return;
}

Why do the workspace chat and expense/transaction thread not show even if they have the RBR? The reportActions in `SidebarUtils.getOrderedReportIDs is coming from

const [allReportActions] = useOnyx(ONYXKEYS.COLLECTION.REPORT_ACTIONS, {selector: (c) => mapOnyxCollectionItems(c, reportActionsSelector)});

and somehow the selector that was recently added in #49918 returns a wrong array of actions. For the transaction thread, the parent report action here is undefined,

const parentReportAction: OnyxEntry<ReportAction> =
!report?.parentReportID || !report?.parentReportActionID ? undefined : allReportActions?.[report.parentReportID ?? '-1']?.[report.parentReportActionID ?? '-1'];

so, isTransactionThread is always false. It's undefined because we access the allReportActions object without appending the reportActions_ key prefix.

if (ReportActionUtils.wasActionTakenByCurrentUser(parentReportAction) && ReportActionUtils.isTransactionThread(parentReportAction)) {

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

We need to check for the report archive status when getting all report errors.

App/src/libs/ReportUtils.ts

Lines 6303 to 6318 in 20b7017

if (ReportActionsUtils.wasActionTakenByCurrentUser(parentReportAction) && ReportActionsUtils.isTransactionThread(parentReportAction)) {
const transactionID = ReportActionsUtils.isMoneyRequestAction(parentReportAction) ? ReportActionsUtils.getOriginalMessage(parentReportAction)?.IOUTransactionID : null;
const transaction = allTransactions?.[`${ONYXKEYS.COLLECTION.TRANSACTION}${transactionID}`];
if (TransactionUtils.hasMissingSmartscanFields(transaction ?? null) && !isSettled(transaction?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = undefined;
}
} else if ((isIOUReport(report) || isExpenseReport(report)) && report?.ownerAccountID === currentUserAccountID) {
if (shouldShowRBRForMissingSmartscanFields(report?.reportID ?? '-1') && !isSettled(report?.reportID)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = getReportActionWithMissingSmartscanFields(report?.reportID ?? '-1');
}
} else if (hasSmartscanError(reportActionsArray)) {
reportActionErrors.smartscan = ErrorUtils.getMicroSecondOnyxErrorWithTranslationKey('iou.error.genericSmartscanFailureMessage');
reportAction = getReportActionWithSmartscanError(reportActionsArray);
}

if (!ReportUtils.isArchivedRoom(report)) {
    ...
}

@allgandalf
Copy link
Contributor

Note

Can i be the C+ for this issue @sonialiap , I reported this bug and have prior context

@melvin-bot melvin-bot bot added the Overdue label Oct 14, 2024
@melvin-bot melvin-bot bot removed the Overdue label Oct 14, 2024
@sonialiap sonialiap added External Added to denote the issue can be worked on by a contributor Overdue labels Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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

@melvin-bot melvin-bot bot changed the title Unable to Act on Report Due to Deleted Workspace, Stuck with RBR in Focus Mode Without Edit Access [$250] Unable to Act on Report Due to Deleted Workspace, Stuck with RBR in Focus Mode Without Edit Access Oct 14, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 14, 2024
Copy link

melvin-bot bot commented Oct 14, 2024

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

@sonialiap
Copy link
Contributor

@allgandalf added you as C+ 👍 thanks for volunteering :)

@melvin-bot melvin-bot bot removed the Overdue label Oct 14, 2024
@allgandalf
Copy link
Contributor

Thanks @sonialiap , appreciated 😄

@allgandalf
Copy link
Contributor

@sonialiap just to confirm, with @bernhardoj proposal above:

  • We will never show a RBR in LHN for archived workspace reports (The report would still have violation technically)? are we onboard with that?

@bernhardoj if you want to add something more the ^ ?

@sonialiap
Copy link
Contributor

@allgandalf extra noise in the LHN is a bane of my existence so my vote is to remove archived reports from LHN when in #focus mode even if there's a violation since the violation can't be actioned 😂 But let me check with the team if there's any preference on whether to show them until read, immediately archive them when the workspace is closed, or do something else

@melvin-bot melvin-bot bot added the Overdue label Oct 16, 2024
@allgandalf
Copy link
Contributor

@sonialiap any update ? If a discussion is ongoing in #expense can you please tag me there :)

@melvin-bot melvin-bot bot removed the Overdue label Oct 17, 2024
@sonialiap
Copy link
Contributor

@allgandalf sorry I forgot to share the results 🙈 thanks for the bump!

The conclusion (in #product) was that if it can't be actioned it shouldn't have RBR. Fine to show violation in the actual expense, but not in the LHN

@allgandalf
Copy link
Contributor

haha, no worries, thanks for the update. @bernhardoj do you want to update your proposal accordingly ? let me know then i will start the review

@bernhardoj
Copy link
Contributor

My proposal actually already have that case, but I updated to remove the others.

@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@allgandalf
Copy link
Contributor

Thanks, I will review the proposal today

@melvin-bot melvin-bot bot removed the Overdue label Oct 21, 2024
@allgandalf
Copy link
Contributor

@bernhardoj proposal here LGTM.

Their RCA is correct and solution would work.

Note to internal engineer: Please see the new expected result here

@allgandalf
Copy link
Contributor

There should be a shortcut for the 🎀👀🎀 C+ reviewed tag i tell you 😢 , I forget this often

Copy link

melvin-bot bot commented Oct 21, 2024

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

Copy link

melvin-bot bot commented Oct 21, 2024

📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸

@melvin-bot melvin-bot bot added the Overdue label Oct 23, 2024
@sonialiap
Copy link
Contributor

@AndrewGable ready for your review! #50573 (comment)

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

melvin-bot bot commented Oct 24, 2024

📣 @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 melvin-bot bot removed the Overdue label Oct 24, 2024
Copy link

melvin-bot bot commented Oct 24, 2024

@AndrewGable @sonialiap @bernhardoj @allgandalf this issue was created 2 weeks ago. Are we close to approving a proposal? If not, what's blocking us from getting this issue assigned? Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

@bernhardoj
Copy link
Contributor

I just re-tested and the expense report now isn't archived when deleting the workspace chat.
image

  • workspace chat (archived, RBR)
  • expense report (not archived, RBR)

The conclusion (in #product) was that if it can't be actioned it shouldn't have RBR. Fine to show violation in the actual expense, but not in the LHN

Based on this, then it's fine to show the RBR for the expense report, but should we still show the RBR for the archived workspace chat?

@melvin-bot melvin-bot bot added the Overdue label Oct 28, 2024
@sonialiap
Copy link
Contributor

@bernhardoj let's not show the RBR for the archived workspace chat. The chat is closed for new comments so there's no way for the user to clear the RBR

@melvin-bot melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 Overdue labels Oct 28, 2024
@bernhardoj
Copy link
Contributor

Thanks for the confirmation!

PR is ready

cc: @allgandalf

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. External Added to denote the issue can be worked on by a contributor Reviewing Has a PR in review Weekly KSv2
Projects
Status: Release 3: Fall 2024 (Nov)
Development

No branches or pull requests

5 participants