[HOLD for payment 2024-01-16][$500] Web - Workspace - Inconsistent tooltip for workspace name in thread and IOU

[izarutskaya]

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: v1.4.3-6
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:
Slack conversation: @

Action Performed:

1. Go to Settings > Workspaces > New Workspace.
2. Change the workspace name if you want
3. Go to the workspace expense room.
4. Send a message.
5. Go to 'Request money.'
6. Enter the amount and send it.
7. Go to the workspace setting and delete it.
8. Click on IOU and hover over the workspace name at the header.
9. Go back, hover over the message, and click on 'Reply In Thread'
10. Hover over the workspace name at the header.
11. Notice inconsistency between workplace name tooltips.

Expected Result:

The workspace name tooltip should be the same for thread header and IOU.

Actual Result:

Inconsistency in the thread workspace name tooltip and IOU workspace name tooltip

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

Bug6290552_1700938828937.2023-11-25_23-29-05.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~010c1c53f77916777c
• Upwork Job ID: 1728926172581572608
• Last Price Increase: 2023-11-26
• Automatic offers:
• cubuspl42 | Reviewer | 27883148

[melvin-bot]

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

[melvin-bot]

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

[melvin-bot]

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]

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

[dukenv0307]

Proposal

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

Inconsistency in the thread workspace name tooltip and IOU workspace name tooltip

What is the root cause of that problem?

When a thread is opened after the workspace has been archived, we're not setting the oldPolicyName of the thread to the name of the workspace. Meanwhile for existing chat/thread before the workspace is archived, it will have oldPolicyName set as in here.

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

When a thread is opened after the workspace has been archived, set the oldPolicyName to the name of the workspace prior to deletion. This will need to be fixed in both optimistic data and back-end.

We can do that by either:

• Use the same oldPolicyName of the main report where the threading happens
• Or make the policy data still available after deletion (with archived status or similar), and use that (I think we might not want this)

What alternative solutions did you explore? (Optional)

• Disable threading & similar actions after the workspace has been archived
• Or if we think it's ok to show Unavailable workspace even on existing threads, we can remove the oldPolicyName setting logic for those threads/chats

[chie2727]

Proposal

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

The tooltip for the workspace name in the thread and the IOU are inconsistent.

What is the root cause of that problem?

For the archived workspace, as HeaderView.js is being called, the report is generated but the parent-report is not. As the report does not include the oldPolicyName variable, we would need to obtain this from the parent-report, however the parent-report is not available.
Since the oldPolicyName has not been found, in ReportUtils.js, when calling the function getWorkspaceIcon() which in turn calls getPolicyName(), we end up with an “undefined” policy name which results in the workspace being named as “Unavailable workspace”.

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

In the ReportUtils.js file, we only need to change 2 lines in the getPolicyName() function to solve the issue.
Firstly, obtain the parent-report we need by passing the parentReportID variable in the report to the function getRootParentReport().
Secondly, from this newly obtained parent-report we can now obtain the oldPolicyName.
Now that the policy name has been obtained, the archived workspace will be named correctly.

expensify_31894_solution.mp4

What alternative solutions did you explore? (Optional)

[melvin-bot]

📣 @chie2727! 📣
Hey, it seems we don’t have your contributor details yet! You'll only have to do this once, and this is how we'll hire you on Upwork.
Please follow these steps:

1. Make sure you've read and understood the contributing guidelines.
2. Get the email address used to login to your Expensify account. If you don't already have an Expensify account, create one here. If you have multiple accounts (e.g. one for testing), please use your main account email.
3. Get the link to your Upwork profile. It's necessary because we only pay via Upwork. You can access it by logging in, and then clicking on your name. It'll look like this. If you don't already have an account, sign up for one here.
4. Copy the format below and paste it in a comment on this issue. Replace the placeholder text with your actual details.
   
   Format:

Contributor details
Your Expensify account email: <REPLACE EMAIL HERE>
Upwork Profile Link: <REPLACE LINK HERE>

[chie2727]

Contributor details
Your Expensify account email: [email protected]
Upwork Profile Link: https://www.upwork.com/freelancers/~01e5bcae9f03690137

[melvin-bot]

✅ Contributor details stored successfully. Thank you for contributing to Expensify!

[kaushiktd]

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

Web - Workspace - Inconsistent tooltip for workspace name in thread and IOU

What is the root cause of that problem?

The function getPolicyName() returns "Unavailable workspace policy" when the workspace name is not available. This is happening when we delete the workspace.

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

