Skip to content

Commit

Permalink
fix: cleaning up sso self service wording and behaviors
Browse files Browse the repository at this point in the history
  • Loading branch information
alex-sheehan-edx committed Nov 20, 2023
1 parent bd8455b commit 37de1eb
Show file tree
Hide file tree
Showing 10 changed files with 244 additions and 104 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const FRESH_CONFIG_POLLING_INTERVAL = 30000;
const UPDATED_CONFIG_POLLING_INTERVAL = 2000;

const NewExistingSSOConfigs = ({
configs, refreshBool, setRefreshBool, enterpriseId, setPollingNetworkError,
configs, refreshBool, setRefreshBool, enterpriseId, setPollingNetworkError, setIsStepperOpen,
}) => {
const [inactiveConfigs, setInactiveConfigs] = useState([]);
const [activeConfigs, setActiveConfigs] = useState([]);
Expand Down Expand Up @@ -60,6 +60,7 @@ const NewExistingSSOConfigs = ({
setRefreshBool={setRefreshBool}
refreshBool={refreshBool}
setUpdateError={setUpdateError}
setIsStepperOpen={setIsStepperOpen}
/>
{updateError?.config === config.uuid && (
<div>
Expand Down Expand Up @@ -197,6 +198,7 @@ NewExistingSSOConfigs.propTypes = {
setRefreshBool: PropTypes.func.isRequired,
enterpriseId: PropTypes.string.isRequired,
setPollingNetworkError: PropTypes.func.isRequired,
setIsStepperOpen: PropTypes.func.isRequired,
};

const mapStateToProps = state => ({
Expand Down
11 changes: 9 additions & 2 deletions src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const NewSSOConfigCard = ({
setRefreshBool,
refreshBool,
setUpdateError,
setIsStepperOpen,
}) => {
const VALIDATED = config.validated_at;
const ENABLED = config.active;
Expand All @@ -23,6 +24,11 @@ const NewSSOConfigCard = ({

const { setProviderConfig } = useContext(SSOConfigContext);

const onConfigureClick = (selectedConfig) => {
setProviderConfig(selectedConfig);
setIsStepperOpen(true);

Check warning on line 29 in src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx#L28-L29

Added lines #L28 - L29 were not covered by tests
};

const convertToReadableDate = (date) => {
const dateObj = new Date(date);
const options = { year: 'numeric', month: 'short', day: 'numeric' };
Expand Down Expand Up @@ -127,7 +133,7 @@ const NewSSOConfigCard = ({
{!VALIDATED && CONFIGURED && (
<Button
className="float-right"
onClick={() => setProviderConfig(config)}
onClick={() => onConfigureClick(config)}

Check warning on line 136 in src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx#L136

Added line #L136 was not covered by tests
variant="outline-primary"
data-testid="existing-sso-config-card-configure-button"
>
Expand Down Expand Up @@ -181,7 +187,7 @@ const NewSSOConfigCard = ({
{VALIDATED && (
<Dropdown.Item
data-testid="existing-sso-config-configure-dropdown"
onClick={() => setProviderConfig(config)}
onClick={() => onConfigureClick(config)}

Check warning on line 190 in src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/settings/SettingsSSOTab/NewSSOConfigCard.jsx#L190

Added line #L190 was not covered by tests
>
Configure
</Dropdown.Item>
Expand Down Expand Up @@ -224,6 +230,7 @@ NewSSOConfigCard.propTypes = {
setRefreshBool: PropTypes.func.isRequired,
refreshBool: PropTypes.bool.isRequired,
setUpdateError: PropTypes.func.isRequired,
setIsStepperOpen: PropTypes.func.isRequired,
};

export default NewSSOConfigCard;
64 changes: 39 additions & 25 deletions src/components/settings/SettingsSSOTab/NewSSOConfigForm.jsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import React, { useContext } from 'react';
import PropTypes from 'prop-types';
import { Alert, Hyperlink } from '@edx/paragon';
import { WarningFilled } from '@edx/paragon/icons';
import { SSOConfigContext } from './SSOConfigContext';
Expand All @@ -7,38 +8,51 @@ import { HELP_CENTER_SAML_LINK } from '../data/constants';
import { features } from '../../../config';
import NewSSOStepper from './NewSSOStepper';

const NewSSOConfigForm = () => {
const NewSSOConfigForm = ({ setIsStepperOpen, isStepperOpen }) => {
const { ssoState: { currentError } } = useContext(SSOConfigContext);
const { AUTH0_SELF_SERVICE_INTEGRATION } = features;

return (
<div className="sso-create-form mt-4.5">
<span>
Connect to a SAML identity provider for single sign-on
to allow quick access to your organization&apos;s learning catalog.
</span>
{AUTH0_SELF_SERVICE_INTEGRATION ? <NewSSOStepper /> : <SSOStepper />}
{!AUTH0_SELF_SERVICE_INTEGRATION && (
<span>
Connect to a SAML identity provider for single sign-on
to allow quick access to your organization&apos;s learning catalog.
</span>
)}
{AUTH0_SELF_SERVICE_INTEGRATION ? (
<NewSSOStepper isStepperOpen={isStepperOpen} setIsStepperOpen={setIsStepperOpen} />
) : (
<SSOStepper />
)}
{currentError && (
<Alert
variant="warning"
stacked
icon={WarningFilled}
actions={[
<Hyperlink
destination={HELP_CENTER_SAML_LINK}
className="btn btn-sm btn-primary"
target="_blank"
>
Help Center
</Hyperlink>,
]}
>
<Alert.Heading>Something went wrong.</Alert.Heading>
<p className="my-3">
{currentError}
</p>
</Alert>
<Alert

Check warning on line 29 in src/components/settings/SettingsSSOTab/NewSSOConfigForm.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/settings/SettingsSSOTab/NewSSOConfigForm.jsx#L29

Added line #L29 was not covered by tests
variant="warning"
stacked
icon={WarningFilled}
actions={[
<Hyperlink
destination={HELP_CENTER_SAML_LINK}
className="btn btn-sm btn-primary"
target="_blank"
>
Help Center
</Hyperlink>,
]}
>
<Alert.Heading>Something went wrong.</Alert.Heading>
<p className="my-3">
{currentError}
</p>
</Alert>
)}
</div>
);
};

NewSSOConfigForm.propTypes = {
setIsStepperOpen: PropTypes.func.isRequired,
isStepperOpen: PropTypes.bool.isRequired,
};

export default NewSSOConfigForm;
7 changes: 4 additions & 3 deletions src/components/settings/SettingsSSOTab/NewSSOStepper.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,11 @@ import { camelCaseDict } from '../../../utils';
import UnsavedSSOChangesModal from './UnsavedSSOChangesModal';
import { IDP_URL_SELECTION, IDP_XML_SELECTION } from './steps/NewSSOConfigConnectStep';

const NewSSOStepper = ({ enterpriseId }) => {
const NewSSOStepper = ({ enterpriseId, isStepperOpen, setIsStepperOpen }) => {
const {
setProviderConfig, refreshBool, setRefreshBool, ssoState: { providerConfig },
setProviderConfig, setRefreshBool, ssoState: { providerConfig, refreshBool },
} = useContext(SSOConfigContext);
const providerConfigCamelCase = camelCaseDict(providerConfig || {});
const [isStepperOpen, setIsStepperOpen] = useState(true);
const [configureError, setConfigureError] = useState(null);
const handleCloseWorkflow = () => {
setProviderConfig?.(null);
Expand Down Expand Up @@ -52,6 +51,8 @@ const NewSSOStepper = ({ enterpriseId }) => {

NewSSOStepper.propTypes = {
enterpriseId: PropTypes.string.isRequired,
isStepperOpen: PropTypes.bool.isRequired,
setIsStepperOpen: PropTypes.func.isRequired,
};

const mapStateToProps = state => ({
Expand Down
43 changes: 26 additions & 17 deletions src/components/settings/SettingsSSOTab/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ const SettingsSSOTab = ({ enterpriseId, setHasSSOConfig }) => {
const { AUTH0_SELF_SERVICE_INTEGRATION } = features;
const [isOpen, open, close] = useToggle(false);
const [pollingNetworkError, setPollingNetworkError] = useState(false);
const [isStepperOpen, setIsStepperOpen] = useState(true);

const newConfigurationButtonOnClick = async () => {
Promise.all(existingConfigs.map(config => LmsApiService.updateEnterpriseSsoOrchestrationRecord(
Expand Down Expand Up @@ -129,6 +130,7 @@ const SettingsSSOTab = ({ enterpriseId, setHasSSOConfig }) => {
refreshBool={refreshBool}
setRefreshBool={setRefreshBool}
setPollingNetworkError={setPollingNetworkError}
setIsStepperOpen={setIsStepperOpen}
/>
)}
{/* Nothing found so guide user to creation/edit form */}
Expand All @@ -137,11 +139,16 @@ const SettingsSSOTab = ({ enterpriseId, setHasSSOConfig }) => {
)}
{/* Since we found a selected providerConfig we know we are in editing mode and can safely
render the create/edit form */}
{((existingConfigs?.length > 0 && providerConfig !== null) || showNewSSOForm) && (<NewSSOConfigForm />)}
{((existingConfigs?.length > 0 && providerConfig !== null) || showNewSSOForm) && (
<NewSSOConfigForm

Check warning on line 143 in src/components/settings/SettingsSSOTab/index.jsx

View check run for this annotation

Codecov / codecov/patch

src/components/settings/SettingsSSOTab/index.jsx#L143

Added line #L143 was not covered by tests
setIsStepperOpen={setIsStepperOpen}
isStepperOpen={isStepperOpen}
/>
)}
{pdError && (
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML data: <p>{pdError?.message}</p>
</Alert>
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML data: <p>{pdError?.message}</p>
</Alert>
)}
<Toast
onClose={() => setInfoMessage(null)}
Expand Down Expand Up @@ -178,27 +185,29 @@ const SettingsSSOTab = ({ enterpriseId, setHasSSOConfig }) => {
existing configs but no providerConfig then we can safely render the listings page */}
{existingConfigs?.length > 0 && (providerConfig === null)
&& (
<ExistingSSOConfigs
providerData={existingProviderData}
configs={existingConfigs}
refreshBool={refreshBool}
setRefreshBool={setRefreshBool}
/>
<ExistingSSOConfigs
providerData={existingProviderData}
configs={existingConfigs}
refreshBool={refreshBool}
setRefreshBool={setRefreshBool}
/>
)}
{/* Nothing found so guide user to creation/edit form */}
{showNoSSOCard && <NoSSOCard setShowNoSSOCard={setShowNoSSOCard} setShowNewSSOForm={setShowNewSSOForm} />}
{/* Since we found a selected providerConfig we know we are in editing mode and can safely
render the create/edit form */}
{((existingConfigs?.length > 0 && providerConfig !== null) || showNewSSOForm) && (<NewSSOConfigForm />)}
{((existingConfigs?.length > 0 && providerConfig !== null) || showNewSSOForm) && (
<NewSSOConfigForm setIsStepperOpen={setIsStepperOpen} isStepperOpen={isStepperOpen} />
)}
{error && (
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML configs: <p>{error?.message}</p>
</Alert>
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML configs: <p>{error?.message}</p>
</Alert>
)}
{pdError && (
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML data: <p>{pdError?.message}</p>
</Alert>
<Alert variant="warning" icon={WarningFilled}>
An error occurred loading the SAML data: <p>{pdError?.message}</p>
</Alert>
)}
{infoMessage && (
<Toast
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const SSOConfigIDPStep = ({ enterpriseId }) => {
const {
metadataURL, entityID, handleMetadataURLUpdate, handleEntityIDUpdate,
} = useIdpState();
const { refreshBool } = useContext(SSOConfigContext);
const { ssoState: { refreshBool } } = useContext(SSOConfigContext);
const [existingProviderData] = useExistingProviderData(enterpriseId, refreshBool);

const TITLE = 'First provide your Identity Provider Metadata and fill out the corresponding fields.';
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
import React from 'react';
import { render, screen, waitFor } from '@testing-library/react';
import {
act,
render,
screen,
waitFor,
} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import '@testing-library/jest-dom/extend-expect';
import configureMockStore from 'redux-mock-store';
Expand Down Expand Up @@ -104,12 +109,15 @@ describe('<ExistingSSOConfigs />', () => {
);
expect(screen.getByText('nacho cheese')).toBeInTheDocument();
expect(screen.getByText('Inactive')).toBeInTheDocument();

userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${inactiveConfig[0].id}`));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${inactiveConfig[0].id}`));
});
expect(screen.getByText('Enable')).toBeInTheDocument();
expect(screen.getByText('Edit')).toBeInTheDocument();

userEvent.click(screen.getByText('Enable'));
act(() => {
userEvent.click(screen.getByText('Enable'));
});
const data = new FormData();
data.append('enabled', true);
data.append('enterprise_customer_uuid', enterpriseId);
Expand All @@ -129,7 +137,9 @@ describe('<ExistingSSOConfigs />', () => {
);
expect(screen.getByText('bbq')).toBeInTheDocument();
expect(screen.getByText('Incomplete')).toBeInTheDocument();
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
});
await waitFor(() => {
expect(screen.getByText('Delete')).toBeInTheDocument();
expect(screen.getByText('Edit')).toBeInTheDocument();
Expand Down Expand Up @@ -162,8 +172,12 @@ describe('<ExistingSSOConfigs />', () => {
/>
</Provider>,
);
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
userEvent.click(screen.getByText('Delete'));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
});
act(() => {
userEvent.click(screen.getByText('Delete'));
});
expect(LmsApiService.deleteProviderConfig).toHaveBeenCalledWith(incompleteConfig[0].id, enterpriseId);
});
it('properly handles errors when deleting provider data', () => {
Expand All @@ -182,8 +196,12 @@ describe('<ExistingSSOConfigs />', () => {
/>
</Provider>,
);
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
userEvent.click(screen.getByText('Delete'));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
});
act(() => {
userEvent.click(screen.getByText('Delete'));
});
expect(handleErrors).toHaveBeenCalled();
});
it('properly handles errors when deleting provider configs', () => {
Expand All @@ -202,8 +220,12 @@ describe('<ExistingSSOConfigs />', () => {
/>
</Provider>,
);
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
userEvent.click(screen.getByText('Delete'));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
});
act(() => {
userEvent.click(screen.getByText('Delete'));
});
expect(handleErrors).toHaveBeenCalled();
});
it('properly displays error message when deleting provider configs', async () => {
Expand All @@ -223,8 +245,12 @@ describe('<ExistingSSOConfigs />', () => {
/>
</Provider>,
);
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
userEvent.click(screen.getByText('Delete'));
act(() => {
userEvent.click(screen.getByTestId(`existing-sso-config-card-dropdown-${incompleteConfig[0].id}`));
});
act(() => {
userEvent.click(screen.getByText('Delete'));
});
await waitFor(() => {
expect(screen.getByText(
'We were unable to delete your configuration. Please try removing again or contact support for help.',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,7 @@ const setupNewExistingSSOConfigs = (configs) => {
refreshBool={false}
setRefreshBool={mockSetRefreshBool}
setPollingNetworkError={mockSetPollingNetworkError}
setIsStepperOpen={jest.fn()}
/>
</Provider>
</SSOConfigContext.Provider>
Expand Down
Loading

0 comments on commit 37de1eb

Please sign in to comment.