Harden subscribe resume and sign-out paths
Address PR review: avoid reintroducing cleared URL markers, treat already-closed sessions as successful logout, and stop stale subscribe/sign-out completions from racing the UI. Signed-off-by: Émile Ré <emile@probo.com>
This commit is contained in:
@@ -29,7 +29,7 @@ import { DialogTitle } from "@probo/ui/src/v2/Dialog/DialogTitle";
|
|||||||
import { Field } from "@probo/ui/src/v2/form/Field";
|
import { Field } from "@probo/ui/src/v2/form/Field";
|
||||||
import { TextField } from "@probo/ui/src/v2/form/TextField";
|
import { TextField } from "@probo/ui/src/v2/form/TextField";
|
||||||
import { Text } from "@probo/ui/src/v2/typography/Text";
|
import { Text } from "@probo/ui/src/v2/typography/Text";
|
||||||
import { type FormEvent, useState } from "react";
|
import { type FormEvent, useEffect, useRef, useState } from "react";
|
||||||
import { useTranslation } from "react-i18next";
|
import { useTranslation } from "react-i18next";
|
||||||
|
|
||||||
import { useSubscribeToMailingList } from "#/lib/mailingList/useSubscribeToMailingList";
|
import { useSubscribeToMailingList } from "#/lib/mailingList/useSubscribeToMailingList";
|
||||||
@@ -45,7 +45,8 @@ interface SubscribeDialogProps {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Auth-gated mailing-list subscribe confirmation. The form only mounts while
|
// Auth-gated mailing-list subscribe confirmation. The form only mounts while
|
||||||
// open so each open starts clean without a reset effect.
|
// open so each open starts clean without a reset effect. Dismiss is blocked
|
||||||
|
// while the mutation is in flight so a Cancel/Escape cannot race a reopen.
|
||||||
export function SubscribeDialog({
|
export function SubscribeDialog({
|
||||||
open,
|
open,
|
||||||
onOpenChange,
|
onOpenChange,
|
||||||
@@ -53,12 +54,23 @@ export function SubscribeDialog({
|
|||||||
viewerEmail,
|
viewerEmail,
|
||||||
organizationName,
|
organizationName,
|
||||||
}: SubscribeDialogProps) {
|
}: SubscribeDialogProps) {
|
||||||
|
const [isSubmitting, setIsSubmitting] = useState(false);
|
||||||
|
|
||||||
return (
|
return (
|
||||||
<Dialog open={open} onOpenChange={onOpenChange}>
|
<Dialog
|
||||||
|
open={open}
|
||||||
|
onOpenChange={(next) => {
|
||||||
|
if (!next && isSubmitting) {
|
||||||
|
return;
|
||||||
|
}
|
||||||
|
onOpenChange(next);
|
||||||
|
}}
|
||||||
|
>
|
||||||
<DialogPopup>
|
<DialogPopup>
|
||||||
{open && (
|
{open && (
|
||||||
<SubscribeForm
|
<SubscribeForm
|
||||||
onClose={() => onOpenChange(false)}
|
onClose={() => onOpenChange(false)}
|
||||||
|
onSubmittingChange={setIsSubmitting}
|
||||||
trustCenterId={trustCenterId}
|
trustCenterId={trustCenterId}
|
||||||
viewerEmail={viewerEmail}
|
viewerEmail={viewerEmail}
|
||||||
organizationName={organizationName}
|
organizationName={organizationName}
|
||||||
@@ -71,6 +83,7 @@ export function SubscribeDialog({
|
|||||||
|
|
||||||
interface SubscribeFormProps {
|
interface SubscribeFormProps {
|
||||||
onClose: () => void;
|
onClose: () => void;
|
||||||
|
onSubmittingChange: (submitting: boolean) => void;
|
||||||
trustCenterId: string;
|
trustCenterId: string;
|
||||||
viewerEmail: string;
|
viewerEmail: string;
|
||||||
organizationName: string;
|
organizationName: string;
|
||||||
@@ -78,23 +91,39 @@ interface SubscribeFormProps {
|
|||||||
|
|
||||||
function SubscribeForm({
|
function SubscribeForm({
|
||||||
onClose,
|
onClose,
|
||||||
|
onSubmittingChange,
|
||||||
trustCenterId,
|
trustCenterId,
|
||||||
viewerEmail,
|
viewerEmail,
|
||||||
organizationName,
|
organizationName,
|
||||||
}: SubscribeFormProps) {
|
}: SubscribeFormProps) {
|
||||||
const { t } = useTranslation("updates");
|
const { t } = useTranslation("updates");
|
||||||
const [subscribe, isSubscribing] = useSubscribeToMailingList(trustCenterId);
|
const [subscribe, isSubscribing] = useSubscribeToMailingList(trustCenterId);
|
||||||
const [submitted, setSubmitted] = useState(false);
|
const aliveRef = useRef(true);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
aliveRef.current = true;
|
||||||
|
return () => {
|
||||||
|
aliveRef.current = false;
|
||||||
|
};
|
||||||
|
}, []);
|
||||||
|
|
||||||
|
useEffect(() => {
|
||||||
|
onSubmittingChange(isSubscribing);
|
||||||
|
return () => {
|
||||||
|
onSubmittingChange(false);
|
||||||
|
};
|
||||||
|
}, [isSubscribing, onSubmittingChange]);
|
||||||
|
|
||||||
const onSubmit = async (event: FormEvent) => {
|
const onSubmit = async (event: FormEvent) => {
|
||||||
event.preventDefault();
|
event.preventDefault();
|
||||||
if (submitted) {
|
if (isSubscribing) {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
try {
|
try {
|
||||||
await subscribe();
|
await subscribe();
|
||||||
setSubmitted(true);
|
if (aliveRef.current) {
|
||||||
onClose();
|
onClose();
|
||||||
|
}
|
||||||
} catch {
|
} catch {
|
||||||
// Errors are surfaced by the mutation notifier; keep the form open.
|
// Errors are surfaced by the mutation notifier; keep the form open.
|
||||||
}
|
}
|
||||||
@@ -119,7 +148,13 @@ function SubscribeForm({
|
|||||||
</DialogBody>
|
</DialogBody>
|
||||||
|
|
||||||
<DialogFooter>
|
<DialogFooter>
|
||||||
<Button type="button" variant="soft" color="neutral" onClick={onClose}>
|
<Button
|
||||||
|
type="button"
|
||||||
|
variant="soft"
|
||||||
|
color="neutral"
|
||||||
|
disabled={isSubscribing}
|
||||||
|
onClick={onClose}
|
||||||
|
>
|
||||||
{t("dialog.cancel")}
|
{t("dialog.cancel")}
|
||||||
</Button>
|
</Button>
|
||||||
<Button type="submit" variant="solid" color="neutral" highContrast loading={isSubscribing}>
|
<Button type="submit" variant="solid" color="neutral" highContrast loading={isSubscribing}>
|
||||||
|
|||||||
@@ -43,8 +43,12 @@ export function useSignOut() {
|
|||||||
});
|
});
|
||||||
|
|
||||||
const signOut = useCallback(async () => {
|
const signOut = useCallback(async () => {
|
||||||
|
try {
|
||||||
await commit({ variables: {} });
|
await commit({ variables: {} });
|
||||||
window.location.reload();
|
window.location.reload();
|
||||||
|
} catch {
|
||||||
|
// errorToast already handles user-facing feedback.
|
||||||
|
}
|
||||||
}, [commit]);
|
}, [commit]);
|
||||||
|
|
||||||
return [signOut, isSigningOut] as const;
|
return [signOut, isSigningOut] as const;
|
||||||
|
|||||||
@@ -87,11 +87,17 @@ export function SubscribeDialogProvider({
|
|||||||
}, [openSignIn, viewer]);
|
}, [openSignIn, viewer]);
|
||||||
|
|
||||||
const unsubscribe = useCallback(async () => {
|
const unsubscribe = useCallback(async () => {
|
||||||
|
try {
|
||||||
await unsubscribeFromMailingList({ variables: {} });
|
await unsubscribeFromMailingList({ variables: {} });
|
||||||
|
} catch {
|
||||||
|
// Errors are surfaced by the mutation notifier.
|
||||||
|
}
|
||||||
}, [unsubscribeFromMailingList]);
|
}, [unsubscribeFromMailingList]);
|
||||||
|
|
||||||
// After a guest signs in to subscribe, they land back with the subscribe
|
// After a guest signs in to subscribe, they land back with the subscribe
|
||||||
// marker; open the dialog once and drop the marker so a reload can't re-open.
|
// marker; open the dialog once and drop the marker so a reload can't re-open.
|
||||||
|
// Derive the next params from the updater's previous snapshot so a concurrent
|
||||||
|
// effect that already cleared an access-request marker is not undone.
|
||||||
const resumed = useRef(false);
|
const resumed = useRef(false);
|
||||||
useEffect(() => {
|
useEffect(() => {
|
||||||
if (resumed.current || viewer == null || searchParams.get(SUBSCRIBE_PARAM) == null) {
|
if (resumed.current || viewer == null || searchParams.get(SUBSCRIBE_PARAM) == null) {
|
||||||
@@ -99,9 +105,11 @@ export function SubscribeDialogProvider({
|
|||||||
}
|
}
|
||||||
resumed.current = true;
|
resumed.current = true;
|
||||||
setDialogOpen(true);
|
setDialogOpen(true);
|
||||||
const next = new URLSearchParams(searchParams);
|
setSearchParams((previous) => {
|
||||||
|
const next = new URLSearchParams(previous);
|
||||||
next.delete(SUBSCRIBE_PARAM);
|
next.delete(SUBSCRIBE_PARAM);
|
||||||
setSearchParams(next, { replace: true });
|
return next;
|
||||||
|
}, { replace: true });
|
||||||
}, [viewer, searchParams, setSearchParams]);
|
}, [viewer, searchParams, setSearchParams]);
|
||||||
|
|
||||||
const value = useMemo(
|
const value = useMemo(
|
||||||
|
|||||||
@@ -196,14 +196,15 @@ func (r *mutationResolver) SignOut(ctx context.Context) (*types.SignOutPayload,
|
|||||||
|
|
||||||
err := r.iam.SessionService.CloseSession(ctx, session.ID)
|
err := r.iam.SessionService.CloseSession(ctx, session.ID)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
if _, ok := errors.AsType[*iam.ErrSessionNotFound](err); ok {
|
_, notFound := errors.AsType[*iam.ErrSessionNotFound](err)
|
||||||
return &types.SignOutPayload{Success: true}, nil
|
_, expired := errors.AsType[*iam.ErrSessionExpired](err)
|
||||||
}
|
if !notFound && !expired {
|
||||||
|
|
||||||
r.logger.ErrorCtx(ctx, "cannot close session", log.Error(err))
|
r.logger.ErrorCtx(ctx, "cannot close session", log.Error(err))
|
||||||
|
|
||||||
return nil, gqlutils.Internal(ctx)
|
return nil, gqlutils.Internal(ctx)
|
||||||
}
|
}
|
||||||
|
// Already closed or missing — still clear the cookie so the browser
|
||||||
|
// drops the stale session on concurrent / retried logout.
|
||||||
|
}
|
||||||
|
|
||||||
w := gqlutils.HTTPResponseWriterFromContext(ctx)
|
w := gqlutils.HTTPResponseWriterFromContext(ctx)
|
||||||
r.sessionCookie.Clear(w)
|
r.sessionCookie.Clear(w)
|
||||||
|
|||||||
Reference in New Issue
Block a user