Address PR requests

target · Dec 21, 2022 · a4e65b0 · a4e65b0
1 parent 67c6ed4
commit a4e65b0
Show file tree

Hide file tree

Showing 5 changed files with 26 additions and 16 deletions.
diff --git a/notification/store.go b/notification/store.go
@@ -254,8 +254,9 @@ func (s *Store) SendContactMethodTest(ctx context.Context, id string) error {
 	}
 
 	// if the contact method user id does not match the current user id, return an error
-	if cmUserID != permission.UserID(ctx) {
-		return validation.NewFieldError("ContactMethod", "contact method does not belong to user")
+	err = permission.LimitCheckAny(ctx, permission.MatchUser(cmUserID))
+	if err != nil {
+		return err
 	}
 
 	tx, err := s.db.BeginTx(ctx, nil)

diff --git a/test/smoke/twilioenablebysms_test.go b/test/smoke/twilioenablebysms_test.go
@@ -14,7 +14,7 @@ func TestTwilioEnableBySMS(t *testing.T) {
 	sqlQuery := `
 		insert into users (id, name, email, role) 
 		values 
-			({{uuid "user"}}, 'bob', 'joe', 'admin');
+			({{uuid "user"}}, 'bob', 'joe', 'user');
 		insert into user_contact_methods (id, user_id, name, type, value, disabled) 
 		values
 			({{uuid "cm1"}}, {{uuid "user"}}, 'personal', 'SMS', {{phone "1"}}, true),

diff --git a/test/smoke/twiliotestsmsnotcmowner_test.go b/test/smoke/twiliotestsmsnotcmowner_test.go
@@ -26,7 +26,7 @@ func TestTwilioSMSNotCMOwner(t *testing.T) {
 	doQL := func(query string) {
 		g := h.GraphQLQuery2(query)
 		require.Len(t, g.Errors, 1, "errors returned from GraphQL")
-		require.Equal(t, "contact method does not belong to user", g.Errors[0].Message)
+		require.Equal(t, "access denied", g.Errors[0].Message)
 	}
 	cm1 := h.UUID("cm1")
 

diff --git a/test/smoke/twiliotestvoicenotcmowner_test.go b/test/smoke/twiliotestvoicenotcmowner_test.go
@@ -26,7 +26,7 @@ func TestTwilioVoiceNotCMOwner(t *testing.T) {
 	doQL := func(query string) {
 		g := h.GraphQLQuery2(query)
 		require.Len(t, g.Errors, 1, "errors returned from GraphQL")
-		require.Equal(t, "contact method does not belong to user", g.Errors[0].Message)
+		require.Equal(t, "access denied", g.Errors[0].Message)
 	}
 	cm1 := h.UUID("cm1")
 

diff --git a/web/src/app/users/SendTestDialog.tsx b/web/src/app/users/SendTestDialog.tsx
@@ -1,4 +1,4 @@
-import React, { useEffect, MouseEvent, useRef } from 'react'
+import React, { useEffect, MouseEvent, useState } from 'react'
 import { gql, useQuery, useMutation } from 'urql'
 
 import Spinner from '../loading/components/Spinner'
@@ -53,14 +53,13 @@ export default function SendTestDialog(
     requestPolicy: 'network-only',
   })
 
-  const now = useRef(DateTime.utc())
+  // We keep a stable timestampe to track how long the dialog has been open
+  const [now] = useState(DateTime.utc())
   const status = data?.userContactMethod?.lastTestMessageState?.status ?? ''
   const cmDestValue = data?.userContactMethod?.formattedValue ?? ''
   const cmType: ContactMethodType = data?.userContactMethod?.type ?? ''
   const lastTestVerifyAt = data?.userContactMethod?.lastTestVerifyAt ?? ''
-  const timeSinceLastVerified = now.current.diff(
-    DateTime.fromISO(lastTestVerifyAt),
-  )
+  const timeSinceLastVerified = now.diff(DateTime.fromISO(lastTestVerifyAt))
   const fromValue =
     data?.userContactMethod?.lastTestMessageState?.formattedSrcValue ?? ''
   const errorMessage = (error?.message || sendTestStatus.error?.message) ?? ''
@@ -77,8 +76,8 @@ export default function SendTestDialog(
     }
   }, [lastTestVerifyAt, fetching])
 
-  let details
-  if (sendTestStatus.data && lastTestVerifyAt > now.current.toISO()) {
+  let details = ''
+  if (sendTestStatus.data && lastTestVerifyAt > now.toISO()) {
     details = data?.userContactMethod?.lastTestMessageState?.details ?? ''
   }
 
@@ -107,6 +106,18 @@ export default function SendTestDialog(
     }
   }
 
+  const loading = (): boolean => {
+    // if the sendTestStatus.fetching is true OR there's no details and no error message OR the status is pending, return true
+    if (
+      sendTestStatus.fetching ||
+      (!details && !errorMessage) ||
+      status === 'pending'
+    ) {
+      return true
+    }
+    return false
+  }
+
   return (
     <Dialog open onClose={onClose}>
       <DialogTitle>{title}</DialogTitle>
@@ -115,15 +126,13 @@ export default function SendTestDialog(
         <DialogContentText>
           GoAlert is sending a test {msg()}.
         </DialogContentText>
-        {(sendTestStatus.fetching || (!details && !errorMessage)) && (
-          <Spinner text='Sending Test...' />
-        )}
+        {loading() && <Spinner text='Sending Test...' />}
         {fromValue && (
           <DialogContentText>
             The test message was sent from {fromValue}.
           </DialogContentText>
         )}
-        {details && (
+        {!!details && (
           <DialogContentText color={getTestStatusColor(status)}>
             {toTitleCase(details)}
           </DialogContentText>