[Due for payment 2025-02-19] [$250] Debug - Rooms that deny access to post are listed in list of contacts to share log data

[vincdargento]

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: v9.0.82-4
Reproducible in staging?: Yes
Reproducible in production?: Yes
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?: N/A
If this was caught during regression testing, add the test name, ID and link from TestRail: N/A
Email or phone of affected tester (no customers): N/A
Issue reported by: Applause Internal Team
Device used: Windows 11/ Chrome, Android 13/ Chrome, Tecno Spark 20/ Android 13
App Component: User Settings

Action Performed:

Pre-condition: Be a member of a workspace with more than 2 members

1. Sign in to NewDot
2. Navigate to Account settings > Troubleshoot
3. Enable client side logging
4. Navigate to view client side logging
5. Click on share

Expected Result:

List of accounts and workspace that the user can send messages to are shown.

Actual Result:

Rooms that the user cannot send messages to like #announce room of another workspace are included in the list of accounts to share to.

Workaround:

Unknown

Platforms:

• Android: Standalone
• Android: HybridApp
• Android: mWeb Chrome
• iOS: Standalone
• iOS: HybridApp
• iOS: mWeb Safari
• MacOS: Chrome / Safari
• MacOS: Desktop

Screenshots/Videos

bug.mp4

View all open jobs on GitHub

Upwork Automation - Do Not Edit

• Upwork Job URL: https://www.upwork.com/jobs/~021884015946368018307
• Upwork Job ID: 1884015946368018307
• Last Price Increase: 2025-02-03

Issue Owner

Current Issue Owner: @strepanier03

[melvin-bot]

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

[bernhardoj]

Proposal

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

Announce room is shown on the share log list even though it's not writable.

What is the root cause of that problem?

Currently, we don't have any filter to filter out read-only report.

App/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx

Line 40 in df73b07

const shareLogOptions = OptionsListUtils.getShareLogOptions(options, betas ?? []);

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

We need to add a new option to getValidOptions whether to filter out read-only report or not. We pass it as true from getShareLogOptions. In getValidOptions, we filter it out using the condition from the ReportFooter.

App/src/pages/home/report/ReportFooter.tsx

Lines 115 to 116 in df73b07

	const canPerformWriteAction = ReportUtils.canUserPerformWriteAction(report) ?? shouldShowComposerOptimistically;
	const shouldHideComposer = !canPerformWriteAction || isBlockedFromChat;

App/src/libs/OptionsListUtils.ts

