Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions src/pages/workspace/WorkspacesListPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,19 @@ function WorkspacesListPage() {

for (const policy of workspaceListPolicies ?? []) {
if (policy.isJoinRequestPending && policy.nonMemberDetails) {
const {policyID, ownerAccountID} = policy.nonMemberDetails;
const ownerDetails = ownerAccountID ? ownerDisplayDetails?.[ownerAccountID] : undefined;
const {policyID, ownerAccountID, ownerEmail, ownerDefaultAvatar} = policy.nonMemberDetails;
let ownerDetails = ownerAccountID ? ownerDisplayDetails?.[ownerAccountID] : undefined;

// The owner of a policy the user only requested to join is usually not in the personal details list,
// so fall back to the owner email and default avatar the join request already provides.
if (!ownerDetails && ownerAccountID && ownerEmail) {
ownerDetails = {
accountID: ownerAccountID,
login: ownerEmail,
displayName: ownerEmail,
avatar: ownerDefaultAvatar,
};
}

const pendingWorkspaceRow: WorkspaceRowData = {
keyForList: policyID,
Expand All @@ -121,7 +132,7 @@ function WorkspacesListPage() {
ownerAvatar: ownerDetails ? ownerDetails.avatar : undefined,
ownerName: ownerDetails ? temporaryGetDisplayNameOrDefault({passedPersonalDetails: ownerDetails, translate}) : undefined,
iconType: policy.nonMemberDetails.avatar ? CONST.ICON_TYPE_AVATAR : CONST.ICON_TYPE_ICON,
icon: policy.nonMemberDetails.avatar ? policy.nonMemberDetails.avatar : getDefaultWorkspaceAvatar(policy.name),
icon: policy.nonMemberDetails.avatar ? policy.nonMemberDetails.avatar : getDefaultWorkspaceAvatar(policy.nonMemberDetails.name),
action: () => null,
dismissError: () => null,
};
Expand Down
11 changes: 10 additions & 1 deletion src/selectors/Policy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {hasSynchronizationErrorMessage, isConnectionUnverified} from '@libs/acti
import {getDisplayNameForWorkspace} from '@libs/actions/Policy/Policy';
// eslint-disable-next-line no-restricted-imports -- isPaidGroupPolicy is intentional: copy-settings targets are billing/paid-only (Collect/Control), so free group plans like Submit must be excluded (see createCopySettingsEligibleTargetsSelector).
import {getActiveAdminWorkspaces, getOwnedPaidPolicies, isPaidGroupPolicy, isPendingDeletePolicy, isPolicyAdmin, shouldShowPolicy} from '@libs/PolicyUtils';
import {getDefaultAvatarURL} from '@libs/UserAvatarUtils';

import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -67,7 +68,12 @@ type WorkspaceListPolicy = Pick<Policy, 'id' | 'name' | 'type' | 'role' | 'owner
isJoinRequestPending: boolean;

/** Projection of policyDetailsForNonMembers for join-request-pending policies */
nonMemberDetails?: Pick<PolicyDetailsForNonMembers, 'name' | 'type' | 'ownerAccountID' | 'avatar'> & {policyID: string};
nonMemberDetails?: Pick<PolicyDetailsForNonMembers, 'name' | 'type' | 'ownerAccountID' | 'ownerEmail' | 'avatar'> & {
policyID: string;

/** Default avatar URL for the owner, derived here so the page doesn't re-hash the email on every render */
ownerDefaultAvatar?: string;
};
};

/**
Expand Down Expand Up @@ -95,6 +101,9 @@ const createWorkspaceListPoliciesSelector =
name: details.name,
type: details.type,
ownerAccountID: details.ownerAccountID,
ownerEmail: details.ownerEmail,
ownerDefaultAvatar:
details.ownerAccountID && details.ownerEmail ? getDefaultAvatarURL({accountID: details.ownerAccountID, accountEmail: details.ownerEmail}) : undefined,
avatar: details.avatar,
};
}
Expand Down
30 changes: 30 additions & 0 deletions tests/ui/WorkspaceListPageTest.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import type {WorkspaceNavigatorParamList} from '@libs/Navigation/types';

import WorkspacesListPage from '@pages/workspace/WorkspacesListPage';

import CONST from '@src/CONST';
import ONYXKEYS from '@src/ONYXKEYS';
import SCREENS from '@src/SCREENS';

Expand Down Expand Up @@ -135,6 +136,35 @@ describe('WorkspaceListPage', () => {
expect(newWorkspaceButton).toBeOnTheScreen();
});

it('should show the owner email of a pending join request workspace when the owner is not in the personal details list', async () => {
const TEST_POLICY_ID = 'pending-policy-id';
const OWNER_EMAIL = 'owner@example.com';

await Onyx.set(`${ONYXKEYS.COLLECTION.POLICY}${TEST_POLICY_ID}`, {
id: TEST_POLICY_ID,
isJoinRequestPending: true,
policyDetailsForNonMembers: {
[TEST_POLICY_ID]: {
name: 'Pending Workspace',
type: CONST.POLICY.TYPE.TEAM,
ownerAccountID: 42,
ownerEmail: OWNER_EMAIL,
},
},
});

renderPage();

await waitForBatchedUpdatesWithAct();

expect(screen.getByText('Pending Workspace')).toBeOnTheScreen();
expect(screen.getByText(new RegExp(OWNER_EMAIL))).toBeOnTheScreen();
Comment on lines +160 to +161

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

// The workspace default icon is derived from nonMemberDetails.name ("Pending Workspace"), so the
// rendered SVG test ID is keyed by its first alphanumeric character. The icon is decorative and
// hidden from accessibility, so the query must include hidden elements.
expect(screen.getByTestId('SvgDefaultAvatar_p Icon', {includeHiddenElements: true})).toBeOnTheScreen();
});

it('should show a "New workspace" button when there are workspaces but no domains', async () => {
const TEST_POLICY_ID = 'test-policy-id';

Expand Down
5 changes: 5 additions & 0 deletions tests/unit/PolicySelectorTest.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {getDefaultAvatarURL} from '@libs/UserAvatarUtils';

import CONST from '@src/CONST';
import IntlStore from '@src/languages/IntlStore';
import ONYXKEYS from '@src/ONYXKEYS';
Expand Down Expand Up @@ -337,6 +339,7 @@ describe('createWorkspaceListPoliciesSelector', () => {
name: 'External WS',
type: CONST.POLICY.TYPE.CORPORATE,
ownerAccountID: 99,
ownerEmail: 'owner@example.com',
avatar: 'https://img/ext.png',
},
},
Expand All @@ -349,6 +352,8 @@ describe('createWorkspaceListPoliciesSelector', () => {
name: 'External WS',
type: CONST.POLICY.TYPE.CORPORATE,
ownerAccountID: 99,
ownerEmail: 'owner@example.com',
ownerDefaultAvatar: getDefaultAvatarURL({accountID: 99, accountEmail: 'owner@example.com'}),
avatar: 'https://img/ext.png',
});
});
Expand Down
Loading