[$500] Public room can't be seen on LHN after opening it from a link #30615

lanitochka17 · 2023-10-30T23:44:07Z

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.3.93-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:

Navigate to https://staging.new.expensify.com/
Log out
Navigate to this link - https://staging.new.expensify.com/r/3504895439653267
Log in with any accounts

Expected Result:

The public room should be displayed

Actual Result:

The public room can't be seen on LHN

Workaround:

Unknown

Platforms:

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

Screenshots/Videos

Android: Native

Android: mWeb Chrome

iOS: Native

iOS: mWeb Safari

Window: Chrome

MacOS: Desktop

View all open jobs on GitHub

Upwork Automation - Do Not Edit

Upwork Job URL: https://www.upwork.com/jobs/~010cd18e947fb2a8d5
Upwork Job ID: 1719138151952470016
Last Price Increase: 2023-10-30
Automatic offers:

dukenv0307 | Contributor | 27493079

melvin-bot · 2023-10-30T23:44:12Z

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

melvin-bot · 2023-10-30T23:44:12Z

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

melvin-bot · 2023-10-30T23:44:14Z

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 · 2023-10-30T23:44:17Z

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

erquhart · 2023-10-31T00:28:58Z

Proposal

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

If the current chat is a public room, it should show in the left hand nav.

What is the root cause of that problem?

Public rooms were removed from the left hand nav in a recent PR:
https://github.com/Expensify/App/pull/27966/files#diff-65c096044d5f69b35bcdec14c99c4fda4580759df9b1a7c36650d58eea276f1d

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

Revert to fix (tested locally, works).

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.

jliexpensify · 2023-10-31T01:08:10Z

Hmm I'm unable to repro on Staging v1.3.93-0 (Chrome) - LHN is there for me:

erquhart · 2023-10-31T01:16:01Z

@jliexpensify left hand nav is there, but the public channel currently being viewed isn’t in the list.

jliexpensify · 2023-10-31T02:42:59Z

Ah sorry, my mistake - thanks @erquhart!

dukenv0307 · 2023-10-31T09:27:05Z

Proposal

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

Public room can't be seen on LHN after opening it from a link

What is the root cause of that problem?

BE returns participantAccountIDs as an empty array for this public room

And then this report is excluded in LHN by this condition

App/src/libs/ReportUtils.js

Lines 3278 to 3282 in deb267f

    
           report.participantAccountIDs.length === 0 && 
        
           !isChatThread(report) && 
        
           !isUserCreatedPolicyRoom(report) && 
        
           !isArchivedRoom(report) && 
        
           !isMoneyRequestReport(report) &&

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

We can fix this issue in BE. We should return correctly participantAccountIDs of the public room.
If it's expected behavior to return participantAccountIDs as empty for some special public room, we can add the check for public room here to not exclude the public room if the participantAccountIDs is empty

report.participantAccountIDs.length === 0 &&
!isChatThread(report) &&
!isPublicRoom(report) &&
!isUserCreatedPolicyRoom(report) &&
!isArchivedRoom(report) &&
!isMoneyRequestReport(report) &&
!isTaskReport(report))

App/src/libs/ReportUtils.js

Lines 3278 to 3282 in deb267f

    
           report.participantAccountIDs.length === 0 && 
        
           !isChatThread(report) && 
        
           !isUserCreatedPolicyRoom(report) && 
        
           !isArchivedRoom(report) && 
        
           !isMoneyRequestReport(report) &&

What alternative solutions did you explore? (Optional)

thesahindia · 2023-10-31T17:32:44Z

Asked @jasperhuangg for their opinion.

jasperhuangg · 2023-11-01T18:52:18Z

@thesahindia I can't seem to find your comment, do you mind linking it again?

jasperhuangg · 2023-11-01T18:54:03Z

So it's actually expected that the participantAccountIDs are empty if you're not a member of the workspace that the public room is a part of.

This is because of privacy concerns. So no BE fixes are needed here.

jasperhuangg · 2023-11-01T18:56:28Z

We just need to update the condition here to be the following. I forgot to take into account public announce rooms in my PR.

report.participantAccountIDs.length === 0 && 
 !isChatThread(report) && 
 !isUserCreatedPolicyRoom(report) && 
 !isPublicRoom(report) && 
 !isArchivedRoom(report) && 
 !isMoneyRequestReport(report) &&

I think @erquhart's proposal was the closest to this (but we definitely shouldn't be reverting that entire PR). If you're willing to get another proposal up explaining why this works then I'd be happy to accept it.

erquhart · 2023-11-01T20:20:18Z

