fix: Bug bash fixes (#1029)

* fix: Bug bash fixes * fix: test fixes
openedx · Sep 22, 2023 · ed98264 · ed98264
1 parent e5edf15
commit ed98264
Show file tree

Hide file tree

Showing 5 changed files with 69 additions and 51 deletions.
diff --git a/src/components/settings/SettingsApiCredentialsTab/APICredentialsPage.jsx b/src/components/settings/SettingsApiCredentialsTab/APICredentialsPage.jsx
@@ -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);
   };
@@ -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.
@@ -77,6 +77,7 @@ const APICredentialsPage = ({ data, setData }) => {
           <Hyperlink
             variant="muted"
             destination={HELP_CENTER_LINK}
+            target="_blank"
           >
             contact Enterprise Customer Support.
           </Hyperlink>

diff --git a/src/components/settings/SettingsApiCredentialsTab/CopyButton.jsx b/src/components/settings/SettingsApiCredentialsTab/CopyButton.jsx
@@ -12,6 +12,8 @@ const CopyButton = ({ data }) => {
 
   const handleCopyLink = async () => {
     try {
+      const newData = data;
+      ['user', 'id'].forEach(prop => delete newData[prop]);
       const jsonString = JSON.stringify(data);
       await navigator.clipboard.writeText(jsonString);
     } catch (error) {

diff --git a/src/components/settings/SettingsApiCredentialsTab/ZeroStateCard.jsx b/src/components/settings/SettingsApiCredentialsTab/ZeroStateCard.jsx
@@ -1,4 +1,4 @@
-import React, { useState, useContext } from 'react';
+import React, { useContext, useState } from 'react';
 import PropTypes from 'prop-types';
 
 import {
@@ -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' : ''}>

diff --git a/src/components/settings/SettingsApiCredentialsTab/index.jsx b/src/components/settings/SettingsApiCredentialsTab/index.jsx
@@ -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 = {

diff --git a/src/components/settings/SettingsApiCredentialsTab/tests/SettingsAPICredentialsPage.test.jsx b/src/components/settings/SettingsApiCredentialsTab/tests/SettingsAPICredentialsPage.test.jsx
@@ -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,
@@ -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);
@@ -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);
@@ -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);
@@ -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);
@@ -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();
   });
@@ -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);
   });
 });