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

[Backwards compatibility] [$250] LHN - GRB not removed from workspace when moving report to another workspace in OD #46287

Open
2 of 6 tasks
lanitochka17 opened this issue Jul 26, 2024 · 43 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Weekly KSv2

Comments

@lanitochka17
Copy link

lanitochka17 commented Jul 26, 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: 9.0.13.0
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 #45970

Action Performed:

  1. Go https://staging.new.expensify.com/
    and log in with an account that has several workspace
  2. Navigate to the workspace chat
  3. Submit an expense
  4. Go to OD and log in with the same account
  5. Open the submitted report and move it to a different workspace
  6. Navigate back to ND

Expected Result:

The GRB is removed from the workspace since the report was moved to another workspace

Actual Result:

The GRB is still displayed and workspace preview in LHN shows " owes " until the workspace chat is opened

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

Add any screenshot/video evidence

Bug6552887_1721926476541.Recording__640.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~01b2e9e01981038ce0
  • Upwork Job ID: 1818045384678794129
  • Last Price Increase: 2024-08-12
Issue OwnerCurrent Issue Owner: @mollfpr
@lanitochka17 lanitochka17 added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 26, 2024
Copy link

melvin-bot bot commented Jul 26, 2024

Triggered auto assignment to @johncschuster (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
Copy link
Author

@johncschuster 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

@melvin-bot melvin-bot bot added the Overdue label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 29, 2024

@johncschuster Whoops! This issue is 2 days overdue. Let's get this updated quick!

@johncschuster johncschuster added the External Added to denote the issue can be worked on by a contributor label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 29, 2024

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

@melvin-bot melvin-bot bot changed the title LHN - GRB not removed from workspace when moving report to another workspace in OD [$250] LHN - GRB not removed from workspace when moving report to another workspace in OD Jul 29, 2024
@melvin-bot melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Jul 29, 2024
Copy link

melvin-bot bot commented Jul 29, 2024

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

@melvin-bot melvin-bot bot removed the Overdue label Jul 29, 2024
@Amoralchik
Copy link
Contributor

Amoralchik commented Jul 30, 2024

Edited by proposal-police: This proposal was edited at 2024-08-11 21:57:55 UTC.

Proposal

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

GRB and Subtitle do not change after a move expense to another workspace

What is the root cause of that problem?

So, has the subtitle not changed because the "report?.lastMessageText" in this code still contains old data like"[policyName] owes [currency]1.00"

return lastMessageTextFromReport || (report?.lastMessageText ?? '');

As a result, we set "lastMessageText" to "result.alternateText" here.
result.alternateText = lastMessageTextFromReport.length > 0 ? lastMessageText : ReportActionsUtils.getLastVisibleMessage(report.reportID, {}, lastAction)?.lastMessageText;

GBR we set here:

result.hasOutstandingChildRequest = report.hasOutstandingChildRequest;

We retrieve it from the backend server. The only way to fully update is to click the "new message" button or reopen the report, which triggers an action called OpenReport.

#46287 (comment)

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

To accomplish this, we'll need to create two functions: allExpenseActionsDeleted and someExpenseActionsDeleted.

allExpenseActionsDeleted will check if all ReportAction items with the type Expense have been deleted.
someExpenseActionsDeleted will check if there are any undeleted ReportAction items with the type Expense in ReportActions.

function allExpenseActionsDeleted(reportActions: OnyxEntry<ReportActions>): boolean {
    return Object.entries(reportActions ?? {})
        .filter(([, action]) => {
            return action.childType === CONST.REPORT.TYPE.EXPENSE;
        })
        .every(([, action]) => ReportActionsUtils.isDeletedAction(action) || action.childStatusNum === CONST.REPORT.STATUS_NUM.REIMBURSED);
}


function someExpenseActionsDeleted(reportActions: OnyxEntry<ReportActions>): boolean {
    return Object.entries(reportActions ?? {})
        .filter(([, action]) => {
            return action.childType === CONST.REPORT.TYPE.EXPENSE;
        })
        .some(([, action]) => ReportActionsUtils.isDeletedAction(action)  || action.childStatusNum === CONST.REPORT.STATUS_NUM.REIMBURSED);
}

If all of them have been deleted, we'll set result.hasOutstandingChildRequest to false and also fetch the last visible message in the report to update result.alternateText.
In cases where there are still undeleted ReportAction items with the type Expense, we'll set result.hasOutstandingChildRequest to true.
For it in the getOptionData function, add:

if (ReportUtils.allExpenseActionsDeleted(reportActions)) {
    lastMessageTextFromReport = ReportActionsUtils.getLastVisibleMessage(report.reportID, {}, lastAction)?.lastMessageText;
    result.hasOutstandingChildRequest = false;
} else if (ReportUtils.someExpenseActionsDeleted(reportActions)) {
    result.hasOutstandingChildRequest = true;
}

Also, we need to change lastMessageTextFromReport from const to let

Proof of concept:

2024-08-12.02-17-45.mp4

@melvin-bot melvin-bot bot added the Overdue label Aug 1, 2024
@johncschuster
Copy link
Contributor

@thesahindia can you take a look at the above proposal?

@thesahindia
Copy link
Member

thesahindia commented Aug 2, 2024

Could you share a video? @Amoralchik, the proposal didn't fix it for me.

@melvin-bot melvin-bot bot removed the Overdue label Aug 2, 2024
@Amoralchik
Copy link
Contributor

Amoralchik commented Aug 2, 2024

@thesahindia
Apologies if I may have overlooked something. Please notify me if there is anything I missed. Thank you.
Oh I see, sorry completely forgot about GRB

Copy link

melvin-bot bot commented Aug 5, 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 Aug 5, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

@johncschuster, @thesahindia Uh oh! This issue is overdue by 2 days. Don't forget to update your issues!

@johncschuster
Copy link
Contributor

@thesahindia bump on the comment above

@melvin-bot melvin-bot bot removed the Overdue label Aug 5, 2024
@Amoralchik
Copy link
Contributor

@thesahindia
Hi,

I conducted extensive testing and attempts to find a way to fix the GBR issue. I noticed that hasOutstandingChildRequest, which causes GBR, is obtained solely from the report already in ONYX. When an update is triggered by moving an expense to another workspace, we do not receive hasOutstandingChildRequest to change it from true to false. Meanwhile, the last report action is updated, but it still lacks sufficient data to accurately change hasOutstandingChildRequest. The only effective solution I found was to add ReportActions.openReport(fullReport?.reportID ?? '-1'); in OptionRowLHNData. This way, we could retrieve the report with the updated hasOutstandingChildRequest, but it would generate many requests and potentially lead to performance degradation.

Regarding the lastMessageText, as I suggested in the proposal, we can try to fix it.

@melvin-bot melvin-bot bot added the Overdue label Aug 8, 2024
@johncschuster
Copy link
Contributor

Bumped in Slack

@melvin-bot melvin-bot bot removed the Overdue label Aug 8, 2024
@trjExpensify
Copy link
Contributor

@puneetlath @greg-schroeder @JmillsExpensify here's another interesting one for backwards compatibility. Sounds like we need to make sure the GBR moves accordingly as well.

@johncschuster
Copy link
Contributor

johncschuster commented Aug 29, 2024

I'm sorry for the noise, @mollfpr. I accidentally hotkeyed the label. I... didn't realize I could call the label menu with keystrokes.

@johncschuster johncschuster removed the External Added to denote the issue can be worked on by a contributor label Aug 29, 2024
@johncschuster
Copy link
Contributor

Discussed in Slack – this is currently a lower priority issue. I'm going to bump this to weekly and keep checking in on the urgency each week.

@johncschuster johncschuster added Weekly KSv2 and removed Daily KSv2 labels Aug 29, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@johncschuster
Copy link
Contributor

Bumped. Still a lesser priority

@melvin-bot melvin-bot bot removed the Overdue label Sep 9, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 18, 2024
@johncschuster
Copy link
Contributor

Bumped in Slack to see if the priority has changed.

@melvin-bot melvin-bot bot removed the Overdue label Sep 19, 2024
@Amoralchik
Copy link
Contributor

@johncschuster We can use my Propoasal as a workaround for this issues

@melvin-bot melvin-bot bot added the Overdue label Sep 30, 2024
@melvin-bot melvin-bot bot removed Help Wanted Apply this label when an issue is open to proposals by contributors Overdue labels Oct 1, 2024
@johncschuster
Copy link
Contributor

@Amoralchik, I'm happy to assign to you, but please wait until I've had an internal engineer verify that your proposal is the path forward.

@johncschuster
Copy link
Contributor

Actually, I'm reviewing the issue with fresh eyes, and I don't think we want to apply a workaround for this one. Both @thesahindia and @shubham1206agra suggested against the proposed solution, and suggested we move this Internal to avoid performance degradations and Pusher needs to make necessary changes.

@Amoralchik
Copy link
Contributor

@johncschuster
About the last message
you are right, an issue has already been Internal since Aug 16
About the proposed solution, performance degradations have been in #46287 (comment) after that, I started thinking of better solutions, and now I think we need to move GBR logic from the server to the client-side.

@melvin-bot melvin-bot bot added the Overdue label Oct 10, 2024
@johncschuster
Copy link
Contributor

Bumping in Slack to get a sense check of the priority

@melvin-bot melvin-bot bot removed the Overdue label Oct 11, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 21, 2024
@johncschuster
Copy link
Contributor

Still low priority

@melvin-bot melvin-bot bot removed the Overdue label Oct 22, 2024
@melvin-bot melvin-bot bot added the Overdue label Oct 30, 2024
@johncschuster
Copy link
Contributor

No update. Keeping this LOW priority.

@melvin-bot melvin-bot bot removed the Overdue label Oct 30, 2024
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. Hot Pick Ready for an engineer to pick up and run with Internal Requires API changes or must be handled by Expensify staff Weekly KSv2
Projects
None yet
Development

No branches or pull requests

9 participants