Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Focus mode is not enabled when adding a bunch of members to the workspace #58535

Closed
1 of 8 tasks
m-natarajan opened this issue Mar 16, 2025 · 12 comments
Closed
1 of 8 tasks
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2

Comments

@m-natarajan
Copy link

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: 9.1.13-4
Reproducible in staging?: y
Reproducible in production?: y
If this was caught on HybridApp, is this reproducible on New Expensify Standalone?:
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: @danielrvidal
Slack conversation (hyperlinked to channel name): #convert

Action Performed:

  1. Create a workspace
  2. Add bunch of members to the workspace (20+)

Expected Result:

This should trigger focus mode instead of showing all expense chats in the LHN

Actual Result:

This creates a bunch of expense chats in the LHN, and not triggering focus mode

Workaround:

unknown

Platforms:

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

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

Screenshots/Videos

Add any screenshot/video evidence
Recording.1057.mp4

View all open jobs on GitHub

@m-natarajan m-natarajan added Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2 labels Mar 16, 2025
Copy link

melvin-bot bot commented Mar 16, 2025

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

@samranahm
Copy link
Contributor

samranahm commented Mar 16, 2025

🚨 Edited by proposal-police: This proposal was edited at 2025-03-16 18:47:08 UTC.

Proposal

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

Focus mode is not enabled when adding a bunch of members to the workspace

What is the root cause of that problem?

feature request

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

Here

const inviteUser = useCallback(() => {

we should explicitly check if the invited members are more then a limit(we choose) and call this function

App/src/libs/actions/User.ts

Lines 1102 to 1104 in 6d00a6b

function updateChatPriorityMode(mode: ValueOf<typeof CONST.PRIORITY_MODE>, automatic = false) {
const autoSwitchedToFocusMode = mode === CONST.PRIORITY_MODE.GSD && automatic;
const optimisticData: OnyxUpdate[] = [

In WorkspaceInvitePage function

Add new const bellow [betas]

const [priorityMode] = useOnyx(ONYXKEYS.NVP_PRIORITY_MODE);

and update current inviteUser logic like this

if (selectedOptions.length >= 20) {
    if(priorityMode === CONST.PRIORITY_MODE.GSD){
        return;
    }
    updateChatPriorityMode(CONST.PRIORITY_MODE.GSD, true);
}

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

Render the workspaceInvitePage and mock the invite process
Add unit test to check

  • Early return if selectedOptions.length >= 20 and priorityMode === CONST.PRIORITY_MODE.GSD
  • Should call updateChatPriorityMode if selectedOptions.length >= 20 and priorityMode !== GSD
  • Verify priorityMode updates in onyx

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.

Copy link
Contributor

⚠️ @samranahm Thanks for your proposal. Please update it to follow the proposal template, as proposals are only reviewed if they follow that format (note the mandatory sections).

@rushatgabhane
Copy link
Member

@samranahm it'll be valuable to add an unit test for this. could you please update your proposal accordingly

@rushatgabhane
Copy link
Member

rushatgabhane commented Mar 16, 2025

@NicMendonca could you please assign me for c+ here, i was involved in the discussions of this bug here - slack 🧵

@CyberAndrii

This comment has been minimized.

@rushatgabhane
Copy link
Member

rushatgabhane commented Mar 16, 2025

@CyberAndrii's proposal LGTM 🎀 👀 🎀

(assuming this goes external)

Copy link

melvin-bot bot commented Mar 16, 2025

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

@samranahm
Copy link
Contributor

@truph01
Copy link
Contributor

truph01 commented Mar 17, 2025

I think this is expected behavior because: if the user has more than 30 reports but previously enabled focus mode and then turned it off (nvp_tryFocusMode is true), it won’t turn on automatically again.

@CyberAndrii
Copy link
Contributor

if the user has more than 30 reports but previously enabled focus mode and then turned it off (nvp_tryFocusMode is true), it won’t turn on automatically again.

That's probably the case (judging by the number of workspaces in the video). So ignore my proposal

@cristipaval
Copy link
Contributor

I think this is expected behavior because: if the user has more than 30 reports but previously enabled focus mode and then turned it off (nvp_tryFocusMode is true), it won’t turn on automatically again.

I agree with this. Also, if the user hasn't tried the focus mode before, it will show up the popup later anyway, so I think it is not worth fixing this to make it show right away for a specific action.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. Daily KSv2
Projects
Development

No branches or pull requests

7 participants