refactor: change selectors name, fix translation paths, remove unneccessary comments, pull proper data about users domain security groups

Author: jakubstec

src/libs/Navigation/AppNavigator/ModalStackNavigators/index.tsx

@@ -119,7 +119,6 @@ const OPTIONS_PER_SCREEN: Partial<Record<Screen, PlatformStackNavigationOptions>
     [SCREENS.TWO_FACTOR_AUTH.SUCCESS]: {
         animationTypeForReplace: 'push',
     },
-    // Opened via forceReplace from the add domain page, so it needs a forward (push) transition instead of the default reversed (pop) one.
     [SCREENS.WORKSPACES_DOMAIN_ALREADY_EXISTS]: {
         animationTypeForReplace: 'push',
     },

src/pages/domain/AddDomainPage.tsx

@@ -20,7 +20,7 @@ import {getFieldRequiredErrors, isPublicDomain} from '@libs/ValidationUtils';
 
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
-import {hasDomainAccessSelector} from '@src/selectors/Domain';
+import {hasDomainAccess} from '@src/selectors/Domain';
 import {accountIDSelector} from '@src/selectors/Session';
 import INPUT_IDS from '@src/types/form/CreateDomainForm';
 
@@ -35,6 +35,7 @@ function AddDomainPage() {
     const [currentUserAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector});
     const [form] = useOnyx(ONYXKEYS.FORMS.CREATE_DOMAIN_FORM);
     const [allDomains] = useOnyx(ONYXKEYS.COLLECTION.DOMAIN);
