Fix missing owner details on pending join request workspace rows#95417
Conversation
The owner of a workspace the user only requested to join is usually not in the personal details list, so the owner column rendered empty. Fall back to the ownerEmail already provided by policyDetailsForNonMembers, and use the pending policy name for the default workspace avatar. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@marufsharifi 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] |
| expect(screen.getByText('Pending Workspace')).toBeOnTheScreen(); | ||
| expect(screen.getByText(new RegExp(OWNER_EMAIL))).toBeOnTheScreen(); |
There was a problem hiding this comment.
NAB: This UI test verifies the owner email, but not the two visual fallback changes: the owner default avatar and the workspace default icon derived from nonMemberDetails.name. Since the manual steps cover this, I wouldn’t block on it, but adding an assertion for the workspace default avatar test ID would improve regression coverage.
There was a problem hiding this comment.
[Nico's Agent] Added an assertion for the workspace default avatar test ID (SvgDefaultAvatar_p Icon) to the UI test; the owner default avatar is now covered by the selector unit test since it's derived there. Done in 5d98bf8.
| describe('createWorkspaceListPoliciesSelector', () => { | ||
| it('projects owner details from policyDetailsForNonMembers for join-request-pending policies', () => { | ||
| const pendingPolicyID = '1'; | ||
| const policies: OnyxCollection<Policy> = { | ||
| pendingPolicy: buildSelectorPolicy(1, { | ||
| isJoinRequestPending: true, | ||
| policyDetailsForNonMembers: { | ||
| [pendingPolicyID]: { | ||
| name: 'Pending Workspace', | ||
| type: CONST.POLICY.TYPE.TEAM, |
There was a problem hiding this comment.
Blocking: this adds duplicate coverage for createWorkspaceListPoliciesSelector in tests/unit/selectors/PolicyTest.ts. There’s already a dedicated suite for this selector in tests/unit/PolicySelectorTest.ts, including pending join nonMemberDetails projection and the regular-policy nonMemberDetails undefined case. Could we remove this new duplicated describe block and instead update the existing pending join projection test to include ownerEmail in the fixture and expected nonMemberDetails?
There was a problem hiding this comment.
[Nico's Agent] Removed the duplicated describe block from tests/unit/selectors/PolicyTest.ts and updated the existing pending join projection test in tests/unit/PolicySelectorTest.ts to cover ownerEmail (and the new derived ownerDefaultAvatar). Fixed in 5d98bf8.
Reviewer Checklist
Screenshots/VideosAndroid: HybridAppNot Applicable iOS: HybridAppNot Applicable |
…upe tests - Derive the pending join request owner's default avatar URL in createWorkspaceListPoliciesSelector so the email is only hashed when policy data changes, not on every page render. - Remove the duplicated createWorkspaceListPoliciesSelector suite from tests/unit/selectors/PolicyTest.ts and extend the existing projection test in tests/unit/PolicySelectorTest.ts with ownerEmail and the derived ownerDefaultAvatar. - Assert the workspace default avatar test ID in the pending join request UI test. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…uage files and update references in PolicyRulesPage components to use 'importRulesTitle' instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
trjExpensify
left a comment
There was a problem hiding this comment.
Makes sense to fix this to me. 👍
|
@MelvinBot review |
|
🤖 Code review — this is a clean, well-scoped fix. All CI checks are green, and the change correctly addresses all three root causes described. LGTM overall; no blocking issues. What I verified:
Minor, non-blocking observations
Nice work — the selector-level memoization detail and the offline test coverage are both solid. |
|
@marufsharifi still waiting for your final review |
|
@NicolasBonet, I will finish my testing in an hour. thanks. |
|
LGTM. thanks |
|
@stitesExpensify 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] |
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚧 stitesExpensify has triggered a test Expensify/App build. You can view the workflow run here. |
|
🧪🧪 Use the links below to test this adhoc build on Android, iOS, and Web. Happy testing! 🧪🧪
|
|
🚀 Deployed to staging by https://github.com/stitesExpensify in version: 9.4.33-0 🚀
|
|
🤖 Help site review — no changes required. I reviewed the changes in this PR against the help site content under Why: This is a UI bug fix that corrects how existing data is rendered — it does not add, remove, or change any user-facing feature or workflow:
Docs checked: I searched No draft PR was created since there's nothing to change. @NicolasBonet, please confirm you agree no help site changes are required here. If you believe an article should be updated, let me know which behavior to document and I'll open a draft PR. |



Explanation of Change
For a workspace with a pending join request, the owner is usually someone the current user has never interacted with, so their entry is missing from
PERSONAL_DETAILS_LIST. The workspaces list built the owner column exclusively from that list, leaving the Owner cell empty for pending-join rows even though the join request data (policyDetailsForNonMembers) already includes the owner's email and accountID.This PR:
ownerEmailthrough thecreateWorkspaceListPoliciesSelectorprojection (nonMemberDetails).getDefaultAvatarURL({accountID, accountEmail})instead of the generic fallback icon.policy.name, which isundefinedon the parent policy object for these rows — the name lives inpolicy.nonMemberDetails.name.Fixed Issues
$ #94550
PROPOSAL: #94550 (comment)
Tests
Offline tests
QA Steps
Same as Tests.
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectiontoggleReportand notonIconClick)Avatar, 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.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.Screenshots/Videos
MacOS: Chrome / Safari