mirror of
https://github.com/block/buzz.git
synced 2026-08-18 06:50:31 +02:00
fix(onboarding): skip community profile setup for existing relay members (#2300)
Signed-off-by: Will Pfleger <pfleger.will@gmail.com> Co-authored-by: npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7 <dcfd242e557282d7a1e2cf2e6877522682f1e5c6156dc92ca7d90eaedd3b0f95@sprout-oss.stage.blox.sqprod.co>
This commit is contained in:
co-authored by
npub1mn7jgtj4w2pd0g0zeuhxsa6jy6p0rewxz4kujt98my82ahfmp72sxjexk7
parent
a5c5bcc2e7
commit
25f631870e
+46
-4
@@ -20,6 +20,9 @@ import { useMachineOnboardingState } from "@/features/onboarding/machineOnboardi
|
||||
import {
|
||||
type FirstCommunityPage,
|
||||
useCommunityOnboarding,
|
||||
markCommunityOnboardingComplete,
|
||||
resolveProfileCheckAction,
|
||||
isTransactionStillConnecting,
|
||||
} from "@/features/onboarding/communityOnboarding";
|
||||
import { CommunityOnboardingFlow } from "@/features/onboarding/ui/CommunityOnboardingFlow";
|
||||
import {
|
||||
@@ -43,6 +46,7 @@ import { CommunityApplyErrorScreen } from "@/features/communities/ui/CommunityAp
|
||||
import { CommunityChangeOverlay } from "@/features/communities/ui/CommunityChangeOverlay";
|
||||
import { createBuzzQueryClient } from "@/shared/api/queryClient";
|
||||
import { isSharedIdentity as isSharedIdentityCmd } from "@/shared/api/tauri";
|
||||
import { getProfile } from "@/shared/api/tauriProfiles";
|
||||
import {
|
||||
type AddCommunityDeepLinkPayload,
|
||||
listenForDeepLinks,
|
||||
@@ -279,6 +283,14 @@ function CommunityApp({
|
||||
} = useCommunities();
|
||||
const communityOnboarding = useCommunityOnboarding();
|
||||
const connectingTransactionRef = useRef<string | null>(null);
|
||||
// Tracks the ID of the profile-check request that has been launched for the
|
||||
// current connecting transaction. Prevents the effect from launching a
|
||||
// second request if it re-runs while a fetch is in flight.
|
||||
const profileCheckTransactionRef = useRef<string | null>(null);
|
||||
// Always reflects the live transaction object so async callbacks can perform
|
||||
// an atomic check of both ID and stage before mutating state.
|
||||
const transactionRef = useRef(communityOnboarding.transaction);
|
||||
transactionRef.current = communityOnboarding.transaction;
|
||||
const [isCommunityChangeOpen, setIsCommunityChangeOpen] = useState(false);
|
||||
const [resumeFirstCommunityPage, setResumeFirstCommunityPage] =
|
||||
useState<FirstCommunityPage | null>(null);
|
||||
@@ -384,6 +396,7 @@ function CommunityApp({
|
||||
useEffect(() => {
|
||||
if (transaction?.stage !== "connecting") {
|
||||
connectingTransactionRef.current = null;
|
||||
profileCheckTransactionRef.current = null;
|
||||
}
|
||||
}, [transaction?.stage]);
|
||||
const targetIsReady =
|
||||
@@ -391,10 +404,39 @@ function CommunityApp({
|
||||
community.isReady &&
|
||||
community.appliedKey === communityKey;
|
||||
useEffect(() => {
|
||||
if (transaction?.stage === "connecting" && targetIsReady) {
|
||||
communityOnboarding.update({ stage: "profile", error: undefined });
|
||||
}
|
||||
}, [communityOnboarding.update, targetIsReady, transaction?.stage]);
|
||||
if (transaction?.stage !== "connecting" || !targetIsReady) return;
|
||||
const transactionId = transaction.id;
|
||||
const relayUrl = transaction.relayUrl;
|
||||
if (profileCheckTransactionRef.current === transactionId) return;
|
||||
profileCheckTransactionRef.current = transactionId;
|
||||
|
||||
// resolveProfileCheckAction resolves exactly once (Promise.race + timer
|
||||
// cleared on settle), so no settled flag is needed here.
|
||||
void resolveProfileCheckAction(getProfile, 10_000).then((result) => {
|
||||
// Atomic staleness guard via isTransactionStillConnecting: the
|
||||
// transaction must still be the same one that launched this request
|
||||
// AND still be in connecting. Covers cancel+replacement (B's ID !== A's)
|
||||
// and cancel-without-replacement (transactionRef.current is null).
|
||||
if (!isTransactionStillConnecting(transactionRef.current, transactionId))
|
||||
return;
|
||||
|
||||
if (result.action === "skip") {
|
||||
markCommunityOnboardingComplete(result.profile.pubkey, relayUrl);
|
||||
communityOnboarding.clear();
|
||||
} else {
|
||||
communityOnboarding.update(
|
||||
{ stage: "profile", error: undefined },
|
||||
transactionId,
|
||||
);
|
||||
}
|
||||
});
|
||||
}, [
|
||||
communityOnboarding,
|
||||
targetIsReady,
|
||||
transaction?.stage,
|
||||
transaction?.id,
|
||||
transaction?.relayUrl,
|
||||
]);
|
||||
// During "entering" the transaction stays alive as a curtain: the app mounts
|
||||
// underneath (already pointed at the Welcome channel route) while the
|
||||
// onboarding screen covers it, then fades once Welcome reports ready.
|
||||
|
||||
@@ -3,8 +3,11 @@ import test from "node:test";
|
||||
|
||||
import {
|
||||
clearCommunityOnboardingTransaction,
|
||||
isTransactionStillConnecting,
|
||||
loadCommunityOnboardingTransaction,
|
||||
markCommunityOnboardingComplete,
|
||||
resolveProfileCheckAction,
|
||||
shouldSkipCommunityOnboarding,
|
||||
startCommunityOnboarding,
|
||||
updateCommunityOnboardingTransaction,
|
||||
updateCurrentCommunityOnboardingTransaction,
|
||||
@@ -147,3 +150,195 @@ test("completion is scoped by relay and pubkey and preserves legacy gate", () =>
|
||||
);
|
||||
assert.equal(storage.getItem("buzz-onboarding-complete.v1:pubkey"), "true");
|
||||
});
|
||||
|
||||
// ── shouldSkipCommunityOnboarding ────────────────────────────────────────────
|
||||
|
||||
/** Minimal Profile stub — only the fields the helper inspects. */
|
||||
function makeProfile(hasProfileEvent, overrides = {}) {
|
||||
return {
|
||||
pubkey: "aabbcc",
|
||||
displayName: null,
|
||||
avatarUrl: null,
|
||||
about: null,
|
||||
nip05Handle: null,
|
||||
ownerPubkey: null,
|
||||
hasProfileEvent,
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
test("shouldSkipCommunityOnboarding_hasProfileEvent_returnsTrue", () => {
|
||||
assert.equal(
|
||||
shouldSkipCommunityOnboarding(makeProfile(true)),
|
||||
true,
|
||||
"existing kind:0 ⇒ skip onboarding",
|
||||
);
|
||||
});
|
||||
|
||||
test("shouldSkipCommunityOnboarding_noProfileEvent_returnsFalse", () => {
|
||||
assert.equal(
|
||||
shouldSkipCommunityOnboarding(makeProfile(false)),
|
||||
false,
|
||||
"no kind:0 ⇒ show profile step",
|
||||
);
|
||||
});
|
||||
|
||||
test("shouldSkipCommunityOnboarding_fetchError_returnsFalse", () => {
|
||||
// fetch error is represented as null — must never block onboarding
|
||||
assert.equal(
|
||||
shouldSkipCommunityOnboarding(null),
|
||||
false,
|
||||
"fetch error ⇒ show profile step (safe fallback)",
|
||||
);
|
||||
});
|
||||
|
||||
// ── resolveProfileCheckAction — async orchestration ──────────────────────────
|
||||
|
||||
/**
|
||||
* Returns a fake scheduleTimeout that captures registered callbacks so tests
|
||||
* can fire or skip them manually without real timers.
|
||||
*/
|
||||
function makeScheduler() {
|
||||
const callbacks = [];
|
||||
return {
|
||||
schedule: (fn, _ms) => callbacks.push(fn),
|
||||
fireTimeout: () => {
|
||||
const fn = callbacks.shift();
|
||||
if (fn) fn();
|
||||
},
|
||||
pendingCount: () => callbacks.length,
|
||||
};
|
||||
}
|
||||
|
||||
test("resolveProfileCheckAction_hasProfileEvent_returnsSkipWithProfile", async () => {
|
||||
const profile = makeProfile(true, { pubkey: "aabbcc" });
|
||||
const result = await resolveProfileCheckAction(
|
||||
() => Promise.resolve(profile),
|
||||
10_000,
|
||||
makeScheduler().schedule,
|
||||
);
|
||||
assert.equal(result.action, "skip");
|
||||
assert.equal(result.profile.pubkey, "aabbcc");
|
||||
});
|
||||
|
||||
test("resolveProfileCheckAction_noProfileEvent_returnsShowProfile", async () => {
|
||||
const result = await resolveProfileCheckAction(
|
||||
() => Promise.resolve(makeProfile(false)),
|
||||
10_000,
|
||||
makeScheduler().schedule,
|
||||
);
|
||||
assert.equal(result.action, "show-profile");
|
||||
});
|
||||
|
||||
test("resolveProfileCheckAction_fetchRejects_returnsShowProfile", async () => {
|
||||
const result = await resolveProfileCheckAction(
|
||||
() => Promise.reject(new Error("network error")),
|
||||
10_000,
|
||||
makeScheduler().schedule,
|
||||
);
|
||||
assert.equal(result.action, "show-profile");
|
||||
});
|
||||
|
||||
test("resolveProfileCheckAction_timeout_returnsShowProfile", async () => {
|
||||
// Fetch never settles; scheduler fires the timeout immediately.
|
||||
const scheduler = makeScheduler();
|
||||
const result = await resolveProfileCheckAction(
|
||||
() => new Promise(() => {}), // hangs forever
|
||||
10_000,
|
||||
(fn, ms) => {
|
||||
scheduler.schedule(fn, ms);
|
||||
// Fire synchronously so the test does not wait for a real timer.
|
||||
scheduler.fireTimeout();
|
||||
},
|
||||
);
|
||||
assert.equal(
|
||||
result.action,
|
||||
"show-profile",
|
||||
"timeout ⇒ show-profile (never strands onboarding)",
|
||||
);
|
||||
});
|
||||
|
||||
test("resolveProfileCheckAction_lateSuccessAfterTimeout_doesNotSkip", async () => {
|
||||
// Fetch resolves AFTER the timeout has already fired.
|
||||
// resolveProfileCheckAction must return show-profile from the timeout path,
|
||||
// and the late fetch result must have no effect.
|
||||
let resolveFetch;
|
||||
const fetchPromise = new Promise((resolve) => {
|
||||
resolveFetch = resolve;
|
||||
});
|
||||
|
||||
const scheduler = makeScheduler();
|
||||
const resultPromise = resolveProfileCheckAction(
|
||||
() => fetchPromise,
|
||||
10_000,
|
||||
scheduler.schedule,
|
||||
);
|
||||
|
||||
// Fire the timeout — race settles with the timeout rejection.
|
||||
scheduler.fireTimeout();
|
||||
|
||||
// Now resolve the fetch with a kind:0 profile.
|
||||
resolveFetch(makeProfile(true));
|
||||
|
||||
const result = await resultPromise;
|
||||
assert.equal(
|
||||
result.action,
|
||||
"show-profile",
|
||||
"late success after timeout must not complete onboarding",
|
||||
);
|
||||
});
|
||||
|
||||
// ── isTransactionStillConnecting — stale-transaction guard ───────────────────
|
||||
|
||||
/**
|
||||
* Builds a minimal transaction stub for testing the guard predicate.
|
||||
* Only `id` and `stage` are inspected by isTransactionStillConnecting.
|
||||
*/
|
||||
function makeTransaction(id, stage) {
|
||||
return {
|
||||
id,
|
||||
stage,
|
||||
source: "add-community",
|
||||
relayUrl: "wss://relay.example",
|
||||
communityName: "test",
|
||||
createdAt: new Date().toISOString(),
|
||||
updatedAt: new Date().toISOString(),
|
||||
};
|
||||
}
|
||||
|
||||
test("isTransactionStillConnecting_matchingIdAndStage_returnsTrue", () => {
|
||||
assert.equal(
|
||||
isTransactionStillConnecting(makeTransaction("tx-a", "connecting"), "tx-a"),
|
||||
true,
|
||||
"same id + connecting stage → guard passes",
|
||||
);
|
||||
});
|
||||
|
||||
test("isTransactionStillConnecting_replacedTransaction_returnsFalse", () => {
|
||||
// Transaction B replaced A while the fetch was in flight.
|
||||
// The callback for A must not clear B.
|
||||
assert.equal(
|
||||
isTransactionStillConnecting(makeTransaction("tx-b", "connecting"), "tx-a"),
|
||||
false,
|
||||
"different id (replacement) → guard rejects stale success",
|
||||
);
|
||||
});
|
||||
|
||||
test("isTransactionStillConnecting_cancelWithNoReplacement_returnsFalse", () => {
|
||||
// User cancelled without starting a new transaction; ref is null.
|
||||
assert.equal(
|
||||
isTransactionStillConnecting(null, "tx-a"),
|
||||
false,
|
||||
"null ref (cancel without replacement) → guard rejects stale success",
|
||||
);
|
||||
});
|
||||
|
||||
test("isTransactionStillConnecting_stagePastConnecting_returnsFalse", () => {
|
||||
// Fallback already advanced the transaction to 'profile' before the skip
|
||||
// result arrived — double-completion must not occur.
|
||||
assert.equal(
|
||||
isTransactionStillConnecting(makeTransaction("tx-a", "profile"), "tx-a"),
|
||||
false,
|
||||
"stage advanced past connecting → guard rejects late action",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
normalizeRelayUrl,
|
||||
} from "@/features/communities/communityStorage";
|
||||
import { setLocalStorageItemWithRecovery } from "@/shared/lib/localStorageQuota";
|
||||
import type { Profile } from "@/shared/api/types";
|
||||
|
||||
const STORAGE_KEY = "buzz-community-onboarding-transaction.v1";
|
||||
|
||||
@@ -227,6 +228,90 @@ export function markCommunityOnboardingComplete(
|
||||
storage.setItem(`buzz-onboarding-complete.v1:${pubkey}`, "true");
|
||||
}
|
||||
|
||||
/**
|
||||
* Returns true when a relay-profile check result means the user should skip
|
||||
* community onboarding entirely and land directly in the app.
|
||||
*
|
||||
* A profile fetch error is represented as `null` and always returns false so
|
||||
* that the fallback (show the profile step) applies — the skip must never
|
||||
* block or strand onboarding.
|
||||
*/
|
||||
export function shouldSkipCommunityOnboarding(
|
||||
profile: Profile | null,
|
||||
): boolean {
|
||||
return profile !== null && profile.hasProfileEvent === true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Outcome of a profile-check attempt during the connecting → profile
|
||||
* transition. Produced by `resolveProfileCheckAction`.
|
||||
*
|
||||
* - `{ action: "skip", profile }` — kind:0 exists; mark complete and enter
|
||||
* the app. The resolved `Profile` is included so callers have the pubkey
|
||||
* for `markCommunityOnboardingComplete` without a second fetch.
|
||||
* - `{ action: "show-profile" }` — no kind:0, or the fetch failed / timed
|
||||
* out; show the profile setup step.
|
||||
*/
|
||||
export type ProfileCheckAction =
|
||||
| { action: "skip"; profile: Profile }
|
||||
| { action: "show-profile" };
|
||||
|
||||
/**
|
||||
* Returns true when a live transaction snapshot still represents the
|
||||
* same connecting request that launched the profile check.
|
||||
*
|
||||
* Extracted as a pure predicate so the stale-result guard in App.tsx can
|
||||
* be unit-tested without mounting a component.
|
||||
*/
|
||||
export function isTransactionStillConnecting(
|
||||
live: CommunityOnboardingTransaction | null | undefined,
|
||||
transactionId: string,
|
||||
): boolean {
|
||||
return live?.id === transactionId && live.stage === "connecting";
|
||||
}
|
||||
|
||||
/**
|
||||
* Runs a bounded profile fetch and returns the action to take at the
|
||||
* `connecting → profile` transition.
|
||||
*
|
||||
* Accepts `fetchProfile`, `timeoutMs`, and `scheduleTimeout` as parameters so
|
||||
* callers (and tests) can supply controlled implementations. `scheduleTimeout`
|
||||
* must return a cancellation handle (like `window.setTimeout`) so the timer
|
||||
* can be cleared when the fetch settles before the deadline.
|
||||
*
|
||||
* Any fetch error or timeout → `{ action: "show-profile" }` (never strands
|
||||
* onboarding).
|
||||
*/
|
||||
export async function resolveProfileCheckAction(
|
||||
fetchProfile: () => Promise<Profile>,
|
||||
timeoutMs: number,
|
||||
scheduleTimeout: (
|
||||
fn: () => void,
|
||||
ms: number,
|
||||
) => ReturnType<typeof setTimeout> = (fn, ms) => window.setTimeout(fn, ms),
|
||||
): Promise<ProfileCheckAction> {
|
||||
let timerId: ReturnType<typeof setTimeout> | undefined;
|
||||
try {
|
||||
const profile = await Promise.race([
|
||||
fetchProfile(),
|
||||
new Promise<never>(
|
||||
(_, reject) =>
|
||||
(timerId = scheduleTimeout(
|
||||
() => reject(new Error("profile-check-timeout")),
|
||||
timeoutMs,
|
||||
)),
|
||||
),
|
||||
]);
|
||||
return shouldSkipCommunityOnboarding(profile)
|
||||
? { action: "skip", profile }
|
||||
: { action: "show-profile" };
|
||||
} catch {
|
||||
return { action: "show-profile" };
|
||||
} finally {
|
||||
if (timerId !== undefined) clearTimeout(timerId);
|
||||
}
|
||||
}
|
||||
|
||||
import * as React from "react";
|
||||
|
||||
type CommunityOnboardingContextValue = {
|
||||
|
||||
@@ -20,7 +20,7 @@ import {
|
||||
parseEmojiAvatarDataUrl,
|
||||
ProfileAvatarEditor,
|
||||
} from "@/features/profile/ui/ProfileAvatarEditor";
|
||||
import { updateProfile } from "@/shared/api/tauriProfiles";
|
||||
import { getProfile, updateProfile } from "@/shared/api/tauriProfiles";
|
||||
import { getIdentity, importIdentity } from "@/shared/api/tauriIdentity";
|
||||
import { listPersonas } from "@/shared/api/tauriPersonas";
|
||||
import { relayClient } from "@/shared/api/relayClient";
|
||||
@@ -286,6 +286,30 @@ export function CommunityOnboardingFlow({
|
||||
transaction?.stage === "finalizing" ||
|
||||
transaction?.stage === "entering";
|
||||
|
||||
// Seed display name and avatar from the relay profile when the profile step
|
||||
// is shown. This covers the case where the skip raced or was bypassed (e.g.,
|
||||
// the user navigated Back). Only seeds fields that are still empty so that
|
||||
// any user edits are preserved.
|
||||
React.useEffect(() => {
|
||||
if (!isProfileStage) return;
|
||||
void getProfile()
|
||||
.then((profile) => {
|
||||
if (profile.displayName) {
|
||||
setDisplayName((prev) =>
|
||||
prev === "" ? (profile.displayName ?? "") : prev,
|
||||
);
|
||||
}
|
||||
if (profile.avatarUrl) {
|
||||
setAvatarUrl((prev) =>
|
||||
prev === "" ? (profile.avatarUrl ?? "") : prev,
|
||||
);
|
||||
}
|
||||
})
|
||||
.catch(() => {
|
||||
// Seeding is best-effort; silently ignore failures.
|
||||
});
|
||||
}, [isProfileStage]);
|
||||
|
||||
React.useLayoutEffect(() => {
|
||||
if (isProfileStage && !isAvatarEditorOpen) {
|
||||
nameInputRef.current?.focus();
|
||||
|
||||
@@ -113,9 +113,15 @@ test("connect deep link shows a static acknowledgment during setup", async ({
|
||||
test("add-community deep link starts onboarding when no community is configured", async ({
|
||||
page,
|
||||
}) => {
|
||||
// profileReadError forces the fallback path (error → profile step), so the
|
||||
// test asserts pre-existing-profile behavior without the default mock
|
||||
// identity's has_profile_event:true triggering the skip.
|
||||
await installMockBridge(
|
||||
page,
|
||||
{ pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK] },
|
||||
{
|
||||
pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK],
|
||||
profileReadError: "no-kind-0",
|
||||
},
|
||||
{ skipCommunitySeed: true },
|
||||
);
|
||||
await page.goto("/");
|
||||
@@ -142,6 +148,26 @@ test("add-community deep link starts onboarding when no community is configured"
|
||||
.toContain('"communityName":"Acme Team"');
|
||||
});
|
||||
|
||||
test("add-community deep link skips profile step when identity has an existing kind:0 profile", async ({
|
||||
page,
|
||||
}) => {
|
||||
// The default mock identity (deadbeef...) is pre-seeded with
|
||||
// has_profile_event:true. The skip should fire on connecting → clear the
|
||||
// transaction entirely, never showing the profile step.
|
||||
await installMockBridge(
|
||||
page,
|
||||
{ pendingCommunityDeepLinks: [PENDING_ADD_COMMUNITY_LINK] },
|
||||
{ skipCommunitySeed: true },
|
||||
);
|
||||
await page.goto("/");
|
||||
|
||||
// Onboarding flow must disappear — the skip cleared the transaction.
|
||||
await expect(page.getByTestId("community-onboarding-flow")).toHaveCount(0);
|
||||
// handleCommunityOnboardingConnect already added the community when the
|
||||
// transaction reached "connecting", so the app lands in the full UI.
|
||||
await expect(page.getByTestId("sidebar-profile-avatar-button")).toBeVisible();
|
||||
});
|
||||
|
||||
test("add-community deep link opens one editable prefill and acknowledges the queue", async ({
|
||||
page,
|
||||
}) => {
|
||||
|
||||
Reference in New Issue
Block a user