+    const [myDomainSecurityGroups] = useOnyx(ONYXKEYS.MY_DOMAIN_SECURITY_GROUPS);
 
     const validate = useCallback(
         (values: FormOnyxValues<typeof ONYXKEYS.FORMS.CREATE_DOMAIN_FORM>) => {
@@ -84,13 +85,13 @@ function AddDomainPage() {
             return;
         }
 
-        if (!!currentUserAccountID && hasDomainAccessSelector(currentUserAccountID)(existingDomain)) {
+        if (!!currentUserAccountID && hasDomainAccess(currentUserAccountID, myDomainSecurityGroups)(existingDomain)) {
             setCreateDomainAlreadyHaveAccessError();
             return;
         }
 
         Navigation.setNavigationActionToMicrotaskQueue(() => Navigation.navigate(ROUTES.WORKSPACES_DOMAIN_ALREADY_EXISTS.getRoute(domainAccountID), {forceReplace: true}));
-    }, [form?.domainAccountID, allDomains, currentUserAccountID]);
+    }, [form?.domainAccountID, allDomains, currentUserAccountID, myDomainSecurityGroups]);
 
     useEffect(() => {
         return () => {

src/pages/domain/DomainAlreadyExistsPage.tsx

@@ -45,12 +45,11 @@ function DomainAlreadyExistsPage({route}: DomainAlreadyExistsPageProps) {
                     description={translate('domain.domainAlreadyExists.description')}
                     descriptionStyle={styles.textSupporting}
                     shouldShowSecondaryButton
-                    secondaryButtonText={translate('domain.domainAlreadyExists.neverMind')}
+                    secondaryButtonText={translate('domain.common.neverMind')}
                     onSecondaryButtonPress={goToDomainsList}
                     shouldShowButton
                     buttonText={translate('domain.domainAlreadyExists.requestAccess')}
                     onButtonPress={() => {
-                        // TODO: call the BE "request domain admin access" endpoint for domainAccountID once it exists.
                         goToDomainsList();
                     }}
                 />

src/pages/domain/DomainsListPage.tsx

@@ -4,7 +4,6 @@ import type {DomainRowData} from '@components/Tables/DomainListTable';
 import DomainListTable from '@components/Tables/DomainListTable';
 import WorkspaceListLayout from '@components/WorkspaceListLayout';
 
-import useCurrentUserPersonalDetails from '@hooks/useCurrentUserPersonalDetails';
 import useDocumentTitle from '@hooks/useDocumentTitle';
 import {useIsAppLoadPending} from '@hooks/useInFlightRequests';
 import {useMemoizedLazyExpensifyIcons} from '@hooks/useLazyAsset';
@@ -20,7 +19,8 @@ import Navigation from '@libs/Navigation/Navigation';
 import CONST from '@src/CONST';
 import ONYXKEYS from '@src/ONYXKEYS';
 import ROUTES from '@src/ROUTES';
-import {hasDomainAccessSelector, isAdminSelector} from '@src/selectors/Domain';
+import {hasDomainAccess, isAdminSelector} from '@src/selectors/Domain';
+import {accountIDSelector} from '@src/selectors/Session';
 import {isEmptyObject} from '@src/types/utils/EmptyObject';
 
 import {Str} from 'expensify-common';
@@ -32,13 +32,14 @@ function DomainsListPage() {
     const {isOffline} = useNetwork();
     const {translate} = useLocalize();
     const icons = useMemoizedLazyExpensifyIcons(['Plus']);
-    const currentUserPersonalDetails = useCurrentUserPersonalDetails();
 
     useDocumentTitle(translate('common.domains'));
 
     const isAppLoadPending = useIsAppLoadPending();
+    const [currentUserAccountID] = useOnyx(ONYXKEYS.SESSION, {selector: accountIDSelector});
     const [allDomains] = useOnyx(ONYXKEYS.COLLECTION.DOMAIN);
     const [allDomainErrors] = useOnyx(ONYXKEYS.COLLECTION.DOMAIN_ERRORS);
+    const [myDomainSecurityGroups] = useOnyx(ONYXKEYS.MY_DOMAIN_SECURITY_GROUPS);
 
     const navigateToDomain = ({domainAccountID, isAdmin}: {domainAccountID: number; isAdmin: boolean}) => {
         if (!isAdmin) {
@@ -57,14 +58,11 @@ function DomainsListPage() {
                 continue;
             }
 
-            const currentUserAccountID = currentUserPersonalDetails?.accountID;
-            const isDomainAdmin = isAdminSelector(currentUserAccountID)(domain);
-
-            // we don't want to show domains that the user has no access to, so we skip them in the list
-            if (!currentUserAccountID || !hasDomainAccessSelector(currentUserAccountID)(domain)) {
+            if (!currentUserAccountID || !hasDomainAccess(currentUserAccountID, myDomainSecurityGroups)(domain)) {
                 continue;
             }
 
+            const isDomainAdmin = isAdminSelector(currentUserAccountID)(domain);
             const domainErrors = allDomainErrors?.[`${ONYXKEYS.COLLECTION.DOMAIN_ERRORS}${domain.accountID}`];
 
             domainRows.push({

src/pages/domain/Groups/DefaultGroupToggle.tsx

@@ -49,7 +49,7 @@ function DefaultGroupToggle({domainAccountID, groupID, groupName}: DefaultGroupT
             title: translate('domain.groups.defaultGroup'),
             prompt: translate('domain.groups.defaultGroupPrompt', defaultSecurityGroup.name, groupName),
             confirmText: translate('domain.groups.makeDefault'),
-            cancelText: translate('domain.groups.neverMind'),
+            cancelText: translate('domain.common.neverMind'),
             shouldShowCancelButton: true,
         });
 

src/pages/domain/Groups/DeleteGroupRow.tsx

@@ -48,7 +48,7 @@ function DeleteGroupRow({domainAccountID, groupID}: DeleteGroupRowProps) {
             title: translate('domain.groups.deleteGroupDangerConfirmationModal'),
             prompt: translate('domain.groups.deleteGroupDangerConfirmationModalDescription', defaultSecurityGroup?.name ?? ''),
             confirmText: translate('domain.groups.deleteGroup'),
-            cancelText: translate('domain.groups.neverMind'),
+            cancelText: translate('domain.common.neverMind'),
             shouldShowCancelButton: true,
             danger: true,
         });

src/selectors/Domain.ts

@@ -213,17 +213,19 @@ function isAdminSelector(accountID: number) {
     };
 }
 
-/**
- * Creates a selector that checks if a given account ID has any access to the domain,
- * either as an admin or as a member of one of its security groups.
- */
-function hasDomainAccessSelector(accountID: number) {
+/** Checks if a given account ID has any access to the domain, as an admin, a security group member, or via `myDomainSecurityGroups`. */
+function hasDomainAccess(accountID: number, myDomainSecurityGroups: OnyxEntry<Record<string, string>>) {
     return (domain: OnyxEntry<Domain>): boolean => {
         if (!domain || !accountID) {
             return false;
         }
 
-        return isAdminSelector(accountID)(domain) || memberAccountIDsSelector(domain).includes(accountID);
+        if (isAdminSelector(accountID)(domain) || memberAccountIDsSelector(domain).includes(accountID)) {
+            return true;
+        }
+
+        const domainName = domainNameSelector(domain);
+        return !!domainName && !!myDomainSecurityGroups?.[domainName];
     };
 }
 
@@ -269,7 +271,7 @@ export {
     vacationDelegateSelector,
     accountLockSelector,
     isAdminSelector,
-    hasDomainAccessSelector,
+    hasDomainAccess,
     selectGroupByID,
     domainSecurityGroupSettingPendingActionSelector,
     domainSecurityGroupSettingErrorsSelector,

tests/unit/DomainSelectorsTest.ts

@@ -14,7 +14,7 @@ import {
     domainSecurityGroupSettingPendingActionSelector,
     domainSettingsPrimaryContactSelector,
     groupsSelector,
-    hasDomainAccessSelector,
+    hasDomainAccess,
     isAdminSelector,
     isSecurityGroupEntry,
     isSecurityGroupPendingDeleteSelector,
@@ -844,19 +844,19 @@ describe('domainSelectors', () => {
         });
     });
 
-    describe('hasDomainAccessSelector', () => {
+    describe('hasDomainAccess', () => {
         it('Should return false if domain is undefined', () => {
-            expect(hasDomainAccessSelector(userID1)(undefined)).toBe(false);
+            expect(hasDomainAccess(userID1, undefined)(undefined)).toBe(false);
         });
 
         it('Should return false if accountID is 0', () => {
             const domain = createDomainFixture({admins: [['123456', userID1]]});
-            expect(hasDomainAccessSelector(0)(domain)).toBe(false);
+            expect(hasDomainAccess(0, undefined)(domain)).toBe(false);
         });
 
         it('Should return true if the accountID is an admin', () => {
             const domain = createDomainFixture({admins: [['123456', userID1]]});
-            expect(hasDomainAccessSelector(userID1)(domain)).toBe(true);
+            expect(hasDomainAccess(userID1, undefined)(domain)).toBe(true);
         });
 
         it('Should return true if the accountID is a member of a security group', () => {
@@ -870,7 +870,7 @@ describe('domainSelectors', () => {
                 },
             });
 
-            expect(hasDomainAccessSelector(userID1)(domain)).toBe(true);
+            expect(hasDomainAccess(userID1, undefined)(domain)).toBe(true);
         });
 
         it('Should return false if the accountID is neither an admin nor a member', () => {
@@ -885,12 +885,28 @@ describe('domainSelectors', () => {
                 },
             });
 
-            expect(hasDomainAccessSelector(userID1)(domain)).toBe(false);
+            expect(hasDomainAccess(userID1, undefined)(domain)).toBe(false);
         });
 
         it('Should return false for empty domain object', () => {
             const domain = createDomainFixture({empty: true});
-            expect(hasDomainAccessSelector(userID1)(domain)).toBe(false);
+            expect(hasDomainAccess(userID1, undefined)(domain)).toBe(false);
+        });
+
+        it("Should return true if the domain name is present in the account's own myDomainSecurityGroups map, even without admin/security-group data on the domain entry", () => {
+            // Mirrors the payload a non-admin member's own `domain_<id>` entry actually looks like: no
+            // `expensify_adminPermissions_*` nor `domain_securityGroup_*` data, since only admins receive that.
+            const domain = createDomainFixture({email: 'member@example.com'});
+            const myDomainSecurityGroups = {'example.com': 'groupID123'};
+
+            expect(hasDomainAccess(userID1, myDomainSecurityGroups)(domain)).toBe(true);
+        });
+
+        it("Should return false if the domain name is not present in the account's own myDomainSecurityGroups map", () => {
+            const domain = createDomainFixture({email: 'member@example.com'});
+            const myDomainSecurityGroups = {'other-domain.com': 'groupID123'};
+
+            expect(hasDomainAccess(userID1, myDomainSecurityGroups)(domain)).toBe(false);
         });
     });