Skip to content

Commit 4910f7f

Browse files
committed
fix case when there may be leftover group key
1 parent 09dbb22 commit 4910f7f

3 files changed

Lines changed: 38 additions & 2 deletions

File tree

src/pages/domain/Members/MoveUserBetweenGroupsPage.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ function MoveUserBetweenGroupsPage({route}: MoveUserBetweenGroupsPageProps) {
3939
const [domainName] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: domainNameSelector});
4040
const [securityGroups] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {selector: groupsSelector});
4141

42-
const securityGroupSelector = useCallback((domain: OnyxEntry<Domain>) => selectSecurityGroupForAccount(accountID)(domain), [accountID]);
42+
const securityGroupSelector = (domain: OnyxEntry<Domain>) => selectSecurityGroupForAccount(accountID)(domain);
4343
const [userSecurityGroup] = useOnyx(`${ONYXKEYS.COLLECTION.DOMAIN}${domainAccountID}`, {
4444
selector: securityGroupSelector,
4545
});

src/selectors/Domain.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -133,7 +133,7 @@ function selectSecurityGroupForAccount(accountID: number) {
133133

134134
const [key, group] = entry;
135135

136-
if (group.shared && accountIDStr in group.shared) {
136+
if (group.shared?.[accountIDStr] != null) {
137137
return {
138138
key,
139139
securityGroup: group,

tests/unit/DomainSelectorsTest.ts

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -388,6 +388,42 @@ describe('domainSelectors', () => {
388388
securityGroup: group1,
389389
});
390390
});
391+
392+
it('Should skip a group whose shared entry for the account is a null tombstone and return the active group', () => {
393+
// After changeDomainSecurityGroup fires optimistically, the old group gets
394+
// shared[accountID] = null while the new group gets shared[accountID] = 'read'.
395+
// The selector must skip the tombstone and return the new (active) group.
396+
const key1 = `${CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}1`;
397+
const key2 = `${CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}2`;
398+
399+
const domain: Domain = {
400+
validated: true,
401+
accountID: 1,
402+
email: 'test@example.com',
403+
// eslint-disable-next-line @typescript-eslint/naming-convention
404+
[key1]: {shared: {'123': null}, enableRestrictedPrimaryLogin: false, enableRestrictedPolicyCreation: false},
405+
// eslint-disable-next-line @typescript-eslint/naming-convention
406+
[key2]: {shared: {'123': 'read'}, enableRestrictedPrimaryLogin: false, enableRestrictedPolicyCreation: false},
407+
} as unknown as Domain;
408+
409+
const result = selectSecurityGroupForAccount(123)(domain);
410+
411+
expect(result?.key).toBe(key2);
412+
});
413+
414+
it('Should return undefined when the only matching shared entry is a null tombstone', () => {
415+
const key1 = `${CONST.DOMAIN.DOMAIN_SECURITY_GROUP_PREFIX}1`;
416+
417+
const domain: Domain = {
418+
validated: true,
419+
accountID: 1,
420+
email: 'test@example.com',
421+
// eslint-disable-next-line @typescript-eslint/naming-convention
422+
[key1]: {shared: {'123': null}, enableRestrictedPrimaryLogin: false, enableRestrictedPolicyCreation: false},
423+
} as unknown as Domain;
424+
425+
expect(selectSecurityGroupForAccount(123)(domain)).toBeUndefined();
426+
});
391427
});
392428

393429
describe('isSecurityGroupEntry', () => {

0 commit comments

Comments
 (0)