Expensify
[HOLD #431875][$500] Announce - Deleted announce room is not removed from LHN for employee
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: 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:
- With an admin account A, create workspace > invite user B to workspace > send message in announce room
- Verify user B sees the announce room in LHN
- 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
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
Job added to Upwork: https://www.upwork.com/jobs/~0197a54c54a4f9ab7b
![melvin-bot[bot] avatar](https://avatars.githubusercontent.com/in/179547?v=4 "Published by a pub.dev verified publisher"){kind=small}
Triggered auto assignment to Contributor-plus team member for initial proposal review - @Santhosh-Sellavel (External)
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.
📣 @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:
- Make sure you've read and understood the contributing guidelines.
- 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.
- 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.
- 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>
Contributor details Your Expensify account email: [email protected] Upwork Profile Link: https://www.upwork.com/freelancers/~017f672cbef45659ec
✅ Contributor details stored successfully. Thank you for contributing to Expensify!
@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!
@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.
Triggered auto assignment to @adelekennedy (Bug), see https://stackoverflow.com/c/expensify/questions/14418 for more details.
@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
reportstill 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?
@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.
@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?
@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.
@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 for announce channels only we can check if type === "chat" && chatType === "policyAnnounce"
@Santhosh-Sellavel
Yep, as it stands, #announce should be unshared with all members apart from the workspace admins at the time of deletion
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
@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.
@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.
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
📣 It's been a week! Do we have any satisfactory proposals yet? Do we need to adjust the bounty for this issue? 💸
@cheng-kang's proposal looks good!
We can make use of ReportUtils.isAnnounceRoom method to check #announce room.
C+ Reviewed 🎀 👀 🎀
Triggered auto assignment to @pecanoro, see https://stackoverflow.com/c/expensify/questions/7972 for more details.
@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!
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?
