refactor: rework frontend use effect calls (#630)

* refactor: rework frontend use effect calls

* fix: rabbit comments

* fix: handle empty oauth url in login page

frontend/src/pages/login-page.tsx

@@ -40,10 +40,11 @@ export const LoginPage = () => {
   const { providers, title, oauthAutoRedirect } = useAppContext();
   const { search } = useLocation();
   const { t } = useTranslation();
-  const [oauthAutoRedirectHandover, setOauthAutoRedirectHandover] =
-    useState(false);
+
   const [showRedirectButton, setShowRedirectButton] = useState(false);
 
+  const hasAutoRedirectedRef = useRef(false);
+
   const redirectTimer = useRef<number | null>(null);
   const redirectButtonTimer = useRef<number | null>(null);
 
@@ -54,6 +55,11 @@ export const LoginPage = () => {
     compiled: compiledOIDCParams,
   } = useOIDCParams(searchParams);
 
+  const [isOauthAutoRedirect, setIsOauthAutoRedirect] = useState(
+    providers.find((provider) => provider.id === oauthAutoRedirect) !==
+      undefined && props.redirect_uri,
+  );
+
   const oauthProviders = providers.filter(
     (provider) => provider.id !== "local" && provider.id !== "ldap",
   );
@@ -62,10 +68,15 @@ export const LoginPage = () => {
       (provider) => provider.id === "local" || provider.id === "ldap",
     ) !== undefined;
 
-  const oauthMutation = useMutation({
+  const {
+    mutate: oauthMutate,
+    data: oauthData,
+    isPending: oauthIsPending,
+    variables: oauthVariables,
+  } = useMutation({
     mutationFn: (provider: string) =>
       axios.get(
-        `/api/oauth/url/${provider}?redirect_uri=${encodeURIComponent(props.redirect_uri)}`,
+        `/api/oauth/url/${provider}${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
       ),
     mutationKey: ["oauth"],
     onSuccess: (data) => {
@@ -76,22 +87,28 @@ export const LoginPage = () => {
       redirectTimer.current = window.setTimeout(() => {
         window.location.replace(data.data.url);
       }, 500);
+
+      if (isOauthAutoRedirect) {
+        redirectButtonTimer.current = window.setTimeout(() => {
+          setShowRedirectButton(true);
+        }, 5000);
+      }
     },
     onError: () => {
-      setOauthAutoRedirectHandover(false);
+      setIsOauthAutoRedirect(false);
       toast.error(t("loginOauthFailTitle"), {
         description: t("loginOauthFailSubtitle"),
       });
     },
   });
 
-  const loginMutation = useMutation({
+  const { mutate: loginMutate, isPending: loginIsPending } = useMutation({
     mutationFn: (values: LoginSchema) => axios.post("/api/user/login", values),
     mutationKey: ["login"],
     onSuccess: (data) => {
       if (data.data.totpPending) {
         window.location.replace(
-          `/totp?redirect_uri=${encodeURIComponent(props.redirect_uri)}`,
+          `/totp${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
         );
         return;
       }
@@ -106,7 +123,7 @@ export const LoginPage = () => {
           return;
         }
         window.location.replace(
-          `/continue?redirect_uri=${encodeURIComponent(props.redirect_uri)}`,
+          `/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`,
         );
       }, 500);
     },
@@ -122,34 +139,34 @@ export const LoginPage = () => {
 
   useEffect(() => {
     if (
-      providers.find((provider) => provider.id === oauthAutoRedirect) &&
       !isLoggedIn &&
-      props.redirect_uri !== ""
+      isOauthAutoRedirect &&
+      !hasAutoRedirectedRef.current &&
+      props.redirect_uri
     ) {
-      // Not sure of a better way to do this
-      // eslint-disable-next-line react-hooks/set-state-in-effect
-      setOauthAutoRedirectHandover(true);
-      oauthMutation.mutate(oauthAutoRedirect);
-      redirectButtonTimer.current = window.setTimeout(() => {
-        setShowRedirectButton(true);
-      }, 5000);
+      hasAutoRedirectedRef.current = true;
+      oauthMutate(oauthAutoRedirect);
     }
   }, [
-    providers,
     isLoggedIn,
-    props.redirect_uri,
+    oauthMutate,
+    hasAutoRedirectedRef,
     oauthAutoRedirect,
-    oauthMutation,
+    isOauthAutoRedirect,
+    props.redirect_uri,
   ]);
 
-  useEffect(
-    () => () => {
-      if (redirectTimer.current) clearTimeout(redirectTimer.current);
-      if (redirectButtonTimer.current)
+  useEffect(() => {
+    return () => {
+      if (redirectTimer.current) {
+        clearTimeout(redirectTimer.current);
+      }
+
+      if (redirectButtonTimer.current) {
         clearTimeout(redirectButtonTimer.current);
-    },
-    [],
-  );
+      }
+    };
+  }, [redirectTimer, redirectButtonTimer]);
 
   if (isLoggedIn && isOidc) {
     return <Navigate to={`/authorize?${compiledOIDCParams}`} replace />;
@@ -158,7 +175,7 @@ export const LoginPage = () => {
   if (isLoggedIn && props.redirect_uri !== "") {
     return (
       <Navigate
-        to={`/continue?redirect_uri=${encodeURIComponent(props.redirect_uri)}`}
+        to={`/continue${props.redirect_uri ? `?redirect_uri=${encodeURIComponent(props.redirect_uri)}` : ""}`}
         replace
       />
     );
@@ -168,7 +185,7 @@ export const LoginPage = () => {
     return <Navigate to="/logout" replace />;
   }
 
-  if (oauthAutoRedirectHandover) {
+  if (isOauthAutoRedirect) {
     return (
       <Card className="min-w-xs sm:min-w-sm">
         <CardHeader>
@@ -183,7 +200,14 @@ export const LoginPage = () => {
           <CardFooter className="flex flex-col items-stretch">
             <Button
               onClick={() => {
-                window.location.replace(oauthMutation.data?.data.url);
+                if (oauthData?.data.url) {
+                  window.location.replace(oauthData.data.url);
+                } else {
+                  setIsOauthAutoRedirect(false);
+                  toast.error(t("loginOauthFailTitle"), {
+                    description: t("loginOauthFailSubtitle"),
+                  });
+                }
               }}
             >
               {t("loginOauthAutoRedirectButton")}
@@ -214,12 +238,9 @@ export const LoginPage = () => {
                 title={provider.name}
                 icon={iconMap[provider.id] ?? <OAuthIcon />}
                 className="w-full"
-                onClick={() => oauthMutation.mutate(provider.id)}
-                loading={
-                  oauthMutation.isPending &&
-                  oauthMutation.variables === provider.id
-                }
-                disabled={oauthMutation.isPending || loginMutation.isPending}
+                onClick={() => oauthMutate(provider.id)}
+                loading={oauthIsPending && oauthVariables === provider.id}
+                disabled={oauthIsPending || loginIsPending}
               />
             ))}
           </div>
@@ -229,8 +250,8 @@ export const LoginPage = () => {
         )}
         {userAuthConfigured && (
           <LoginForm
-            onSubmit={(values) => loginMutation.mutate(values)}
-            loading={loginMutation.isPending || oauthMutation.isPending}
+            onSubmit={(values) => loginMutate(values)}
+            loading={loginIsPending || oauthIsPending}
           />
         )}
         {providers.length == 0 && (