@jasperhuangg that's what my existing proposal is, I just didn't check to see what else that PR had. When I say to revert, I only meant reverting the exclusion of the isPublicRoom() condition. I'll go ahead and make an updated proposal.

erquhart · 2023-11-01T20:26:33Z

Proposal

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

Public rooms opened via link aren't showing in the left hand nav.

What is the root cause of that problem?

Reports with a visibility property matching PUBLIC or PUBLIC_ANNOUNCE used to be included in the left hand nav via isPublicRoom():

App/src/libs/ReportUtils.js

Lines 411 to 413 in 1700ecf

    
           function isPublicRoom(report) { 
        
               return report && (report.visibility === CONST.REPORT.VISIBILITY.PUBLIC || report.visibility === CONST.REPORT.VISIBILITY.PUBLIC_ANNOUNCE); 
        
           }

This was removed with - based on the PR conversation from #27966 - the understanding that public rooms were expected to be included in isUserCreatedPolicyRoom(), but that function only includes reports with a POLICY_ROOM chat type. isPublicRoom() is still needed to match report visibility against PUBLIC and PUBLIC_ANNOUNCE types.

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

Re-add the !isPublicRoom(report) statement to this conditional:

App/src/libs/ReportUtils.js

Lines 3277 to 3283 in deb267f

    
           (report.participantAccountIDs && 
        
               report.participantAccountIDs.length === 0 && 
        
               !isChatThread(report) && 
        
               !isUserCreatedPolicyRoom(report) && 
        
               !isArchivedRoom(report) && 
        
               !isMoneyRequestReport(report) && 
        
               !isTaskReport(report))

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.

thesahindia · 2023-11-02T10:27:48Z

2. If it's expected behavior to return participantAccountIDs as empty for some special public room, we can add the check for public room here to not exclude the public room if the participantAccountIDs is empty
report.participantAccountIDs.length === 0 &&
!isChatThread(report) &&
!isPublicRoom(report) &&
!isUserCreatedPolicyRoom(report) &&
!isArchivedRoom(report) &&
!isMoneyRequestReport(report) &&
!isTaskReport(report))

Actually @dukenv0307 suggested two solutions and their second solution was to add !isPublicRoom(report). So I think they should be assigned.

🎀 👀 🎀 C+ reviewed

melvin-bot · 2023-11-02T10:27:54Z

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

jasperhuangg · 2023-11-02T18:01:54Z

Got it, since @dukenv0307 had a clearer proposal and was more in line with what I was thinking, let's assign them. cc @neil-marcellini

melvin-bot · 2023-11-02T18:02:02Z

📣 @thesahindia Please request via NewDot manual requests for the Reviewer role ($500)

melvin-bot · 2023-11-02T18:02:07Z

📣 @dukenv0307 🎉 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 📖

neil-marcellini · 2023-11-02T19:32:36Z

Cool, I agree that the proposal from @dukenv0307 looks good.

dukenv0307 · 2023-11-03T04:37:12Z

@thesahindia The PR is ready for review.

jliexpensify · 2023-11-21T04:27:38Z

Hrrrm what happened with the updater here? It looks like this hit production 2 weeks ago right? If so, I should be paying + the checklist should be worked on, right?

dukenv0307 · 2023-11-21T04:33:08Z

Yes, seems like bot not work on this issue.

jliexpensify · 2023-11-21T04:33:11Z

If we are going off #30814, here's the Payment Summary:

Contributor: @dukenv0307 $750 (bonus as it was merged in 3 working days)
C+: @thesahindia$750 (bonus as it was merged in 3 working days) - NEW.DOT PAYMENT

Upworks job

is this all correct? @thesahindia could you complete the checklist too?

jliexpensify · 2023-11-21T04:51:34Z

Paid and job closed. Waiting on checklist to be completed!

thesahindia · 2023-11-22T18:25:25Z

We just need to update the condition here to be the following. I forgot to take into account public announce rooms in my PR.

As mentioned in #30615 (comment), this case wasn't taken into account.

We can add a test case -

Login to user A and create a public room for workspace that doesn't contain user B
Log out and open the public room using the link
Login to account B
Verify the room is visible in the LHN

JmillsExpensify · 2023-11-27T19:49:10Z

$750 payment approved for @thesahindia based on this comment.

lanitochka17 added External Added to denote the issue can be worked on by a contributor Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Oct 30, 2023

melvin-bot bot changed the title ~~Public room can't be seen on LHN after opening it from a link~~ [$500] Public room can't be seen on LHN after opening it from a link Oct 30, 2023

melvin-bot bot assigned jliexpensify Oct 30, 2023

melvin-bot bot added the Help Wanted Apply this label when an issue is open to proposals by contributors label Oct 30, 2023