Lines 1174 to 1191 in df73b07

	const allReportOptions = filteredReportOptions.filter((option) => {
	const report = option.item;
	if (!report) {
	return false;
	}
	const isThread = option.isThread;
	const isTaskReport = option.isTaskReport;
	const isPolicyExpenseChat = option.isPolicyExpenseChat;
	const isMoneyRequestReport = option.isMoneyRequestReport;
	const isSelfDM = option.isSelfDM;
	const isChatRoom = option.isChatRoom;
	const accountIDs = ReportUtils.getParticipantsAccountIDsForDisplay(report);
	if (isPolicyExpenseChat && report.isOwnPolicyExpenseChat && !includeOwnedWorkspaceChats) {
	return false;
	}

if (!includeReadOnlyReport && !ReportUtils.canUserPerformWriteAction(report) ....) {
    return false;
}

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We need to test getShareLogOptions that it correctly filter out any read-only report (dummy list)

[huult]

Edited by proposal-police: This proposal was edited at 2025-01-09 17:00:39 UTC.

Proposal

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

Rooms that deny access to post are listed in list of contacts to share log data

What is the root cause of that problem?

We do not filter valid options for the share log list.

App/src/pages/settings/AboutPage/ShareLogList/BaseShareLogList.tsx

Line 30 in e68f0a8

const defaultOptions = useMemo(() => {

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

To resolve this issue, we should add a new condition to check if isReservedRoomName is true, then filter out the option list.

App/src/libs/OptionsListUtils.ts

Line 1142 in 98bb043

shouldBoldTitleByDefault = true,

add

  isReservedRoomName = false
  isArchivedRoom = false

App/src/libs/OptionsListUtils.ts

Line 1214 in 98bb043

add:

    if (isReservedRoomName && ValidationUtils.isReservedRoomName(option.text)) {
        return false;
    }
    if (isArchivedRoom && ReportUtils.isArchivedRoom(option)) {
        return false;
    }
    ...

App/src/libs/OptionsListUtils.ts

Line 1369 in 98bb043

function getShareLogOptions(options: OptionList, betas: Beta[] = []): Options {

update to:

  function getShareLogOptions(options: OptionList, betas: Beta[] = []): Options {
      return getValidOptions(options, {
          betas,
          includeMultipleParticipantReports: true,
          includeP2P: true,
          forcePolicyNamePreview: true,
          includeOwnedWorkspaceChats: true,
          includeSelfDM: true,
          includeThreads: true,
          isReservedRoomName: true, // add this line
          isArchivedRoom: true, // add this line
      });
  }

What specific scenarios should we cover in automated tests to prevent reintroducing this issue in the future?

We need to test if getShareLogOptions data does not include isReservedRoomName and isArchivedRoom.

What alternative solutions did you explore? (Optional)

Reminder: Please use plain English, be brief and avoid jargon. Feel free to use images, charts or pseudo-code if necessary. Do not post large multi-line diffs or write walls of text. Do not create PRs unless you have been hired for this job.

[melvin-bot]

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

[melvin-bot]

@strepanier03 Eep! 4 days overdue now. Issues have feelings too...

[melvin-bot]

@strepanier03 6 days overdue. This is scarier than being forced to listen to Vogon poetry!

[strepanier03]

Working on testing this but need to make a new test account because my current ones have too many workspaces so it's hard to tell which in the list I shouldn't have access too.

[melvin-bot]

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

[melvin-bot]

@strepanier03 this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

[melvin-bot]

@strepanier03 Huh... This is 4 days overdue. Who can take care of this?

[strepanier03]

Hmm, I'm unable to repro this with a fresh account on Mac/Chrome or Android mWeb Chrome.

[strepanier03]

Sending for repro.

[MelvinBot]

This has been labelled "Needs Reproduction". Follow the steps here: https://stackoverflowteams.com/c/expensify/questions/16989

[allgandalf]

Able to reproduce, taking over as C+ as per

@strepanier03 can you please assign me here

[melvin-bot]

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

[strepanier03]

@allgandalf - Per this post in Slack, it looks like we're not supposed to assign C+ based on request. You are supposed to post repro steps and then the auto-assigner is still supposed to fairly disseminate issues among the C+ team.

[allgandalf]

@allgandalf - Per this post in Slack, it looks like we're not supposed to assign C+ based on request. You are supposed to post repro steps and then the auto-assigner is still supposed to fairly disseminate issues among the C+ team.

@strepanier03 if a issue gets posted in contributor plus and we are able to reproduce the issue, then we can ask to be named as a C+. I came to this issue using this post:

. The first C+ to post on the GH issue with reliable reproduction steps and video or screenshots confirming reproduction will be assigned as C+.

[parasharrajat]

@allgandalf Man. This I got in 10 days, Leave it me, pleassssseeee. 😄

[parasharrajat]

So we should hide read-only chats from the list thus @bernhardoj's proposal looks good to me. Reserved name rooms like announce can be valid options.

🎀 👀 🎀 C+ reviewed

[melvin-bot]

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

[strepanier03]

@allgandalf and @parasharrajat - My apologies, I truly did not mean to take a job away from someone and give it to someone else or mess up the documented process.

Since @parasharrajat has already been assigned this, and they've reviewed and selected a proposal, I am going to leave this assignment to them.

@allgandalf, I am fully up to date on the process for needs reproduction now and I won't make this mistake again.

[melvin-bot]

@strepanier03, @luacmartins, @parasharrajat Eep! 4 days overdue now. Issues have feelings too...

[huult]

@parasharrajat could you review my proposal?

[parasharrajat]

@huult why would we hide reserved rooms? If the member is admin, they can message in a few reserved rooms.

Also, syntactically the params are indicating opposite.

[melvin-bot]

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

[parasharrajat]

Bump @luacmartins

[luacmartins]

Thanks for the bump. I missed this one

[bernhardoj]

PR is ready

cc: @parasharrajat

[melvin-bot]

Reviewing label has been removed, please complete the "BugZero Checklist".

[melvin-bot]

The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.96-1 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• Filter out read only report from share log options #56398

If no regressions arise, payment will be issued on 2025-02-19. 🎊

For reference, here are some details about the assignees on this issue:

• @parasharrajat requires payment through NewDot Manual Requests
• @bernhardoj requires payment through NewDot Manual Requests

[melvin-bot]

@parasharrajat @strepanier03 @parasharrajat The PR fixing this issue has been merged! The following checklist (instructions) will need to be completed before the issue can be closed. Please copy/paste the BugZero Checklist from here into a new comment on this GH and complete it. If you have the K2 extension, you can simply click: [this button]