Updated activitypub Bluesky sharing enablement flow#25189
Conversation
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThis PR bumps apps/activitypub to 2.0.0 and migrates Bluesky endpoints from v1 to v2. ActivityPub API: Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
mike182uk
force-pushed
the
ap-bluesky-handle-confirmation
branch
2 times, most recently
from
6684dcb to
65a2079
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (4)
apps/activitypub/src/api/activitypub.ts (1)
661-666: Add explicit return types for consistency.Most methods specify Promise. Make enableBluesky/disableBluesky explicit.
- async enableBluesky() { + async enableBluesky(): Promise<void> { const url = new URL('.ghost/activitypub/v1/actions/bluesky/enable', this.apiUrl); await this.fetchJSON(url, 'POST'); } - async disableBluesky() { + async disableBluesky(): Promise<void> { const url = new URL('.ghost/activitypub/v1/actions/bluesky/disable', this.apiUrl); await this.fetchJSON(url, 'POST'); }Also applies to: 667-672
apps/activitypub/src/api/activitypub.test.ts (2)
1639-1642: Optionally assert auth and accept headers for disable.Now disable uses fetchJSON (adds Authorization and Accept). Add header assertions for parity with other tests.
- [`https://activitypub.api/.ghost/activitypub/v1/actions/bluesky/disable`]: { - async assert(_resource, init) { - expect(init?.method).toEqual('POST'); - }, + [`https://activitypub.api/.ghost/activitypub/v1/actions/bluesky/disable`]: { + async assert(_resource, init) { + expect(init?.method).toEqual('POST'); + const headers = new Headers(init?.headers); + expect(headers.get('Authorization')).toContain('fake-token'); + expect(headers.get('Accept')).toEqual('application/activity+json'); + }, response: JSONResponse(null) }Based on learnings
Also applies to: 1654-1654
1668-1767: Add request assertions in confirmBlueskyHandle tests.Assert method and headers on the confirm-handle endpoint at least in the “confirms the bluesky handle” test.
- [`https://activitypub.api/.ghost/activitypub/v1/actions/bluesky/confirm-handle`]: { - response: JSONResponse({ - handle: 'foo@bar.baz' - }) - } + [`https://activitypub.api/.ghost/activitypub/v1/actions/bluesky/confirm-handle`]: { + async assert(_resource, init) { + expect(init?.method).toEqual('POST'); + const headers = new Headers(init?.headers); + expect(headers.get('Authorization')).toContain('fake-token'); + expect(headers.get('Accept')).toEqual('application/activity+json'); + }, + response: JSONResponse({ + handle: 'foo@bar.baz' + }) + }apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
82-116: Kick off confirmation immediately and avoid overlapping requests.
- Fire an immediate confirm before starting the interval to reduce perceived latency.
- Optionally guard against overlapping requests with a simple in-flight ref.
- const confirmHandleInterval = setInterval(() => { + // Trigger immediately + confirmHandle(); + + const confirmHandleInterval = setInterval(() => { if (retryCountRef.current >= MAX_CONFIRMATION_RETRIES) { clearInterval(confirmHandleInterval); return; } retryCountRef.current += 1; confirmHandle(); }, CONFIRMATION_INTERVAL);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/activitypub/package.json(1 hunks)apps/activitypub/src/api/activitypub.test.ts(10 hunks)apps/activitypub/src/api/activitypub.ts(2 hunks)apps/activitypub/src/hooks/use-activity-pub-queries.ts(4 hunks)apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx(4 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-22T08:55:11.602Z
Learnt from: mike182uk
PR: TryGhost/Ghost#24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/api/activitypub.ts
🧬 Code graph analysis (2)
apps/activitypub/src/api/activitypub.test.ts (1)
apps/activitypub/src/api/activitypub.ts (1)
ActivityPubAPI(215-684)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (4)
useAccountForUser(1609-1621)useEnableBlueskyMutationForUser(2642-2672)useDisableBlueskyMutationForUser(2674-2704)useConfirmBlueskyHandleMutationForUser(2706-2735)
🔇 Additional comments (8)
apps/activitypub/package.json (1)
3-3: Version bump to 1.0.16 looks good.apps/activitypub/src/api/activitypub.ts (2)
29-31: Account model changes look correct.Nullable blueskyHandle and added blueskyHandleConfirmed align with the new confirm flow.
673-683: confirmBlueskyHandle implementation LGTM.Defensive empty-string fallback is fine for the polling UX.
apps/activitypub/src/api/activitypub.test.ts (1)
1611-1614: enableBluesky test updates LGTM.POST assertion and void return check align with new API.
Also applies to: 1625-1625
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
181-216: Confirmed state UI looks good.Copy, avatar, and external profile link behave as expected.
apps/activitypub/src/hooks/use-activity-pub-queries.ts (3)
2652-2657: Enable onSuccess cache update LGTM.Sets enabled, resets confirmation and handle as expected.
2685-2689: Disable onSuccess cache update LGTM.Clears enabled/confirmed/handle and invalidates follows.
2706-2735: confirmBlueskyHandle mutation LGTM.Only updates cache on non-empty handle; error handling matches rate-limit UX.
| const confirmHandle = useCallback(() => { | ||
| confirmBlueskyHandleMutation.mutateAsync().then((handle) => { | ||
| if (handle) { | ||
| setHandleConfirmed(true); | ||
| } | ||
| }); | ||
| }, [confirmBlueskyHandleMutation]); | ||
|
|
There was a problem hiding this comment.
Catch promise to avoid unhandled rejections during polling.
mutateAsync rejects on error; without catch, interval-triggered calls can cause unhandled rejections.
- const confirmHandle = useCallback(() => {
- confirmBlueskyHandleMutation.mutateAsync().then((handle) => {
- if (handle) {
- setHandleConfirmed(true);
- }
- });
- }, [confirmBlueskyHandleMutation]);
+ const confirmHandle = useCallback(async () => {
+ try {
+ const handle = await confirmBlueskyHandleMutation.mutateAsync();
+ if (handle) {
+ setHandleConfirmed(true);
+ }
+ } catch {
+ // no-op; onError in the mutation already surfaces rate-limit errors
+ }
+ }, [confirmBlueskyHandleMutation]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const confirmHandle = useCallback(() => { | |
| confirmBlueskyHandleMutation.mutateAsync().then((handle) => { | |
| if (handle) { | |
| setHandleConfirmed(true); | |
| } | |
| }); | |
| }, [confirmBlueskyHandleMutation]); | |
| const confirmHandle = useCallback(async () => { | |
| try { | |
| const handle = await confirmBlueskyHandleMutation.mutateAsync(); | |
| if (handle) { | |
| setHandleConfirmed(true); | |
| } | |
| } catch { | |
| // no-op; onError in the mutation already surfaces rate-limit errors | |
| } | |
| }, [confirmBlueskyHandleMutation]); |
🤖 Prompt for AI Agents
In apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx around
lines 74 to 81, the confirmHandle callback calls
confirmBlueskyHandleMutation.mutateAsync() without handling rejections, which
can produce unhandled promise rejections when run from an interval; wrap the
call in error handling by either making confirmHandle async and awaiting
mutateAsync inside a try/catch that logs or ignores the error, or append a
.catch(...) to the promise chain to handle failures and still set state on
success; ensure any caught errors are logged via the app logger or console so
failures during polling are not swallowed.
b66c802 to
2121679
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is being reviewed by Cursor Bugbot
Details
Your team is on the Bugbot Free tier. On this plan, Bugbot will review limited PRs each billing cycle for each member of your team.
To receive Bugbot reviews on all of your PRs, visit the Cursor dashboard to activate Pro and start your 14-day free trial.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
73-81: Catch promise rejection to prevent unhandled errors during polling.The
mutateAsync()call can reject on errors (e.g., rate limits, network failures), and without a.catch()handler, the interval-triggered calls will cause unhandled promise rejections.Apply this diff to handle rejections:
- const confirmHandle = useCallback(() => { - confirmBlueskyHandleMutation.mutateAsync().then((handle) => { - if (handle) { - setHandleConfirmed(true); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); // Empty deps - mutations are stable in practice + const confirmHandle = useCallback(async () => { + try { + const handle = await confirmBlueskyHandleMutation.mutateAsync(); + if (handle) { + setHandleConfirmed(true); + } + } catch { + // no-op; onError in the mutation already surfaces rate-limit errors + } + }, [confirmBlueskyHandleMutation]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/src/components/**/*.{tsx,ts} : Prefer compound subcomponents (e.g., Header.Title/Meta/Actions) with small focused parts attached as static properties and exported as named exports
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-07-14T12:20:35.268Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-10-15T07:53:49.814Z
Learnt from: CR
Repo: TryGhost/Ghost PR: 0
File: apps/shade/AGENTS.md:0-0
Timestamp: 2025-10-15T07:53:49.814Z
Learning: Applies to apps/shade/**/*.{ts,tsx} : Use React + TypeScript and prefer composable components over heavy prop configurations
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
🧬 Code graph analysis (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (4)
useAccountForUser(1615-1627)useEnableBlueskyMutationForUser(2685-2715)useDisableBlueskyMutationForUser(2717-2747)useConfirmBlueskyHandleMutationForUser(2749-2778)
🔇 Additional comments (4)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (4)
4-4: LGTM!The new imports and configuration constants appropriately support the polling-based confirmation workflow.
Also applies to: 25-28
32-32: LGTM!State initialization and refs are appropriate for the confirmation workflow. Using
useReffor retry count avoids unnecessary re-renders.Also applies to: 36-37, 40-40
173-187: LGTM!The loading state UI clearly communicates the handle creation process, and the helper text about leaving the page provides good UX during the potentially lengthy confirmation wait.
140-140: LGTM!The confirmed state UI is well-designed with clear visual indicators (avatar badge, handle display, copy functionality) and the profile link correctly strips the
@prefix for the Bluesky URL.Also applies to: 192-230
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
2664-2683: Type safety issue flagged in previous review still present.The
currentProfileparameter inupdateAccountBlueskyCacheis typed asBlueskyDetails, but the account cache atQUERY_KEYS.account('index')storesAccountobjects. This weakens type safety as previously noted.
🧹 Nitpick comments (2)
apps/activitypub/src/api/activitypub.ts (2)
666-670: Consider explicit return type annotation.While the method correctly returns
Promise<void>implicitly, adding an explicit annotation would improve clarity:-async enableBluesky() { +async enableBluesky(): Promise<void> {This is consistent with methods like
confirmBlueskyHandle(line 678) that do include explicit return types.
672-676: Consider explicit return type annotation.Same suggestion as
enableBluesky- add explicitPromise<void>return type for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/activitypub/package.json(1 hunks)apps/activitypub/src/api/activitypub.ts(2 hunks)apps/activitypub/src/hooks/use-activity-pub-queries.ts(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/activitypub/package.json
🧰 Additional context used
🧠 Learnings (5)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
📚 Learning: 2025-08-22T08:55:11.602Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Applied to files:
apps/activitypub/src/api/activitypub.tsapps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-11-05T16:49:21.083Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/src/hooks/user-preferences.ts:20-20
Timestamp: 2025-11-05T16:49:21.083Z
Learning: In apps/admin/src/hooks/user-preferences.ts, the query key for user preferences intentionally includes `user?.accessibility` (not just `user?.id`) so the cache automatically reacts to changes from any source (useEditUserPreferences, other editUser calls, external updates). Combined with `cacheTime: 0`, this design garbage-collects orphaned entries while keeping the active entry cached. This is a documented, intentional architectural decision.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
🧬 Code graph analysis (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
apps/admin-x-framework/src/index.ts (1)
useQueryClient(44-44)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Setup
- GitHub Check: Build & Push
- GitHub Check: Setup
🔇 Additional comments (4)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (2)
2685-2747: LGTM: Enable/disable mutations correctly manage Bluesky state.The consolidated cache updates using
BlueskyDetailsproperly reflect the Bluesky lifecycle:
- Enable sets
blueskyEnabled: truewith unconfirmed state- Disable resets all fields
- Both appropriately invalidate the following query
2749-2778: LGTM: Confirm mutation correctly handles polling workflow.The early return when
blueskyHandle === ''(line 2762) is appropriate for the polling scenario, keeping the existing cache state until confirmation succeeds. Error handling for rate limiting is properly implemented.apps/activitypub/src/api/activitypub.ts (2)
29-30: LGTM: Account interface correctly models Bluesky confirmation state.The addition of
blueskyHandleConfirmedand makingblueskyHandlenullable properly represents the three-state workflow (disabled, enabled-unconfirmed, enabled-confirmed).
678-688: LGTM: Confirm handle method properly handles response variations.The null/invalid response handling (returning empty string) correctly supports the polling workflow where the handle may not be immediately available.
Minor note: The
String()conversion on line 687 is redundant given thetypeof json.handle !== 'string'check, but it's harmless defensive coding.
E2E Tests FailedTo view the Playwright test report locally, run: REPORT_DIR=$(mktemp -d) && gh run download 19220515993 -n playwright-report -D "$REPORT_DIR" && npx playwright show-report "$REPORT_DIR" |
mike182uk
force-pushed
the
ap-bluesky-handle-confirmation
branch
from
79eddba to
b8dd7a6
Compare
| toast.error('Something went wrong, please try again.'); | ||
|
|
||
| await disableBlueskyMutation.mutateAsync(); | ||
| setLoading(false); |
There was a problem hiding this comment.
Bug: Unhandled error leaves loading state stuck
When confirmation retries are exhausted, disableBlueskyMutation.mutateAsync() is called without error handling. If this call fails, the subsequent setLoading(false) won't execute, leaving the component stuck in a loading state indefinitely with no way for the user to recover.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
2725-2839: Unify Bluesky cache typing withAccountto keep type-safety.The Bluesky enable/disable/confirm flows and cache merges look good, but
updateAccountBlueskyCachecurrently treats theaccount('index')cache asBlueskyDetailsrather thanAccount, which weakens typings compared to the rest of this file where that key holdsAccount. You can keep theBlueskyDetailspayload while preserving theAccount-shaped cache:-type BlueskyDetails = { - blueskyEnabled: boolean; - blueskyHandleConfirmed: boolean; - blueskyHandle: string | null; -} - -function updateAccountBlueskyCache(queryClient: QueryClient, blueskyDetails: BlueskyDetails) { +type BlueskyDetails = Pick<Account, 'blueskyEnabled' | 'blueskyHandleConfirmed' | 'blueskyHandle'>; + +function updateAccountBlueskyCache( + queryClient: QueryClient, + blueskyDetails: BlueskyDetails +) { const profileQueryKey = QUERY_KEYS.account('index'); - queryClient.setQueryData(profileQueryKey, (currentProfile?: BlueskyDetails) => { + queryClient.setQueryData(profileQueryKey, (currentProfile?: Account) => { if (!currentProfile) { return currentProfile; } return { ...currentProfile, ...blueskyDetails }; }); }This keeps the shared cache consistent while still centralizing Bluesky updates.
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
48-71: GuardmutateAsynccalls to avoid unhandled rejections in polling/disable flows.
mutateAsyncrejects on failure; the current uses can surface unhandled promise rejections:
confirmHandleuses.then(...)with no.catch(...).- The confirmation interval
await disableBlueskyMutation.mutateAsync();has no try/catch.handleDisableusesawait disableBlueskyMutation.mutateAsync();inside atrywithout acatch.Because neither the interval nor the click handler ever
awaitthese calls from outside, any rejection escapes as an unhandled rejection. You can keep error surfacing in the hook’sonErrorwhile making these callers safe:-const handleDisable = async () => { - setLoading(true); - try { - await disableBlueskyMutation.mutateAsync(); - setShowConfirm(false); - toast.success('Bluesky sharing disabled'); - } finally { - setLoading(false); - } -}; +const handleDisable = async () => { + setLoading(true); + try { + await disableBlueskyMutation.mutateAsync(); + setShowConfirm(false); + toast.success('Bluesky sharing disabled'); + } catch { + // onError from the mutation already handles surfacing errors (e.g. rate limits) + } finally { + setLoading(false); + } +}; -const confirmHandle = useCallback(() => { - confirmBlueskyHandleMutation.mutateAsync().then((handle) => { - if (handle) { - setHandleConfirmed(true); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps -}, []); // Empty deps - mutations are stable in practice +const confirmHandle = useCallback(async () => { + try { + const handle = await confirmBlueskyHandleMutation.mutateAsync(); + if (handle) { + setHandleConfirmed(true); + } + } catch { + // handled by mutation onError; polling should just continue + } +}, [confirmBlueskyHandleMutation]); - const confirmHandleInterval = setInterval(async () => { - retryCountRef.current += 1; - - if (retryCountRef.current > MAX_CONFIRMATION_RETRIES) { - clearInterval(confirmHandleInterval); - - toast.error('Something went wrong, please try again.'); - - await disableBlueskyMutation.mutateAsync(); - setLoading(false); - - return; - } - - confirmHandle(); - }, CONFIRMATION_INTERVAL); + const confirmHandleInterval = setInterval(async () => { + retryCountRef.current += 1; + + if (retryCountRef.current > MAX_CONFIRMATION_RETRIES) { + clearInterval(confirmHandleInterval); + + toast.error('Something went wrong, please try again.'); + + try { + await disableBlueskyMutation.mutateAsync(); + } catch { + // mutation onError already reported any issues + } finally { + setLoading(false); + } + + return; + } + + confirmHandle(); + }, CONFIRMATION_INTERVAL);This keeps the polling behavior the same while avoiding noisy unhandled rejections.
Also applies to: 73-80, 108-120
🧹 Nitpick comments (2)
apps/activitypub/src/api/activitypub.test.ts (1)
1668-1767: confirmBlueskyHandle tests cover main response shapes well.The success, null, missing-handle, and invalid-handle cases mirror the adapter’s
''-fallback logic nicely. If you want a tiny extra safeguard, you could also assertinit?.method === 'POST'in one of these specs, but it’s not strictly necessary given the implementation.apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
32-37: Bluesky enabled/confirmed state handling is clear; one minor state nit.The
showAsEnabledcheck (blueskyEnabled && blueskyHandleConfirmed) and the separate “enabling” vs “confirmed” UI, including the profile card, copy-handle, and open-profile actions, are all nicely wired to the account cache. The only small nit is that theloadingstate’s initializer derives fromaccountbut is effectively overridden by the effect once account data loads; you could simplify touseState(false)and rely solely on the effect, but the current behavior is functionally fine.Also applies to: 144-196
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/activitypub/package.json(1 hunks)apps/activitypub/src/api/activitypub.test.ts(10 hunks)apps/activitypub/src/api/activitypub.ts(2 hunks)apps/activitypub/src/hooks/use-activity-pub-queries.ts(4 hunks)apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx(6 hunks)
🧰 Additional context used
🧠 Learnings (11)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
📚 Learning: 2025-08-22T08:55:11.602Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/api/activitypub.tsapps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/package.json
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/package.json
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/views/Preferences/components/BlueskySharing.tsxapps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-05-29T10:37:26.369Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
apps/activitypub/src/api/activitypub.test.ts
📚 Learning: 2025-07-14T12:20:35.268Z
Learnt from: sagzy
Repo: TryGhost/Ghost PR: 24346
File: apps/admin-x-settings/src/components/settings/growth/Network.tsx:8-12
Timestamp: 2025-07-14T12:20:35.268Z
Learning: The Network component toggle in `apps/admin-x-settings/src/components/settings/growth/Network.tsx` is intentionally implemented as static UI with a no-op onChange handler, as part of a UI-first development approach before connecting actual ActivityPub functionality.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsxapps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-11-05T16:49:21.124Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/src/hooks/user-preferences.ts:20-20
Timestamp: 2025-11-05T16:49:21.124Z
Learning: In apps/admin/src/hooks/user-preferences.ts, the query key for user preferences intentionally includes `user?.accessibility` (not just `user?.id`) so the cache automatically reacts to changes from any source (useEditUserPreferences, other editUser calls, external updates). Combined with `cacheTime: 0`, this design garbage-collects orphaned entries while keeping the active entry cached. This is a documented, intentional architectural decision.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-11-10T17:07:54.169Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 25377
File: apps/admin/src/ember-bridge/EmberBridge.test.tsx:146-174
Timestamp: 2025-11-10T17:07:54.169Z
Learning: In apps/admin/src/ember-bridge/EmberBridge.tsx, the useEmberAuthSync hook invalidates all React Query caches whenever an emberAuthChange event is emitted, regardless of the isAuthenticated field value in the event payload. The isAuthenticated field exists in the event type but is not currently used by the handler.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
🧬 Code graph analysis (2)
apps/activitypub/src/api/activitypub.test.ts (1)
apps/activitypub/src/api/activitypub.ts (1)
ActivityPubAPI(230-734)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (4)
useAccountForUser(1616-1628)useEnableBlueskyMutationForUser(2746-2776)useDisableBlueskyMutationForUser(2778-2808)useConfirmBlueskyHandleMutationForUser(2810-2839)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cursor Bugbot
🔇 Additional comments (3)
apps/activitypub/package.json (1)
3-3: Version bump to 2.0.0 looks consistent with API changes.The package version aligns with the new Bluesky v2 API and confirm-handle additions; no issues from this file alone.
apps/activitypub/src/api/activitypub.ts (1)
29-30: Bluesky v2 API methods and Account fields look coherent.Account’s optional
blueskyHandleConfirmedand nullableblueskyHandlematch how the hooks/UI treat “enabled but unconfirmed” vs confirmed. The v2enableBluesky/disableBlueskyPOSTs reusingfetchJSONandconfirmBlueskyHandle’s empty-string fallback on null/invalid payloads all look correct and are well-covered by tests.Also applies to: 711-733
apps/activitypub/src/api/activitypub.test.ts (1)
1610-1655: enable/disable Bluesky v2 tests align with new API shape.The tests now assert v2 endpoints, POST method, and that both
enableBlueskyanddisableBlueskyresolve without a return value, matching the new implementation that delegates auth tofetchJSON.
ref TryGhost/ActivityPub#1377 We updated the backend implementation of the Bluesky sharing enablement process to include a confirmation step in which the client would prompt the backend to check what handle has been assigned to the Bluesky account generated by Bridgy Fed - This circumvents the need to for us to compute the handle manually and potentially getting it wrong if we compute it in a different way to Bridgy Fed
mike182uk
force-pushed
the
ap-bluesky-handle-confirmation
branch
from
b8dd7a6 to
c743df5
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (1)
2725-2839: Tighten Bluesky cache typing to matchAccountand avoid narrowing.
updateAccountBlueskyCacheoperates on theAccount('index')cache entry but types bothBlueskyDetailsandcurrentProfileas the three-field subset. That works at runtime but weakens safety and makes it easier to drift from the realAccountshape.Consider aligning the types with
Accountdirectly:-type BlueskyDetails = { - blueskyEnabled: boolean; - blueskyHandleConfirmed: boolean; - blueskyHandle: string | null; -} +type BlueskyDetails = Pick<Account, 'blueskyEnabled' | 'blueskyHandleConfirmed' | 'blueskyHandle'>; -function updateAccountBlueskyCache(queryClient: QueryClient, blueskyDetails: BlueskyDetails) { +function updateAccountBlueskyCache( + queryClient: QueryClient, + blueskyDetails: BlueskyDetails +) { const profileQueryKey = QUERY_KEYS.account('index'); - queryClient.setQueryData(profileQueryKey, (currentProfile?: BlueskyDetails) => { + queryClient.setQueryData(profileQueryKey, (currentProfile?: Account) => { if (!currentProfile) { return currentProfile; } return { ...currentProfile, ...blueskyDetails }; }); }This keeps the helper focused on Bluesky-specific fields while preserving the full
Accounttype in the cache.apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
73-80: HandlemutateAsyncrejections in polling to avoid unhandled promises and stuck loading.Two places in the polling flow can produce unhandled rejections and inconsistent UI state:
confirmHandlecallsconfirmBlueskyHandleMutation.mutateAsync().then(...)with no.catch, so any API error during polling bubbles as an unhandled rejection (especially problematic insidesetInterval).- In the retry timeout path,
await disableBlueskyMutation.mutateAsync();is not wrapped, so if disable fails,setLoading(false)never runs and the user can remain stuck in a loading state even though polling has stopped.You can make both paths safer by converting to
async+try/catch/finallyand letting the hook-levelonErrorown user-facing error messaging:- const confirmHandle = useCallback(() => { - confirmBlueskyHandleMutation.mutateAsync().then((handle) => { - if (handle) { - setHandleConfirmed(true); - } - }); - // eslint-disable-next-line react-hooks/exhaustive-deps - }, []); // Empty deps - mutations are stable in practice + const confirmHandle = useCallback(async () => { + try { + const handle = await confirmBlueskyHandleMutation.mutateAsync(); + if (handle) { + setHandleConfirmed(true); + } + } catch { + // onError in the mutation already surfaces rate-limit errors; swallow here + } + }, [confirmBlueskyHandleMutation]); @@ - const confirmHandleInterval = setInterval(async () => { - retryCountRef.current += 1; - - if (retryCountRef.current > MAX_CONFIRMATION_RETRIES) { - clearInterval(confirmHandleInterval); - - toast.error('Something went wrong, please try again.'); - - await disableBlueskyMutation.mutateAsync(); - setLoading(false); - - return; - } - - confirmHandle(); - }, CONFIRMATION_INTERVAL); + const confirmHandleInterval = setInterval(async () => { + retryCountRef.current += 1; + + if (retryCountRef.current > MAX_CONFIRMATION_RETRIES) { + clearInterval(confirmHandleInterval); + + toast.error('Something went wrong, please try again.'); + + try { + await disableBlueskyMutation.mutateAsync(); + } catch { + // disable hook handles its own error toasts + } finally { + setLoading(false); + } + + return; + } + + await confirmHandle(); + }, CONFIRMATION_INTERVAL);This keeps the polling logic robust without changing the external behavior or toast contract.
Also applies to: 108-127
🧹 Nitpick comments (1)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
48-58: Avoid double toasts by delegating enable failures to the mutation hook.
handleEnablecatches all errors fromenableBlueskyMutation.mutateAsync()and shows a generic “Something went wrong” toast, but the hook’sonErroralready renders a rate‑limit toast for 429 responses. For 429s this results in two error toasts for a single failure.You could either remove the component‑level toast or only show it for non‑429 errors:
- try { - await enableBlueskyMutation.mutateAsync(); - } catch (error) { - setLoading(false); - toast.error('Something went wrong, please try again.'); - } + try { + await enableBlueskyMutation.mutateAsync(); + } catch (error: any) { + setLoading(false); + if (error?.statusCode !== 429) { + toast.error('Something went wrong, please try again.'); + } + }Keeps the “centralized error handling in hooks” pattern while still giving a fallback message for unexpected failures.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/activitypub/package.json(1 hunks)apps/activitypub/src/api/activitypub.test.ts(10 hunks)apps/activitypub/src/api/activitypub.ts(2 hunks)apps/activitypub/src/hooks/use-activity-pub-queries.ts(4 hunks)apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx(6 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/activitypub/package.json
🧰 Additional context used
🧠 Learnings (9)
📓 Common learnings
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
📚 Learning: 2025-08-22T08:55:11.602Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 24733
File: apps/admin-x-activitypub/src/api/activitypub.test.ts:1601-1627
Timestamp: 2025-08-22T08:55:11.602Z
Learning: In the Ghost ActivityPub module, `disableBluesky()` uses direct fetch where auth headers are manually passed, while `enableBluesky()` uses `fetchJSON` which abstracts auth header handling away. This is why the disable test needs to assert Authorization headers but the enable test doesn't.
Applied to files:
apps/activitypub/src/api/activitypub.tsapps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in the Ghost ActivityPub module are covered by tests in the file `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`.
Applied to files:
apps/activitypub/src/api/activitypub.test.ts
📚 Learning: 2025-03-13T09:00:20.205Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/utils/pending-activity.ts:13-71
Timestamp: 2025-03-13T09:00:20.205Z
Learning: The pending activity utilities in Ghost's ActivityPub module are thoroughly tested in `apps/admin-x-activitypub/test/unit/utils/pending-activity.ts`, which covers `generatePendingActivityId`, `isPendingActivity`, and `generatePendingActivity` functions.
Applied to files:
apps/activitypub/src/api/activitypub.test.ts
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the Ghost ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/api/activitypub.test.tsapps/activitypub/src/hooks/use-activity-pub-queries.tsapps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-05-29T10:37:26.369Z
Learnt from: ErisDS
Repo: TryGhost/Ghost PR: 23588
File: ghost/core/test/e2e-api/admin/backup.test.js:136-148
Timestamp: 2025-05-29T10:37:26.369Z
Learning: In the Ghost test framework, when testing CSV exports via the admin API, the response `body` field is empty while the actual CSV data is provided through the `text` field. Tests should use `expectEmptyBody()` and then validate the CSV content via `.expect(({text}) => ...)` - this is not contradictory behavior.
Applied to files:
apps/activitypub/src/api/activitypub.test.ts
📚 Learning: 2025-03-13T09:02:50.102Z
Learnt from: mike182uk
Repo: TryGhost/Ghost PR: 22471
File: apps/admin-x-activitypub/src/views/Feed/components/NewPostModal.tsx:29-34
Timestamp: 2025-03-13T09:02:50.102Z
Learning: In the ActivityPub module, error handling for mutations is handled at the hook level (in use-activity-pub-queries.ts) rather than in individual components. This allows for centralized error handling across the application.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.tsapps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
📚 Learning: 2025-11-05T16:49:21.124Z
Learnt from: rob-ghost
Repo: TryGhost/Ghost PR: 25356
File: apps/admin/src/hooks/user-preferences.ts:20-20
Timestamp: 2025-11-05T16:49:21.124Z
Learning: In apps/admin/src/hooks/user-preferences.ts, the query key for user preferences intentionally includes `user?.accessibility` (not just `user?.id`) so the cache automatically reacts to changes from any source (useEditUserPreferences, other editUser calls, external updates). Combined with `cacheTime: 0`, this design garbage-collects orphaned entries while keeping the active entry cached. This is a documented, intentional architectural decision.
Applied to files:
apps/activitypub/src/hooks/use-activity-pub-queries.ts
📚 Learning: 2025-08-11T19:39:00.428Z
Learnt from: kevinansfield
Repo: TryGhost/Ghost PR: 24651
File: ghost/core/test/utils/urlUtils.js:53-57
Timestamp: 2025-08-11T19:39:00.428Z
Learning: In Ghost's test utilities, when fixing specific issues like async behavior, it's preferred to maintain existing error handling patterns (even if suboptimal) to keep PRs focused on their primary objective. Error handling improvements can be addressed in separate, dedicated PRs.
Applied to files:
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx
🧬 Code graph analysis (2)
apps/activitypub/src/api/activitypub.test.ts (1)
apps/activitypub/src/api/activitypub.ts (1)
ActivityPubAPI(230-734)
apps/activitypub/src/views/Preferences/components/BlueskySharing.tsx (1)
apps/activitypub/src/hooks/use-activity-pub-queries.ts (4)
useAccountForUser(1616-1628)useEnableBlueskyMutationForUser(2746-2776)useDisableBlueskyMutationForUser(2778-2808)useConfirmBlueskyHandleMutationForUser(2810-2839)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Cursor Bugbot
- GitHub Check: Setup
- GitHub Check: Build & Push
- GitHub Check: Setup
🔇 Additional comments (2)
apps/activitypub/src/api/activitypub.ts (1)
29-30: Bluesky account fields and v2 actions look consistent and defensive.The Account bluesky fields and the v2
enableBluesky/disableBluesky/confirmBlueskyHandleimplementations align with the described API: endpoints are correct, enable/disable are POST + void, andconfirmBlueskyHandlesafely normalizes unexpected/empty responses to''so callers don’t need to null-check.Also applies to: 711-733
apps/activitypub/src/api/activitypub.test.ts (1)
1600-1627: Tests correctly cover v2 Bluesky flows including confirm-handle edge cases.The updated tests target the v2 endpoints, assert POST usage for enable/disable, and exercise
confirmBlueskyHandleacross valid, null, and malformed responses, matching the new API semantics and guarding against unexpected payloads.Also applies to: 1629-1656, 1658-1768
ref TryGhost/ActivityPub#1377 We updated the backend implementation of the Bluesky sharing enablement process to include a confirmation step in which the client would prompt the backend to check what handle has been assigned to the Bluesky account generated by Bridgy Fed - This circumvents the need to for us to compute the handle manually and potentially getting it wrong if we compute it in a different way to Bridgy Fed --------- Co-authored-by: Sodbileg Gansukh <sodbileg.gansukh@gmail.com>
2 participants
ref TryGhost/ActivityPub#1377
We updated the backend implementation of the Bluesky sharing enablement process to include a confirmation step in which the client would prompt the backend to check what handle has been assigned to the Bluesky account generated by Bridgy Fed - This circumvents the need to for us to compute the handle manually and potentially getting it wrong if we compute it in a different way to Bridgy Fed
Note
Switch Bluesky sharing to v2 enable/disable endpoints, add handle confirmation flow with polling, update UI/state, and bump package to 2.0.0.
v2(enable,disable) and addconfirm-handleendpoint.enableBluesky()to no longer return a handle; introduceconfirmBlueskyHandle()returning a confirmed handle or empty string.AccountwithblueskyHandleConfirmedand makeblueskyHandlenullable.apps/activitypub/src/hooks/use-activity-pub-queries.ts):useConfirmBlueskyHandleMutationForUserand cache updater for Bluesky details.{blueskyEnabled, blueskyHandleConfirmed, blueskyHandle}appropriately and invalidate follows.views/Preferences/components/BlueskySharing.tsx):src/api/activitypub.test.ts):v2endpoints; add tests for handle confirmation and null/invalid responses.@tryghost/activitypubversion to2.0.0.Written by Cursor Bugbot for commit c743df5. This will update automatically on new commits. Configure here.