Review#1: introduce AuthenticationProvider common interface, do not…

… generalize flags just yet, use object format for `accessAgreement` config, use loading screen.
elastic · Apr 23, 2020 · b9d4713 · b9d4713
1 parent 258e597
commit b9d4713
Show file tree

Hide file tree

Showing 6 changed files with 81 additions and 280 deletions.
diff --git a/.../public/authentication/access_agreement/__snapshots__/access_agreement_page.test.tsx.snap b/.../public/authentication/access_agreement/__snapshots__/access_agreement_page.test.tsx.snap
diff --git a/...ck/plugins/security/public/authentication/access_agreement/access_agreement_page.test.tsx b/...ck/plugins/security/public/authentication/access_agreement/access_agreement_page.test.tsx
@@ -5,11 +5,12 @@
  */
 
 import React from 'react';
+import ReactMarkdown from 'react-markdown';
+import { EuiLoadingContent } from '@elastic/eui';
 import { act } from '@testing-library/react';
 import { mountWithIntl, nextTick } from 'test_utils/enzyme_helpers';
 import { findTestSubject } from 'test_utils/find_test_subject';
 import { coreMock } from '../../../../../../src/core/public/mocks';
-import { AuthenticationStatePage } from '../components/authentication_state_page';
 import { AccessAgreementPage } from './access_agreement_page';
 
 describe('AccessAgreementPage', () => {
@@ -36,15 +37,17 @@ describe('AccessAgreementPage', () => {
       />
     );
 
-    // Shouldn't render anything if state isn't yet available.
-    expect(wrapper.isEmptyRender()).toBe(true);
+    expect(wrapper.exists(EuiLoadingContent)).toBe(true);
+    expect(wrapper.exists(ReactMarkdown)).toBe(false);
 
     await act(async () => {
       await nextTick();
       wrapper.update();
     });
 
-    expect(wrapper.find(AuthenticationStatePage)).toMatchSnapshot();
+    expect(wrapper.find(ReactMarkdown)).toMatchSnapshot();
+    expect(wrapper.exists(EuiLoadingContent)).toBe(false);
+
     expect(coreStartMock.http.get).toHaveBeenCalledTimes(1);
     expect(coreStartMock.http.get).toHaveBeenCalledWith(
       '/internal/security/access_agreement/state'

diff --git a/x-pack/plugins/security/public/authentication/access_agreement/access_agreement_page.tsx b/x-pack/plugins/security/public/authentication/access_agreement/access_agreement_page.tsx
@@ -9,7 +9,15 @@ import './_index.scss';
 import React, { FormEvent, MouseEvent, useCallback, useEffect, useState } from 'react';
 import ReactDOM from 'react-dom';
 import ReactMarkdown from 'react-markdown';
-import { EuiButton, EuiPanel, EuiFlexGroup, EuiFlexItem, EuiSpacer, EuiText } from '@elastic/eui';
+import {
+  EuiButton,
+  EuiPanel,
+  EuiFlexGroup,
+  EuiFlexItem,
+  EuiLoadingContent,
+  EuiSpacer,
+  EuiText,
+} from '@elastic/eui';
 import { i18n } from '@kbn/i18n';
 import { FormattedMessage } from '@kbn/i18n/react';
 import { CoreStart, FatalErrorsStart, HttpStart, NotificationsStart } from 'src/core/public';
@@ -55,9 +63,43 @@ export function AccessAgreementPage({ http, fatalErrors, notifications }: Props)
     [http, notifications]
   );
 
-  if (accessAgreement == null) {
-    return null;
-  }
+  const content = accessAgreement ? (
+    <form onSubmit={onAcknowledge}>
+      <EuiPanel paddingSize="none">
+        <EuiFlexGroup gutterSize="none" direction="column">
+          <EuiFlexItem className="secAccessAgreementPage__textWrapper">
+            <div className="secAccessAgreementPage__text">
+              <EuiText textAlign="left">
+                <ReactMarkdown>{accessAgreement}</ReactMarkdown>
+              </EuiText>
+            </div>
+          </EuiFlexItem>
+          <EuiFlexItem className="secAccessAgreementPage__footer">
+            <div className="secAccessAgreementPage__footerInner">
+              <EuiButton
+                fill
+                type="submit"
+                color="primary"
+                onClick={onAcknowledge}
+                isDisabled={isLoading}
+                isLoading={isLoading}
+                data-test-subj="accessAgreementAcknowledge"
+              >
+                <FormattedMessage
+                  id="xpack.security.accessAgreement.acknowledgeButtonText"
+                  defaultMessage="Acknowledge and continue"
+                />
+              </EuiButton>
+            </div>
+          </EuiFlexItem>
+        </EuiFlexGroup>
+      </EuiPanel>
+    </form>
+  ) : (
+    <EuiPanel paddingSize="l">
+      <EuiLoadingContent lines={10} />
+    </EuiPanel>
+  );
 
   return (
     <AuthenticationStatePage
@@ -69,38 +111,8 @@ export function AccessAgreementPage({ http, fatalErrors, notifications }: Props)
         />
       }
     >
-      <form onSubmit={onAcknowledge}>
-        <EuiPanel paddingSize="none">
-          <EuiFlexGroup gutterSize="none" direction="column">
-            <EuiFlexItem className="secAccessAgreementPage__textWrapper">
-              <div className="secAccessAgreementPage__text">
-                <EuiText textAlign="left">
-                  <ReactMarkdown>{accessAgreement}</ReactMarkdown>
-                </EuiText>
-              </div>
-            </EuiFlexItem>
-            <EuiFlexItem className="secAccessAgreementPage__footer">
-              <div className="secAccessAgreementPage__footerInner">
-                <EuiButton
-                  fill
-                  type="submit"
-                  color="primary"
-                  onClick={onAcknowledge}
-                  isDisabled={isLoading}
-                  isLoading={isLoading}
-                  data-test-subj="accessAgreementAcknowledge"
-                >
-                  <FormattedMessage
-                    id="xpack.security.accessAgreement.acknowledgeButtonText"
-                    defaultMessage="Acknowledge and continue"
-                  />
-                </EuiButton>
-              </div>
-            </EuiFlexItem>
-          </EuiFlexGroup>
-        </EuiPanel>
-        <EuiSpacer size="xxl" />
-      </form>
+      {content}
+      <EuiSpacer size="xxl" />
     </AuthenticationStatePage>
   );
 }