[HOLD #431875][$500] Announce - Deleted announce room is not removed from LHN for employee

[lanitochka17]

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.29-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: Email or phone of affected tester (no customers): Logs: https://stackoverflow.com/c/expensify/questions/4856 Expensify/Expensify Issue URL: Issue reported by: Applause - Internal Team Slack conversation:

Action Performed:

1. With an admin account A, create workspace > invite user B to workspace > send message in announce room
2. Verify user B sees the announce room in LHN
3. From admin account A, delete the workspace

Expected Result:

Announce room should be removed from LHN of user B

Actual Result:

Announce room is removed when user B opens the room, and navigates to another chat

Workaround:

Unknown

Platforms:

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

• [ ] Android: Native
• [ ] Android: mWeb Chrome
• [ ] iOS: Native
• [ ] iOS: mWeb Safari
• [x] MacOS: Chrome / Safari
• [ ] MacOS: Desktop

Screenshots/Videos

Add any screenshot/video evidence

https://github.com/Expensify/App/assets/78819774/173c8244-c5c6-4d2e-af09-4b15af985ba5

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~0197a54c54a4f9ab7b
• Upwork Job ID: 1749457451398172672
• Last Price Increase: 2024-01-29
• Automatic offers:
• aimane-chnaif | Contributor | 0

Issue Owner

Current Issue Owner: @srikarparsi

[melvin-bot[bot]]

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

[melvin-bot[bot]]

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

[cheng-kang]

Proposal

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

Deleted announce room is not removed from LHN for employee

What is the root cause of that problem?

The root cause is that when a policy is deleted and User B receives update from WS, the handler doesn't remove the report from Oynx storage.

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

Update /src/libs/actions/Policy.ts#L76 to update the #announce report value to null.

❗ On a side note, wanted to confirm with the team whether we want to remove all related reports when a policy is deleted, as opposed to only remove the #announce report.

https://github.com/Expensify/App/blob/main/src/libs/actions/Policy.ts#L76

What alternative solutions did you explore? (Optional)

Alternatively, we could send an update from server to update the related reports to null. But it depends on how the backend logic really is. If after policy deletion, backend will not send anything about the related reports to frontend again, then doing it on frontend makes more sense.

[melvin-bot[bot]]

📣 @cheng-kang! 📣 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>

[cheng-kang]

Contributor details Your Expensify account email: cantchengkang@gmail.com Upwork Profile Link: https://www.upwork.com/freelancers/~017f672cbef45659ec

[melvin-bot[bot]]

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

[Santhosh-Sellavel]

Can we a BZ here cc: @Expensify/bug-zero

[Santhosh-Sellavel]

@cheng-kang

Update /src/libs/actions/Policy.ts#L76 to update the #announce report value to null.

Why do we want to do this? How we would do this?

Can you explain how this works, thanks!

[cheng-kang]

@Santhosh-Sellavel

Update /src/libs/actions/Policy.ts#L76 to update the #announce report value to null.

Why do we want to do this? How we would do this?

Can you explain how this works, thanks!

The announce room stays there because the related report still exists in Onyx cache. To remove it from the list, we should remove it from the Onyx cache after workspace deletion.

[melvin-bot[bot]]

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

[Santhosh-Sellavel]

@Santhosh-Sellavel

Update /src/libs/actions/Policy.ts#L76 to update the #announce report value to null.

Why do we want to do this? How we would do this? Can you explain how this works, thanks!

The announce room stays there because the related report still exists in Onyx cache. To remove it from the list, we should remove it from the Onyx cache after workspace deletion.

@cheng-kang How we would do this?

[cheng-kang]

@Santhosh-Sellavel by updating /src/libs/actions/Policy.ts#L76 to set the #announce report value to null.

If we are only interested in the announce channel, we will add a check there to only set the value for the specific report to be null. Otherwise, if we want to remove all related channels, we can just make change to the line referenced and make all reports to null.

[Santhosh-Sellavel]

@cheng-kang

@Santhosh-Sellavel by updating /src/libs/actions/Policy.ts#L76 to set the #announce report value to null.

How would set announce to null there, can you share the snippet?

[Santhosh-Sellavel]

@adelekennedy
Can you check internally & answer this?

On a side note, wanted to confirm with the team whether we want to remove all related reports when a policy is deleted, as opposed to only remove the #announce report.

[cheng-kang]

@Santhosh-Sellavel would be a simple one line change https://github.com/Expensify/App/blob/main/src/libs/actions/Policy.ts#L76

-    cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = {hasDraft: false};
+    cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = null;

cleanUpMergeQueries will be merged into Onyx cache and remove the report, UI that subscribes to reports will be updated to the new list without the reports from deleted workspace

[Santhosh-Sellavel]

I get this would make all workspace reports go from LHN

[adelekennedy]

Asked!

[cheng-kang]

@Santhosh-Sellavel for announce channels only we can check if type === "chat" && chatType === "policyAnnounce"

[adelekennedy]

@Santhosh-Sellavel

Yep, as it stands, #announce should be unshared with all members apart from the workspace admins at the time of deletion

[Santhosh-Sellavel]

Current Status: We need a better proposal here.

I'm unavailable next week, Please assign a new C+ Issue here if required while I am away, thanks!

cc: @adelekennedy

[cheng-kang]

@Santhosh-Sellavel hi do you mind provide more details on what's expected in a proposal? Or what's the issue with the current proposal? Is it not pointing to the right root cause? Or is it not including enough details? Would love to understand it better so I can better contribute. Thanks.

[Santhosh-Sellavel]

@cheng-kang Sorry I might be confused with something else,

Your proposal might be good here, but its bits & part please regroup and submit an updated proposal for hiding only announce report in a new comment with complete solution.

[cheng-kang]

Proposal

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

Deleted announce room is not removed from LHN for employee

What is the root cause of that problem?

The root cause is that when a policy is deleted and User B receives update from WS, the handler doesn't remove the report from Oynx storage.

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

The announce room stays there because the related report still exists in Onyx cache. To remove it from the list, we should remove it from the Onyx cache after workspace deletion, by updating /src/libs/actions/Policy.ts#L76 to update the #announce report value to null.

To remove only the announce channel, we should check if type === "chat" && chatType === "policyAnnounce".

            policyReports.forEach((policyReport) => {
                if (!policyReport) {
                    return;
                }
                const {reportID} = policyReport;
+               // Remove the announce channel
+               if (policyReport.type === 'chat' && policyReport.chatType === 'policyAnnounce') {
+                 cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = null;
+               } else {
+                 cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = {hasDraft: false};
+               }
-              cleanUpMergeQueries[`${ONYXKEYS.COLLECTION.REPORT}${reportID}`] = {hasDraft: false};
                cleanUpSetQueries[`${ONYXKEYS.COLLECTION.REPORT_DRAFT_COMMENT}${reportID}`] = null;
                cleanUpSetQueries[`${ONYXKEYS.COLLECTION.REPORT_ACTIONS_DRAFTS}${reportID}`] = null;
            });

cleanUpMergeQueries will be merged into Onyx cache and remove the report, UI that subscribes to reports will be updated to the new list without the announce channel report from deleted workspace.

No need to check for room ownership because this part is only run for room participants when the workspace is removed locally, the workspace is not removed locally for admin and only marked as archived.

https://github.com/Expensify/App/blob/main/src/libs/actions/Policy.ts#L76

What alternative solutions did you explore? (Optional)

Alternatively, we could send an update from server to update the related reports to null. But it depends on how the backend logic really is. If after policy deletion, backend will not send anything about the related reports to frontend again, then doing it on frontend makes more sense.

Note: consolidated proposal from initial proposal and following comments as requested by @Santhosh-Sellavel

[melvin-bot[bot]]

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

[adelekennedy]

holding on this - @Santhosh-Sellavel will be back in a week

[Santhosh-Sellavel]

@cheng-kang's proposal looks good!

We can make use of ReportUtils.isAnnounceRoom method to check #announce room.

C+ Reviewed 🎀 👀 🎀

[melvin-bot[bot]]

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

[melvin-bot[bot]]

@pecanoro @adelekennedy @Santhosh-Sellavel 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!

[pecanoro]

I am wondering, how is this a problem for the announce room but not the other policy rooms or the admins room? What happens in those cases?

[Santhosh-Sellavel]

Asked!

@pecanoro Check this internal thread

[Santhosh-Sellavel]

@pecanoro Any update have you discussed it on the thread?

cc: @adelekennedy

[melvin-bot[bot]]

@pecanoro @adelekennedy @Santhosh-Sellavel this issue is now 3 weeks old. There is one more week left before this issue breaks WAQ and will need to go internal. What needs to happen to get a PR in review this week? Please create a thread in #expensify-open-source to discuss. Thanks!

[pecanoro]

Oh sorry, I forgot to look into this, I will check the thread today

[Santhosh-Sellavel]

@adelekennedy Unassigning as am planning for OOO, please assign a new C+ here.

[aimane-chnaif]

I am interested as C+ as this looks similar to #35751 I was reviewing (but not same).

[melvin-bot[bot]]

@pecanoro @adelekennedy this issue is now 4 weeks old and preventing us from maintaining WAQ, can you:

• Decide whether any proposals currently meet our guidelines and can be approved as-is today
• If no proposals meet that standard, please take this issue internal and treat it as one of your highest priorities
• If you have any questions, don't hesitate to start a discussion in #expensify-open-source

Thanks!

[melvin-bot[bot]]

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

[pecanoro]

Sorry for the delay, I lost track of this issue, my bad! After reviewing some things, yes, proposal sounds good, assigning!

[melvin-bot[bot]]

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

[mvtglobally]

Issue not reproducible during KI retests. (First week)

[melvin-bot[bot]]

📣 @aimane-chnaif 🎉 An offer has been automatically sent to your Upwork account for the Contributor role 🎉 Thanks for contributing to the Expensify app!

Offer link Upwork job Please accept the offer and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻 Keep in mind: Code of Conduct | Contributing 📖

[adelekennedy]

@aimane-chnaif so sorry! This slipped through while I was ooo last week - added you to the issue here but noting that you are the C+

[pecanoro]

Is this still reproducible?

[melvin-bot[bot]]

@pecanoro, @cheng-kang, @adelekennedy, @aimane-chnaif Whoops! This issue is 2 days overdue. Let's get this updated quick!

[aimane-chnaif]

@lanitochka17 can you please confirm if this is still reproducible on latest staging?

[lanitochka17]

@aimane-chnaif Hello Issue is still reproducible. Announce room is not removed when deleting the workspace but it is removed when removing a member

https://github.com/Expensify/App/assets/78819774/88665a45-67dc-4d24-8518-f5ad80963435

[aimane-chnaif]

@cheng-kang how's it going?

[pecanoro]

@cheng-kang Last ping before I open it back to other people since we haven't heard from you

[pecanoro]

Opening back to contributors since we haven't heard back in a while

[Krishna2323]

@pecanoro @aimane-chnaif, I can help implementing the selected proposal if needed. Thanks