Skip to content

Commit

Permalink
refactor(HMS-4230): rbac out of appcontext
Browse files Browse the repository at this point in the history
This change remove rbac from the appcontext, and
re-implement the rbac hook with a different approach
because the previous hook was mutating continuously
and evoking a refresh continuously. Now it leverages
a useMemo so only when it is required is updated the
rbac permissions.

Propagate the changes along the different pages.

Signed-off-by: Alejandro Visiedo <[email protected]>
Co-authored-by: Petr Vobornik <[email protected]>
  • Loading branch information
avisiedo and pvoborni committed Jun 4, 2024
1 parent 0824d14 commit 90d6335
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 39 deletions.
4 changes: 0 additions & 4 deletions src/AppContext.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { ReactNode, createContext, useState } from 'react';
import { Domain } from './Api/idmsvc';
import { VerifyState } from './Routes/WizardPage/Components/VerifyRegistry/VerifyRegistry';
import React from 'react';
import useIdmPermissions, { IdmPermissions } from './Hooks/useIdmPermissions';

/**
* It represents the application context so common events and properties
Expand Down Expand Up @@ -51,7 +50,6 @@ export interface AppContextType {
/** Set the ephemeral domain information. */
setDomain: (value: Domain) => void;
};
rbac: IdmPermissions;
}