If the workspace name (report.policyName or report.oldPolicyName) is not available, the function will return an empty string or hidden. If the workspace name is available, it will display the workspace name accordingly.

or

When a thread is opened after the workspace has been archived, set the oldPolicyName to the name of the workspace before deletion. Ensure this change is reflected both optimistically (client-side) and on the backend.

[cubuspl42]

C+ reviewed 🎀 👀 🎀

I approve the proposal by @chie2727. In my opinion, obtaining the archived workspace name from the parent-report is a cleaner and more normalized approach in this scenario.

[melvin-bot]

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

[melvin-bot]

📣 @cubuspl42 🎉 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]

📣 @chie2727 You have been assigned to this job!
Please apply to the Upwork job and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Once you apply to this job, your Upwork ID will be stored and you will be automatically hired for future jobs!
Keep in mind: Code of Conduct | Contributing 📖

[chie2727]

Thank you very much.
I will submit a PR as soon as my proposal is accepted on Upwork.

[cubuspl42]

@chie2727 You don't have to wait for anything on Upwork. It's a separate timeline, and I guarantee that everyone is always paid.

[chie2727]

@cubuspl42 Thanks for letting me know. The PR is ready for review: #32325

[chie2727]

Hi @cubuspl42, will the PR be merged soon?

[cubuspl42]

@chie2727

😳

I don't know what happened here, I'll comment on the PR.

[dukenv0307]

@cubuspl42 sorry for getting back late but I don't think the selected proposal is correct in all cases and is future proof, especially in the case of paginated reports. In that solution we're making the thread report rely on the root parent report. When the user has a lot of reports and the reports are paginated, the "root parent report" might not be available in order for us to get it's oldPolicyName.

Also with that we have data inconsistency, the threads that are opened after the thread is archived will not have oldPolicyName, while the threads opened before has.

What do you think?

cc @nkuoch

[chie2727]

@dukenv0307
Interesting to hear your insight. If you have a look at my PR (https://github.com/Expensify/App/pull/32325/files#diff-577fcc5f3a5e4930916ff310e61c38edfa093792fe3ab55b9a30d2aecd8811dbR507) you can see that my proposal is not to make the thread report rely on the root parent report, but instead to add the root parent report to the search condition for finding the policy name. So in the case that the policy name is not present in the policy object or the report object, it can be obtained from the parent report object instead.

[dukenv0307]

So in the case that the policy name is not present in the policy object or the report object, it can be obtained from the parent report object instead.

@chie2727 What I mean is if the reports are paginated, there's no parent report object to get the policyName from (of course in case the policy name is not present in the policy object or the report object). We're assuming here that at least the root parent report will always be available which might not be the case.

cc @cubuspl42

[nkuoch]

Sorry I'm ooo this week. Unassigning myself.

[cubuspl42]

@dukenv0307 Thanks for sharing your findings.

It's unlikely that we change the assignment at this stage.

How many reports does one need to have to trigger paginating?

[dukenv0307]

How many reports does one need to have to trigger paginating?

@cubuspl42 I'm not sure on the exact number. But we shouldn't operate under the assumption that the root parent report will always be available IMO.

It's unlikely that we change the assignment at this stage.

@cubuspl42 I'm good too work together with @chie2727 on this one, if we decide my solution is more robust.

What do you think?

[cubuspl42]

What do you think?

I'll test this solution on my most report-heavy account (most likely heavier than any real customer). I'll see if I can observe any problems!

I'm good too work together

I don't think the Expensify process has room for such a step.

[dukenv0307]

I don't think the Expensify process has room for such a step.

@cubuspl42 It's normal for an issue to be assigned to 2 contributors and both share compensation, I can link some precedents for that if you're interested 😄

[cubuspl42]

It's definitely not "normal" or "common", but I'm not saying there was no precedence. Still, we're assuming that the solution being reviewed right can trigger a bug, although I havent't yet reproduced it or seen it reproduced (or maybe produced would be a more suitable term?).

[melvin-bot]

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

[amyevans]

Looks like #32325 was deployed to prod 1/9 and the automation just didn't trigger. I'll update the labels/title accordingly.

[MitchExpensify]

Thanks @amyevans, reminder set to pay on the 16th!

[MitchExpensify]

Paid and contract ended for @cubuspl42

Offer sent to @chie2727, will pay out once accepted! 🙇

[MitchExpensify]

Paid and contract ended! Thanks all