From 9bae155e79040c764f28933c2a1a4612af7380fa Mon Sep 17 00:00:00 2001
From: Paul Mineev <paul@mineev.me>
Date: Thu, 10 Feb 2022 11:56:28 -0800
Subject: [PATCH] Fixes for telegram

- fix type for an error in fetcher
- use is-object func for checks
- better error handling for auth requests
---
 frontend/app/common/fetcher.ts                |  2 +-
 frontend/app/common/local-storage.ts          |  3 +-
 frontend/app/components/auth/auth.hooks.ts    | 24 ++++++++---
 .../app/components/auth/auth.messsages.ts     |  6 ++-
 frontend/app/components/auth/auth.module.css  |  2 +-
 frontend/app/components/auth/auth.tsx         | 43 +++++++++++++------
 frontend/app/utils/is-object.spec.ts          | 28 ++++++++++++
 frontend/app/utils/is-object.ts               |  3 ++
 8 files changed, 89 insertions(+), 22 deletions(-)
 create mode 100644 frontend/app/utils/is-object.spec.ts
 create mode 100644 frontend/app/utils/is-object.ts

diff --git a/frontend/app/common/fetcher.ts b/frontend/app/common/fetcher.ts
index df19d420..800bae82 100644
--- a/frontend/app/common/fetcher.ts
+++ b/frontend/app/common/fetcher.ts
@@ -106,7 +106,7 @@ const createFetcher = (baseUrl: string = ''): Methods => {
 
       return res.text();
     } catch (e) {
-      if (e?.message === 'Failed to fetch') {
+      if (e instanceof Error && e.message === 'Failed to fetch') {
         throw new RequestError(e.message, 'fetch-error');
       }
 
diff --git a/frontend/app/common/local-storage.ts b/frontend/app/common/local-storage.ts
index d481b048..5fec353b 100644
--- a/frontend/app/common/local-storage.ts
+++ b/frontend/app/common/local-storage.ts
@@ -1,3 +1,4 @@
+import { isObject } from 'utils/is-object';
 import { IS_STORAGE_AVAILABLE } from './constants';
 
 const failMessage = 'remark42: localStorage access denied, check browser preferences';
@@ -57,7 +58,7 @@ export function updateJsonItem<T>(key: string, value: T) {
     return;
   }
 
-  if (value !== null && typeof value === 'object') {
+  if (isObject(value)) {
     setJsonItem(key, { ...savedData, ...value });
     return;
   }
diff --git a/frontend/app/components/auth/auth.hooks.ts b/frontend/app/components/auth/auth.hooks.ts
index 668bde96..dadb1b0c 100644
--- a/frontend/app/components/auth/auth.hooks.ts
+++ b/frontend/app/components/auth/auth.hooks.ts
@@ -2,6 +2,7 @@ import { useEffect, useRef, useState, useMemo } from 'preact/hooks';
 import { useIntl } from 'react-intl';
 
 import { errorMessages, RequestError } from 'utils/errorUtils';
+import { isObject } from 'utils/is-object';
 import { parseMessage, postMessageToParent } from 'utils/post-message';
 import { messages } from './auth.messsages';
 
@@ -16,9 +17,10 @@ export function useDropdown(disableClosing?: boolean) {
   const rootRef = useRef<HTMLDivElement>(null);
   const clickInsideRef = useRef<boolean>(false);
   const [showDropdown, setShowDropdown] = useState(false);
-  const toggleDropdownState = () => {
+
+  function toggleDropdownState() {
     setShowDropdown((s) => !s);
-  };
+  }
 
   useEffect(() => {
     const dropdownElement = rootRef.current;
@@ -93,19 +95,24 @@ export function useDropdown(disableClosing?: boolean) {
 
 export function useErrorMessage(): [string | null, (e: unknown) => void] {
   const intl = useIntl();
-  const [invalidReason, setInvalidReason] = useState<string | null>(null);
+  const [invalidReason, setInvalidReason] = useState<string | number | null>(null);
 
   return useMemo(() => {
     let errorMessage = invalidReason;
 
-    if (invalidReason && messages[invalidReason]) {
+    if (invalidReason !== null && typeof invalidReason === 'string' && messages[invalidReason]) {
       errorMessage = intl.formatMessage(messages[invalidReason]);
     }
 
-    if (invalidReason && errorMessages[invalidReason]) {
+    if (invalidReason !== null && errorMessages[invalidReason]) {
       errorMessage = intl.formatMessage(errorMessages[invalidReason]);
     }
 
+    if (typeof errorMessage === 'number') {
+      console.error('Wrong error message', errorMessage);
+      errorMessage = null;
+    }
+
     function setError(err: unknown): void {
       if (err === null) {
         setInvalidReason(null);
@@ -117,7 +124,12 @@ export function useErrorMessage(): [string | null, (e: unknown) => void] {
         return;
       }
 
-      const errorReason = err instanceof RequestError ? err.error : err instanceof Error ? err.message : 'error.0';
+      const errorReason =
+        err instanceof RequestError || (isObject(err) && typeof (err as Record<string, string>).error === 'string')
+          ? (err as Record<'error', string>).error
+          : err instanceof Error
+          ? err.message
+          : 0;
 
       setInvalidReason(errorReason);
     }
diff --git a/frontend/app/components/auth/auth.messsages.ts b/frontend/app/components/auth/auth.messsages.ts
index f56536f5..261d040c 100644
--- a/frontend/app/components/auth/auth.messsages.ts
+++ b/frontend/app/components/auth/auth.messsages.ts
@@ -65,9 +65,13 @@ export const messages = defineMessages<string>({
     id: 'auth.telegram-check',
     defaultMessage: 'Check',
   },
+  telegramQR: {
+    id: 'auth.telegram-qr',
+    defaultMessage: 'Telegram QR-code',
+  },
   telegramMessage1: {
     id: 'auth.telegram-message-1',
-    defaultMessage: 'Open the Telegram',
+    defaultMessage: 'Open Telegram',
   },
   telegramOptionalQR: {
     id: 'auth.telegram-optional-qr',
diff --git a/frontend/app/components/auth/auth.module.css b/frontend/app/components/auth/auth.module.css
index 9120cfcd..3d84bd7b 100644
--- a/frontend/app/components/auth/auth.module.css
+++ b/frontend/app/components/auth/auth.module.css
@@ -198,7 +198,7 @@
 
 .telegramQR {
   display: block;
-  margin: 0 auto 0 auto;
+  margin: 12px auto;
   width: 75%;
 }
 
diff --git a/frontend/app/components/auth/auth.tsx b/frontend/app/components/auth/auth.tsx
index 75bd66b5..a0a935ac 100644
--- a/frontend/app/components/auth/auth.tsx
+++ b/frontend/app/components/auth/auth.tsx
@@ -69,21 +69,29 @@ export function Auth() {
     const { href, dataset } = evt.currentTarget;
 
     if (dataset.providerName?.toLowerCase() === 'telegram') {
-      telegramParamsRef.current = await getTelegramSigninParams();
-      window.open(`https://t.me/${telegramParamsRef.current.bot}/?start=${telegramParamsRef.current.token}`);
+      try {
+        telegramParamsRef.current = await getTelegramSigninParams();
+      } catch (e) {
+        setError(e);
+        return;
+      }
+
       setView('telegram');
       setError(null);
       return;
     }
 
-    const user = await oauthSignin(href);
+    try {
+      const user = await oauthSignin(href);
 
-    if (user === null) {
-      // TODO: add error message when user is null
-      return;
+      if (user === null) {
+        // TODO: add error message when user is null
+        return;
+      }
+      dispatch(setUser(user));
+    } catch (e) {
+      setError(e);
     }
-
-    dispatch(setUser(user));
   }
 
   function handleProviderChange(evt: Event) {
@@ -144,7 +152,12 @@ export function Auth() {
     setError(null);
 
     if (telegramParamsRef.current === null) {
-      telegramParamsRef.current = await getTelegramSigninParams();
+      try {
+        telegramParamsRef.current = await getTelegramSigninParams();
+      } catch (e) {
+        setError(e);
+        return;
+      }
     }
 
     try {
@@ -218,7 +231,11 @@ export function Auth() {
                 </div>
                 <p className={clsx('telegram', styles.telegram)}>
                   {intl.formatMessage(messages.telegramMessage1)}{' '}
-                  <a href={`https://t.me/${telegramParamsRef.current.bot}/?start=${telegramParamsRef.current.token}`}>
+                  <a
+                    target="_blank"
+                    rel="noopener noreferrer"
+                    href={`https://t.me/${telegramParamsRef.current.bot}/?start=${telegramParamsRef.current.token}`}
+                  >
                     {intl.formatMessage(messages.telegramLink)}
                   </a>
                   {window.screen.width >= 768 && ` ${intl.formatMessage(messages.telegramOptionalQR)}`}{' '}
@@ -229,14 +246,16 @@ export function Auth() {
                 {window.screen.width >= 768 && (
                   <img
                     src={`${BASE_URL}${API_BASE}/qr/telegram?url=https://t.me/${telegramParamsRef.current.bot}/?start=${telegramParamsRef.current.token}`}
+                    height="180"
+                    width="200"
                     className={clsx('telegram-qr', styles.telegramQR)}
-                    alt={'telegram QR-code'}
+                    alt={intl.formatMessage(messages.telegramQR)}
                   />
                 )}
+                {errorMessage && <div className={clsx('auth-error', styles.error)}>{errorMessage}</div>}
                 <Button key="submit" className="auth-submit" type="submit" onClick={handleTelegramSubmit}>
                   {intl.formatMessage(messages.telegramCheck)}
                 </Button>
-                {errorMessage && <div className={clsx('auth-error', styles.error)}>{errorMessage}</div>}
               </>
             ) : view === 'token' ? (
               <>
diff --git a/frontend/app/utils/is-object.spec.ts b/frontend/app/utils/is-object.spec.ts
new file mode 100644
index 00000000..67083993
--- /dev/null
+++ b/frontend/app/utils/is-object.spec.ts
@@ -0,0 +1,28 @@
+import { isObject } from './is-object';
+
+describe('is-object', () => {
+  it.each`
+    input
+    ${null}
+    ${undefined}
+    ${0}
+    ${1}
+    ${''}
+    ${'string'}
+    ${true}
+    ${false}
+    ${[]}
+    ${[1, 2, 3]}
+  `('should return false if is NOT an object', ({ input }) => {
+    expect(isObject(input)).toBe(false);
+  });
+  it.each`
+    input
+    ${{}}
+    ${{ a: 1 }}
+    ${{ a: 1, b: 2 }}
+    ${new Error()}
+  `('should return true if IS an object', ({ input }) => {
+    expect(isObject(input)).toBe(true);
+  });
+});
diff --git a/frontend/app/utils/is-object.ts b/frontend/app/utils/is-object.ts
new file mode 100644
index 00000000..4030e94b
--- /dev/null
+++ b/frontend/app/utils/is-object.ts
@@ -0,0 +1,3 @@
+export function isObject(value: unknown): boolean {
+  return value !== null && typeof value === 'object' && !Array.isArray(value);
+}