/**
Expand All @@ -78,7 +76,6 @@ export const AppContext = createContext<AppContextType>({
domain: {} as Domain,
setDomain: () => undefined,
},
rbac: {} as IdmPermissions,
});

/**
Expand Down Expand Up @@ -182,7 +179,6 @@ export const AppContextProvider: React.FC<AppContextProviderProps> = ({ children
domain: wizardDomain || ({} as Domain),
setDomain: _setWizardDomain,
},
rbac: useIdmPermissions(),
}}
>
{children}
Expand Down
58 changes: 38 additions & 20 deletions src/Hooks/useIdmPermissions.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { usePermissions } from '@redhat-cloud-services/frontend-components-utilities/RBACHook';
import { useEffect, useMemo, useState } from 'react';
import { UsePermissionsState, doesHavePermissions, getRBAC } from '@redhat-cloud-services/frontend-components-utilities/RBAC';

const APP = 'idmsvc';

Expand All @@ -24,25 +25,42 @@ const useIdmPermissions = (): IdmPermissions => {
const domainsDelete = APP + ':domains:delete';
const domainsList = APP + ':domains:list';

const { hasAccess: hasTokensCreate, isLoading: isLoadingTokensCreate } = usePermissions(APP, [tokenCreate], true, true);
const { hasAccess: hasDomainsRead, isLoading: isLoadingDomainsRead } = usePermissions(APP, [domainsRead], true, true);
const { hasAccess: hasDomainsUpdate, isLoading: isLoadingDomainsUpdate } = usePermissions(APP, [domainsUpdate], true, true);
const { hasAccess: hasDomainsDelete, isLoading: isLoadingDomainsDelete } = usePermissions(APP, [domainsDelete], true, true);
const { hasAccess: hasDomainsList, isLoading: isLoadingDomainsList } = usePermissions(APP, [domainsList], true, true);

const isLoading: boolean =
isLoadingTokensCreate || isLoadingDomainsRead || isLoadingDomainsUpdate || isLoadingDomainsDelete || isLoadingDomainsList;

return {
isLoading: isLoading,
permissions: {
hasTokenCreate: hasTokensCreate,
hasDomainsRead: hasDomainsRead,
hasDomainsList: hasDomainsList,
hasDomainsUpdate: hasDomainsUpdate,
hasDomainsDelete: hasDomainsDelete,
},
};
const [permissions, setPermissions] = useState<UsePermissionsState>({
isLoading: true,
hasAccess: false,
isOrgAdmin: false,
permissions: [],
});

useEffect(() => {
(async () => {
// setPermissions((prev) => ({ ...prev, isLoading: true }));

const { isOrgAdmin, permissions: userPermissions } = await getRBAC(APP);

setPermissions({
isLoading: false,
isOrgAdmin,
permissions: userPermissions,
hasAccess: false,
});
})();
}, []);

const idmPermissions = useMemo(() => {
return {
isLoading: permissions.isLoading,
permissions: {
hasTokenCreate: doesHavePermissions(permissions.permissions, [tokenCreate], false),
hasDomainsRead: doesHavePermissions(permissions.permissions, [domainsRead], false),
hasDomainsList: doesHavePermissions(permissions.permissions, [domainsList], false),
hasDomainsUpdate: doesHavePermissions(permissions.permissions, [domainsUpdate], false),
hasDomainsDelete: doesHavePermissions(permissions.permissions, [domainsDelete], false),
},
};
}, [permissions]);

return idmPermissions;
};

export default useIdmPermissions;
10 changes: 6 additions & 4 deletions src/Routes/DefaultPage/DefaultPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { Domain, ResourcesApiFactory } from '../../Api/idmsvc';
import { DomainList } from '../../Components/DomainList/DomainList';
import { AppContext } from '../../AppContext';
import CenteredSpinner from '../../Components/CenteredSpinner/CenteredSpinner';
import useIdmPermissions from '../../Hooks/useIdmPermissions';

const Header = () => {
const linkLearnMoreAbout = 'https://access.redhat.com/articles/1586893';
Expand Down Expand Up @@ -186,15 +187,16 @@ const DefaultPage = () => {
const offset = (page - 1) * perPage;
const [isLoading, setIsLoading] = useState<boolean>(true);
const navigate = useNavigate();
const rbac = useIdmPermissions();

console.log('INFO:DefaultPage render');

useEffect(() => {
if (appContext.rbac.isLoading) {
if (rbac.isLoading) {
setIsLoading(true);
return;
}
if (!appContext.rbac.permissions.hasDomainsList) {
if (!rbac.permissions.hasDomainsList) {
console.error('rbac no list permission');
navigate('/no-permissions', { replace: true });
return;
Expand All @@ -213,7 +215,7 @@ const DefaultPage = () => {
console.log(reason);
setIsLoading(false);
});
}, [page, perPage, appContext.rbac.isLoading]);
}, [page, perPage, rbac]);

const changePageSize = (size: number) => {
setPerPage(size);
Expand Down Expand Up @@ -241,7 +243,7 @@ const DefaultPage = () => {
</>
);

if (appContext.rbac.isLoading || isLoading) {
if (rbac.isLoading || isLoading) {
return loadingContent;
}
const content = appContext.totalDomains <= 0 ? emptyContent : listContent;
Expand Down
24 changes: 16 additions & 8 deletions src/Routes/DetailPage/DetailPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ import { DetailServers } from './Components/DetailServers/DetailServers';
import ConfirmDeleteDomain from '../../Components/ConfirmDeleteDomain/ConfirmDeleteDomain';
import useNotification from '../../Hooks/useNotification';
import { buildDeleteFailedNotification, buildDeleteSuccessNotification } from './detailNotifications';
import useIdmPermissions from '../../Hooks/useIdmPermissions';
import CenteredSpinner from '../../Components/CenteredSpinner/CenteredSpinner';

/**
Expand All @@ -41,25 +42,32 @@ const DetailPage = () => {
const resources_api = ResourcesApiFactory(undefined, base_url, undefined);
const navigate = useNavigate();
const { notifyError, notifySuccess } = useNotification();
const rbac = useIdmPermissions();

// Params
const { domain_id } = useParams();

if (domain_id === undefined) {
navigate('/domains', { replace: true });
return <></>;
}
useEffect(() => {
if (domain_id === undefined) {
navigate('/domains', { replace: true });
}
}, [domain_id]);

// States
const [domain, setDomain] = useState<Domain | undefined>(appContext?.getDomain(domain_id) || undefined);
const [domain, setDomain] = useState<Domain | undefined>(appContext?.getDomain(domain_id as string) || undefined);
const [isOpenConfirmDelete, setIsOpenConfirmDelete] = useState<boolean>(false);

console.log('INFO:DetailPage render:domain_id=' + domain_id);

// Load Domain to display
useEffect(() => {
if (!appContext.rbac.permissions.hasDomainsRead) {
if (rbac.isLoading) {
return;
}

if (!rbac.permissions.hasDomainsRead) {
navigate('/no-permissions', { replace: true });
return;
}
if (domain_id) {
resources_api
Expand All @@ -76,7 +84,7 @@ const DetailPage = () => {
navigate('/domains', { replace: true });
});
}
}, [domain_id, appContext.rbac.isLoading]);
}, [domain_id, rbac]);

// Kebab menu
const [isKebabOpen, setIsKebabOpen] = useState<boolean>(false);
Expand Down Expand Up @@ -119,7 +127,7 @@ const DetailPage = () => {
setActiveTabKey(tabIndex);
};

if (appContext.rbac.isLoading) {
if (rbac.isLoading) {
return <CenteredSpinner />;
}

Expand Down
8 changes: 5 additions & 3 deletions src/Routes/WizardPage/WizardPage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import PageServiceRegistration from './Components/PageServiceRegistration/PageSe
import PageServiceDetails from './Components/PageServiceDetails/PageServiceDetails';
import PageReview from './Components/PageReview/PageReview';
import CenteredSpinner from '../../Components/CenteredSpinner/CenteredSpinner';
import useIdmPermissions from '../../Hooks/useIdmPermissions';

/**
* Wizard page to register a new domain into the service.
Expand All @@ -39,17 +40,18 @@ const WizardPage = () => {
const navigate = useNavigate();
const { notifySuccess, notifyWarning, notifyError, removeNotification } = useNotification();
const [isCancelConfirmationModalOpen, SetIsCancelConfirmationModalOpen] = useState<boolean>(false);
const rbac = useIdmPermissions();

// FIXME Update the URL with the location for docs
const linkLearnMoreAbout = 'https://access.redhat.com/articles/1586893';
const linkLearnMoreAboutRemovingDirectoryAndDomainServices = 'https://access.redhat.com/articles/1586893';

useEffect(() => {
if (!appContext.rbac.permissions.hasTokenCreate || !appContext.rbac.permissions.hasDomainsUpdate) {
if (!rbac.isLoading && (!rbac.permissions.hasTokenCreate || !rbac.permissions.hasDomainsUpdate)) {
navigate('/no-permissions', { replace: true });
return;
}
}, [appContext.rbac.isLoading]);
}, [rbac]);

const notifyNotCompleted = () => {
const notificationID = 'domain-registration-cancelled-notification';
Expand Down Expand Up @@ -236,7 +238,7 @@ const WizardPage = () => {

const title = 'Register identity domain';

if (appContext.rbac.isLoading) {
if (rbac.isLoading) {
return <CenteredSpinner />;
}

Expand Down

0 comments on commit 90d6335

Please sign in to comment.