melvin-bot bot assigned thesahindia Oct 30, 2023

thesahindia mentioned this issue Oct 31, 2023

UI for room invitations/removals #27966

Merged

57 tasks

melvin-bot bot assigned neil-marcellini Nov 2, 2023

jasperhuangg assigned dukenv0307 Nov 2, 2023

melvin-bot bot removed the Help Wanted Apply this label when an issue is open to proposals by contributors label Nov 2, 2023

dukenv0307 mentioned this issue Nov 3, 2023

Display public room in LHN #30814

Merged

59 tasks

melvin-bot bot added Reviewing Has a PR in review Weekly KSv2 and removed Daily KSv2 labels Nov 3, 2023

jliexpensify changed the title ~~[$500] Public room can't be seen on LHN after opening it from a link~~ [NEED TO PAY?][$500] Public room can't be seen on LHN after opening it from a link Nov 21, 2023

jliexpensify changed the title ~~[NEED TO PAY?][$500] Public room can't be seen on LHN after opening it from a link~~ [WAITING ON CHECKLIST][$500] Public room can't be seen on LHN after opening it from a link Nov 21, 2023

jliexpensify changed the title ~~[WAITING ON CHECKLIST][$500] Public room can't be seen on LHN after opening it from a link~~ [$500] Public room can't be seen on LHN after opening it from a link Nov 22, 2023

jliexpensify closed this as completed Nov 22, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[$500] Public room can't be seen on LHN after opening it from a link #30615

[$500] Public room can't be seen on LHN after opening it from a link #30615

lanitochka17 commented Oct 30, 2023 •

edited by melvin-bot bot

Loading

melvin-bot bot commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

erquhart commented Oct 31, 2023 •

edited

Loading

jliexpensify commented Oct 31, 2023

erquhart commented Oct 31, 2023

jliexpensify commented Oct 31, 2023

dukenv0307 commented Oct 31, 2023

thesahindia commented Oct 31, 2023

jasperhuangg commented Nov 1, 2023

jasperhuangg commented Nov 1, 2023

jasperhuangg commented Nov 1, 2023 •

edited

Loading

erquhart commented Nov 1, 2023

erquhart commented Nov 1, 2023 •

edited

Loading

thesahindia commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

jasperhuangg commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

neil-marcellini commented Nov 2, 2023 •

edited

Loading

dukenv0307 commented Nov 3, 2023

jliexpensify commented Nov 21, 2023

dukenv0307 commented Nov 21, 2023

jliexpensify commented Nov 21, 2023

jliexpensify commented Nov 21, 2023

thesahindia commented Nov 22, 2023

JmillsExpensify commented Nov 27, 2023

[$500] Public room can't be seen on LHN after opening it from a link #30615

[$500] Public room can't be seen on LHN after opening it from a link #30615

Comments

lanitochka17 commented Oct 30, 2023 • edited by melvin-bot bot Loading

Action Performed:

Expected Result:

Actual Result:

Workaround:

Platforms:

Screenshots/Videos

melvin-bot bot commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

melvin-bot bot commented Oct 30, 2023

Bug0 Triage Checklist (Main S/O)

melvin-bot bot commented Oct 30, 2023

erquhart commented Oct 31, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

jliexpensify commented Oct 31, 2023

erquhart commented Oct 31, 2023

jliexpensify commented Oct 31, 2023

dukenv0307 commented Oct 31, 2023

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

thesahindia commented Oct 31, 2023

jasperhuangg commented Nov 1, 2023

jasperhuangg commented Nov 1, 2023

jasperhuangg commented Nov 1, 2023 • edited Loading

erquhart commented Nov 1, 2023

erquhart commented Nov 1, 2023 • edited Loading

Proposal

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

What is the root cause of that problem?

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

What alternative solutions did you explore? (Optional)

thesahindia commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

jasperhuangg commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

melvin-bot bot commented Nov 2, 2023

neil-marcellini commented Nov 2, 2023 • edited Loading

dukenv0307 commented Nov 3, 2023

jliexpensify commented Nov 21, 2023

dukenv0307 commented Nov 21, 2023

jliexpensify commented Nov 21, 2023

jliexpensify commented Nov 21, 2023

thesahindia commented Nov 22, 2023

JmillsExpensify commented Nov 27, 2023

lanitochka17 commented Oct 30, 2023 •

edited by melvin-bot bot

Loading

erquhart commented Oct 31, 2023 •

edited

Loading

jasperhuangg commented Nov 1, 2023 •

edited

Loading

erquhart commented Nov 1, 2023 •

edited

Loading

neil-marcellini commented Nov 2, 2023 •

edited

Loading