Filter hidden Account Managers from search From filter and scope to workspace members#84376
Filter hidden Account Managers from search From filter and scope to workspace members#84376
Conversation
Account Managers added as hidden members of managed customer workspaces were appearing in the From filter on the Reports page. This change: 1. Passes shouldExcludeHidden=true in processReport so hidden participants are excluded from report-based option lists 2. Adds shouldScopeToWorkspaceMembers prop to SearchFiltersParticipantsSelector which filters personal details to only workspace members across all user policies 3. Enables free text email input in the From filter so users can type arbitrary emails not in the suggestion list Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
When allowFreeTextInput is true, include the userToInvite option in the sections list so typed emails that don't match existing options appear as selectable entries. Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
I created this PR from an upstream branch since I don't have push access to your fork. To take ownership of this branch and be able to push updates, run: Then you can close this PR and open a new one from your fork. |
Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
Fixed the failing Prettier check. The formatting issues were in
|
|
@MelvinBot Mark this PR as ready |
|
PR has been marked as ready for review. |
|
@DylanDylann Please copy/paste the Reviewer Checklist from here into a new comment on this PR and complete it. If you have the K2 extension, you can simply click: [this button] |
|
@MelvinBot We got failed test |
|
@codex review |
…processReport The processReport change to exclude hidden participants via getParticipantsAccountIDsForDisplay(report, true) caused report 12 (thread with only hidden participants) to be dropped from OPTIONS entirely, making the old excludeHiddenThreads subtraction incorrect. - Add visible participant (current user) to report 11 fixture so it remains in the option list while still having a hidden participant - Update 4 test assertions that previously subtracted 1 for the hidden thread, since it's now excluded at the processReport level Co-authored-by: DylanDylann <DylanDylann@users.noreply.github.com>
|
Fixed the failing Root cause: The Fix:
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cf577856f3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| memberIDs.add(currentUserAccountID); | ||
| return memberIDs; | ||
| }, [shouldScopeToWorkspaceMembers, allPolicies, currentUserAccountID]); |
There was a problem hiding this comment.
Recompute workspace member IDs after personal details hydrate
workspaceMemberAccountIDs is memoized without any dependency on personal details, but getMemberAccountIDsForWorkspace() resolves each member through getPersonalDetailByEmail (src/libs/PolicyUtils.ts:513). On cold start or partial Onyx hydration, policies can load before personal details, so this memo captures an incomplete member set and never refreshes, causing valid workspace members to be missing from the From filter until policies change. Include personal-details state in this memo (or derive account IDs directly from the already-available personalDetails) so the scoped list stays correct.
Useful? React with 👍 / 👎.
Codecov Report❌ Looks like you've decreased code coverage for some files. Please write tests to increase, or at least maintain, the existing level of code coverage. See our documentation here for how to interpret this table.
|
|
I ran out of time before I could finish helping you. This usually means the task was too complex or I got stuck exploring. Please try again with a more specific request or post in the #melvinbot-ai. Run details: https://github.com/Expensify/Expensify/actions/runs/22755517486 |
|
Let me check |
trjExpensify
left a comment
There was a problem hiding this comment.
Agreed product change in Slack, but please get videos up and test this a bunch.
Explanation of Change
Account Managers (AMs) are added as "hidden" members of managed customer workspaces with
notificationPreference: 'hidden'. They should be invisible to the customer, but were appearing in the "From" filter on the Reports/Search page. To match Classic behavior, this PR:Excludes hidden participants from report-based options —
processReportinOptionsListUtilsnow passesshouldExcludeHidden=truetogetParticipantsAccountIDsForDisplay, consistent with other call sites at lines 1123 and 1184 that already passtrue.Scopes the From filter to workspace members only — Adds a
shouldScopeToWorkspaceMembersprop toSearchFiltersParticipantsSelectorthat collects all workspace member accountIDs across the user's policies viagetMemberAccountIDsForWorkspaceand filters the personalDetails suggestions to only those members.Enables free text email input in the From filter — When
shouldScopeToWorkspaceMembersis enabled, the component allows users to type arbitrary emails that aren't in the suggestion list, matching Classic's behavior. TheuserToInviteoption is included in sections so typed emails appear as selectable entries.Fixed Issues
$ #83879
PROPOSAL: #83879 (comment)
Tests
test@example.comin the search fieldOffline tests
QA Steps
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)src/languages/*files and using the translation methodSTYLE.md) were followedAvatar, I verified the components usingAvatarare working as expected)StyleUtils.getBackgroundAndBorderStyle(theme.componentBG))npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.ScrollViewcomponent to make it scrollable when more elements are added to the page.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
Tested on web — From filter shows workspace members only and accepts free text email input.