Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

fix: Bug bash fixes #1029

Merged
merged 5 commits into from
Sep 22, 2023
Merged

fix: Bug bash fixes #1029

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2458133
fix: Bug bash fixes
kiram15 Sep 13, 2023
ee74f15
fix: test fixes
kiram15 Sep 14, 2023
2aff820
Merge branch 'master' into kiram15/ENT-7682
kiram15 Sep 18, 2023
807825a
Merge branch 'master' into kiram15/ENT-7682
kiram15 Sep 19, 2023
79d7f7d
Merge branch 'master' into kiram15/ENT-7682
kiram15 Sep 22, 2023
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
5 changes: 3 additions & 2 deletions src/components/settings/SettingsApiCredentialsTab/APICredentialsPage.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import CopyButton from './CopyButton';
import { API_CLIENT_DOCUMENTATION, HELP_CENTER_LINK } from '../data/constants';

const APICredentialsPage = ({ data, setData }) => {
const [formValue, setFormValue] = useState('');
const [formValue, setFormValue] = useState(data?.redirect_uris);
const handleFormChange = (e) => {
setFormValue(e.target.value);
};
Expand Down Expand Up @@ -48,7 +48,7 @@ const APICredentialsPage = ({ data, setData }) => {
<CopyButton data={data} />
</div>
</div>
<div className="mb-4">
<div className="my-5">
<h3>Redirect URIs (optional)</h3>
<p>
If you need additional redirect URIs, add them below and regenerate your API credentials.
Expand Down Expand Up @@ -77,6 +77,7 @@ const APICredentialsPage = ({ data, setData }) => {
<Hyperlink
variant="muted"
destination={HELP_CENTER_LINK}
target="_blank"
>
contact Enterprise Customer Support.
</Hyperlink>
Expand Down
2 changes: 2 additions & 0 deletions src/components/settings/SettingsApiCredentialsTab/CopyButton.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ const CopyButton = ({ data }) => {

const handleCopyLink = async () => {
try {
const newData = data;
['user', 'id'].forEach(prop => delete newData[prop]);
Comment on lines +15 to +16
Copy link
Contributor

@marlonkeating marlonkeating Sep 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Do we need to make a newData variable here? Because we're just making a reference copy, this statement is equivalent to deleting from the original data variable.

Copy link
Contributor Author

@kiram15 kiram15 Sep 19, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

We just had some eslint things about making direct changes to a parameter. Maybe the error isn't smart enough to catch something being pass by reference vs value but I still don't love to change passed in variables

const jsonString = JSON.stringify(data);
await navigator.clipboard.writeText(jsonString);
} catch (error) {
Expand Down
21 changes: 15 additions & 6 deletions src/components/settings/SettingsApiCredentialsTab/ZeroStateCard.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import React, { useState, useContext } from 'react';
import React, { useContext, useState } from 'react';
import PropTypes from 'prop-types';

import {
Expand Down Expand Up @@ -41,24 +41,33 @@ const ZeroStateCard = ({ setShowToast, setData }) => {
<h2>You don&apos;t have API credentials yet.</h2>
{ !displayFailureAlert && (
<p>
This page allows you to generate API credentials to send to&nbsp;
This page allows you to generate API credentials to send to
your developers so they can work on integration projects.
If you believe you are seeing this page in error,&nbsp;
<Hyperlink
variant="muted"
destination={HELP_CENTER_LINK}
target="_blank"
rel="noopener noreferrer"
>
contact Enterprise Customer Support.
</Hyperlink>
</p>
)}
<p>
edX for Business API credentials credentials will provide access&nbsp;
to the following edX API endpoints: reporting dashboard, dashboard, and catalog administration.
edX for Business API credentials will provide access to the following
edX API endpoints: reporting dashboard, dashboard, and catalog administration.
</p>
<p>
By clicking the button below, you and your organization accept the {'\n'}
<a href={API_TERMS_OF_SERVICE}>edX API terms of service</a>.
By clicking the button below, you and your organization accept the&nbsp;
<Hyperlink
variant="muted"
destination={API_TERMS_OF_SERVICE}
target="_blank"
rel="noopener noreferrer"
>
edX API terms of service.
</Hyperlink>
</p>
</Card.Section>
<Card.Footer className={displayFailureAlert ? 'error-footer d-table-row' : ''}>
Expand Down
46 changes: 24 additions & 22 deletions src/components/settings/SettingsApiCredentialsTab/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,34 +34,36 @@ const SettingsApiCredentialsTab = ({
}, [enterpriseId]);

return (
<EnterpriseId.Provider value={enterpriseId}>
<ErrorContext.Provider value={[hasRegenerationError, setHasRegenerationError]}>
<ShowSuccessToast.Provider value={[showToast, setShowToast]}>
{ hasRegenerationError && <FailedAlert /> }
<ActionRow>
<h2>API credentials</h2>
<ActionRow.Spacer />
<HelpCenterButton url={HELP_CENTER_API_GUIDE}>
Help Center: EdX Enterprise API Guide
</HelpCenterButton>
</ActionRow>
<div className="mt-4">
{!data ? (
<ZeroStateCard setShowToast={setShowToast} setData={setData} />
) : (<APICredentialsPage data={data} setData={setData} />)}
</div>
<div />
{ showToast && (
<p className="m-3">
<EnterpriseId.Provider value={enterpriseId}>
<ErrorContext.Provider value={[hasRegenerationError, setHasRegenerationError]}>
<ShowSuccessToast.Provider value={[showToast, setShowToast]}>
{ hasRegenerationError && <FailedAlert /> }
<ActionRow>
<h2>API credentials</h2>
<ActionRow.Spacer />
<HelpCenterButton url={HELP_CENTER_API_GUIDE}>
Help Center: EdX Enterprise API Guide
</HelpCenterButton>
</ActionRow>
<div className="mt-4">
{!data ? (
<ZeroStateCard setShowToast={setShowToast} setData={setData} />
) : (<APICredentialsPage data={data} setData={setData} />)}
</div>
<div />
{ showToast && (
<Toast
onClose={() => setShowToast(false)}
show={showToast}
>
API credentials successfully generated
</Toast>
)}
</ShowSuccessToast.Provider>
</ErrorContext.Provider>
</EnterpriseId.Provider>
)}
</ShowSuccessToast.Provider>
</ErrorContext.Provider>
</EnterpriseId.Provider>
</p>
);
};
SettingsApiCredentialsTab.propTypes = {
Expand Down
46 changes: 25 additions & 21 deletions src/components/settings/SettingsApiCredentialsTab/tests/SettingsAPICredentialsPage.test.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,18 +20,19 @@ const name = "edx's Enterprise Credentials";
const clientId = 'y0TCvOEvvIs6ll95irirzCJ5EaF0RnSbBIIXuNJE';
const clientSecret = '1G896sVeT67jtjHO6FNd5qFqayZPIV7BtnW01P8zaAd4mDfmBVVVsUP33u';
const updated = '2023-07-28T04:28:20.909550Z';
const redirectUris = 'www.customercourses.edx.com, www.customercourses.edx.stage.com';
const redirectUri1 = 'www.customercourses.edx.com';
const redirectUri2 = 'www.customercourses.edx.com';

const data = {
name,
client_id: clientId,
client_secret: clientSecret,
redirect_uris: redirectUris,
redirect_uris: redirectUri1,
updated,
};
const regenerationData = {
...data,
redirect_uris: redirectUris,
redirect_uris: redirectUri1,
};
const copiedData = {
...data,
Expand Down Expand Up @@ -66,7 +67,7 @@ describe('API Credentials Tab', () => {
expect(screen.queryByText('Help Center: EdX Enterprise API Guide')).toBeInTheDocument();
const helpLink = screen.getByText('Help Center: EdX Enterprise API Guide');
expect(helpLink.getAttribute('href')).toBe(HELP_CENTER_API_GUIDE);
const serviceLink = screen.getByText('edX API terms of service');
const serviceLink = screen.getByText('edX API terms of service.');
expect(serviceLink.getAttribute('href')).toBe(API_TERMS_OF_SERVICE);

expect(screen.getByText('Generate API Credentials').toBeInTheDocument);
Expand All @@ -86,7 +87,7 @@ describe('API Credentials Tab', () => {
await waitFor(() => expect(screen.getByText(name)).toBeInTheDocument);

expect(screen.getByText(name).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUris}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri1}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client ID: ${clientId}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client secret: ${clientSecret}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client documentation: ${API_CLIENT_DOCUMENTATION}` }).toBeInTheDocument);
Expand Down Expand Up @@ -147,7 +148,7 @@ describe('API Credentials Tab', () => {
});

expect(screen.getByRole('heading', { name: `Application name: ${name}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUris}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri1}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client ID: ${clientId}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client secret: ${clientSecret}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client documentation: ${API_CLIENT_DOCUMENTATION}` }).toBeInTheDocument);
Expand Down Expand Up @@ -200,20 +201,25 @@ describe('API Credentials Tab', () => {
);
await waitFor(() => expect(mockFetchFn).toHaveBeenCalled());
const input = screen.getByTestId('form-control');
expect(input).toHaveValue('');
userEvent.type(input, redirectUris);
await waitFor(() => expect(input).toHaveValue(redirectUris));
expect(input).toHaveValue(redirectUri1);
userEvent.clear(input);
userEvent.type(input, redirectUri2);

await waitFor(() => expect(input).toHaveValue(redirectUri2));
const button = screen.getByText('Regenerate API Credentials');
userEvent.click(button);

await waitFor(() => expect(screen.getByText('Regenerate API credentials?')).toBeInTheDocument());
const confirmedButton = screen.getByText('Regenerate');
userEvent.click(confirmedButton);
await waitFor(() => {
expect(mockPatchFn).toHaveBeenCalledWith(redirectUris, enterpriseId);
expect(mockPatchFn).toHaveBeenCalledWith(redirectUri2, enterpriseId);
});
await waitFor(() => {
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri2}` }).toBeInTheDocument);
});
expect(screen.getByRole('heading', { name: `Application name: ${name}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUris}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri2}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client ID: ${clientId}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client secret: ${clientSecret}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `API client documentation: ${API_CLIENT_DOCUMENTATION}` }).toBeInTheDocument);
Expand All @@ -234,19 +240,19 @@ describe('API Credentials Tab', () => {
);
await waitFor(() => expect(mockFetchFn).toHaveBeenCalled());
const input = screen.getByTestId('form-control');
expect(input).toHaveValue('');
userEvent.type(input, redirectUris);
await waitFor(() => expect(input).toHaveValue(redirectUris));
expect(input).toHaveValue(redirectUri1);
userEvent.type(input, ` ${redirectUri2}`);
await waitFor(() => expect(input).toHaveValue(`${redirectUri1} ${redirectUri2}`));
const button = screen.getByText('Regenerate API Credentials');
userEvent.click(button);

await waitFor(() => expect(screen.getByText('Regenerate API credentials?')).toBeInTheDocument());
const confirmedButton = screen.getByText('Regenerate');
userEvent.click(confirmedButton);
await waitFor(() => {
expect(mockPatchFn).toHaveBeenCalledWith(redirectUris, enterpriseId);
expect(mockPatchFn).toHaveBeenCalledWith(`${redirectUri1} ${redirectUri2}`, enterpriseId);
});
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUris}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri1}` }).toBeInTheDocument);
expect(screen.getByText('Something went wrong while generating your credentials. Please try again. If the issue continues, contact Enterprise Customer Support.'))
.toBeInTheDocument();
});
Expand All @@ -264,18 +270,16 @@ describe('API Credentials Tab', () => {

await waitFor(() => expect(mockFetchFn).toHaveBeenCalled());
const input = screen.getByTestId('form-control');
expect(input).toHaveValue('');
userEvent.type(input, redirectUris);
await waitFor(() => expect(input).toHaveValue(redirectUris));
expect(input).toHaveValue(redirectUri1);
const button = screen.getByText('Regenerate API Credentials');
userEvent.click(button);

await waitFor(() => expect(screen.getByText('Regenerate API credentials?')).toBeInTheDocument());
const cancelButton = screen.getByText('Cancel');
userEvent.click(cancelButton);
await waitFor(() => {
expect(mockPatchFn).not.toHaveBeenCalledWith(redirectUris, enterpriseId);
expect(mockPatchFn).not.toHaveBeenCalledWith(redirectUri1, enterpriseId);
});
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUris}` }).toBeInTheDocument);
expect(screen.getByRole('heading', { name: `Allowed URIs: ${redirectUri1}` }).toBeInTheDocument);
});
});