Merge pull request awslabs#377 from awslabs/bugfix

Fix bugs with email validation and catalog visibility listings, remove aws-sdk from catalog updater

Author: amazon-meaisiah

dev-portal/src/pages/Admin/Accounts/PendingInvites.jsx

@@ -1,4 +1,4 @@
-import React, { useCallback, useEffect, useMemo, useState } from 'react'
+import React, { useCallback, useEffect, useMemo, useState, useRef } from 'react'
 import {
   Button,
   Container,
@@ -179,25 +179,53 @@ const TableActions = ({
   </Button.Group>
 )
 
+// Pulled from https://html.spec.whatwg.org/multipage/input.html#e-mail-state-(type%3Demail) and
+// optimized in a few ways for size:
+// - Classes of `[A-Za-z0-9]` were shortened to the equivalent `[^_\W]`.
+// - Other instances of `0-9` in classes were converted to the shorthand `\d`.
+// - The whole regexp was made case-insensitive to avoid the need for `A-Za-z` in classes.
+// - As we're only testing, I replaced all the non-capturing groups with capturing ones.
+const validEmailRegex =
+  /^[\w.!#$%&'*+\/=?^`{|}~-]+@[^_\W]([a-z\d-]{0,61}[^_\W])?(\.[^_\W]([a-z\d-]{0,61}[^_\W])?)*$/i
+
 /*
  * Note: `onConfirm` should return a boolean indicating whether the creation
  * succeeded.
  */
 const CreateInviteModal = ({ onConfirm, open, onClose, messages }) => {
   const [email, setEmail] = useState('')
   const [loading, setLoading] = useState(false)
-  const isEmailValid = useMemo(() => /^[^@\s]+@[^@\s]+$/.test(email), [email])
+  const isEmailValid = useMemo(() => validEmailRegex.test(email), [email])
   const onChangeEmailAddress = useCallback(
     (_event, { value }) => setEmail(value),
     [],
   )
   const onClickCreate = useCallback(async () => {
     setLoading(true)
-    if (await onConfirm(email)) {
-      setEmail('')
+    try {
+      if (await onConfirm(email)) {
+        setEmail('')
+      }
+    } finally {
+      setLoading(false)
     }
-    setLoading(false)
   }, [onConfirm, email])
+  
+  // If the user stops typing, but the email is invalid, show the invalid email message as a hint
+  // for why they can't proceed. Don't make the timeout so short that it'd annoy a slow typer,
+  // though.
+  const [needsAssistance, setNeedsAssistance] = useState(false)
+  const emailEverSet = useRef(false)
+
+  useEffect(() => {
+    if (email) emailEverSet.current = true
+    if (isEmailValid) {
+      setNeedsAssistance(false)
+    } else if (!needsAssistance && emailEverSet.current) {
+      const timer = setTimeout(() => { setNeedsAssistance(true) }, 3000 /* ms */)
+      return () => { clearTimeout(timer) }
+    }
+  }, [email, needsAssistance])
 
   return (
     <Modal open={open} onClose={onClose} size={'small'}>
@@ -208,7 +236,7 @@ const CreateInviteModal = ({ onConfirm, open, onClose, messages }) => {
           send an invitation to create an account.
         </p>
         <MessageList.MessageList messages={messages} />
-        <Message hidden={isEmailValid || loading} warning>
+        <Message hidden={!needsAssistance || isEmailValid || loading} warning>
           Please enter a valid email address.
         </Message>
         <Input

lambdas/backend/__tests__/routes/admin/catalog/visibility.js

@@ -246,6 +246,140 @@ describe('GET /admin/catalog/visibility', () => {
       }
     })
   })
+
+  test('includes unsubscribable API Gateway-managed APIs with subscribable ones', async () => {
+    util.catalog.mockReturnValue(Promise.resolve({
+      apiGateway: [
+        {
+          id: 'aaaaaa',
+          name: 'basic usage plan',
+          apis: [
+            {
+              apiId: 'a1b2c3',
+              apiStage: 'prod',
+              swagger: {}
+            }
+          ]
+        }
+      ],
+      generic: [
+        {
+          apiId: 'd1e2f3',
+          apiStage: 'prod',
+          swagger: {}
+        }
+      ]
+    }))
+
+    util.apigateway.getRestApis = jest.fn().mockReturnValue(promiser({
+      items: [
+        {
+          id: 'a1b2c3',
+          name: 'first'
+        },
+        {
+          id: 'd1e2f3',
+          name: 'second'
+        }
+      ]
+    }))
+
+    util.apigateway.getStages = jest.fn()
+      .mockReturnValueOnce(promiser({
+        item: [
+          {
+            id: 'a1b2c3',
+            name: 'first',
+            stageName: 'prod'
+          },
+          {
+            id: 'a1b2c3',
+            name: 'first',
+            stageName: 'exclude'
+          }
+        ]
+      }))
+      .mockReturnValueOnce(promiser({
+        item: [
+          {
+            id: 'd1e3f3',
+            name: 'second',
+            stageName: 'prod'
+          },
+          {
+            id: 'd1e3f3',
+            name: 'second',
+            stageName: 'exclude'
+          }
+        ]
+      }))
+
+    util.apigateway.getUsagePlans = jest.fn()
+      .mockReturnValue(promiser({
+        items: [
+          {
+            id: 'z1x2c3',
+            name: 'basic',
+            apiStages: [
+              {
+                apiId: 'a1b2c3',
+                stage: 'prod'
+              },
+              {
+                apiId: 'a1b2c3',
+                stage: 'exclude'
+              }
+            ]
+          }
+        ]
+      }))
+
+    await adminCatalogVisibility.get(generateRequestContext(), mockResponseObject)
+
+    expect(util.apigateway.getRestApis).toHaveBeenCalledTimes(1)
+    expect(util.apigateway.getStages).toHaveBeenCalledTimes(2)
+    expect(mockResponseObject.status).toHaveBeenCalledWith(200)
+    expect(mockResponseObject.json).toHaveBeenCalledWith({
+      apiGateway: [
+        {
+          id: 'a1b2c3',
+          stage: 'prod',
+          visibility: true,
+          subscribable: true,
+          name: 'first',
+          usagePlanId: 'z1x2c3',
+          usagePlanName: 'basic',
+          sdkGeneration: false
+        },
+        {
+          id: 'a1b2c3',
+          stage: 'exclude',
+          visibility: false,
+          subscribable: true,
+          name: 'first',
+          usagePlanId: 'z1x2c3',
+          usagePlanName: 'basic',
+          sdkGeneration: false
+        },
+        {
+          id: 'd1e2f3',
+          stage: 'prod',
+          visibility: true,
+          subscribable: false,
+          name: 'second',
+          sdkGeneration: false
+        },
+        {
+          id: 'd1e2f3',
+          stage: 'exclude',
+          visibility: false,
+          subscribable: false,
+          name: 'second',
+          sdkGeneration: false
+        }
+      ]
+    })
+  })
 })
 
 describe('POST /admin/catalog/visibility', () => {

lambdas/backend/routes/admin/catalog/visibility.js

@@ -80,6 +80,15 @@ exports.get = async (req, res) => {
 
     // mark every api in the generic catalog as visible
     catalogObject.generic.forEach((catalogEntry) => {
+      // Unlike in the catalog and elsewhere, the visibility's `apiGateway` contains *all* API
+      // Gateway-managed APIs, and only unmanaged APIs are in `visibility.generic`.
+      if (catalogEntry.apiId != null) {
+        const target = visibility.apiGateway.find((api) =>
+          api.id === catalogEntry.apiId && api.stage === catalogEntry.apiStage
+        )
+        if (target != null) { target.visibility = true; return }
+      }
+
       if (!visibility.generic) {
         visibility.generic = {}
       }
@@ -89,8 +98,6 @@ exports.get = async (req, res) => {
         name: (catalogEntry.swagger && catalogEntry.swagger.info && catalogEntry.swagger.info.title) || 'Untitled'
       }
 
-      if (catalogEntry.stage) { visibility.generic[catalogEntry.id].stage = catalogEntry.stage }
-      if (catalogEntry.apiId) { visibility.generic[catalogEntry.id].apiId = catalogEntry.apiId }
       if (catalogEntry.sdkGeneration !== undefined) {
         console.log(`catalogEntry: ${inspect(catalogEntry)}`)
         visibility.apiGateway.map((api) => {