feat: wire follow/unfollow to SocialController using AuthenticationController profileId#28843
Conversation
…ntroller profileId
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Prevents spurious re-renders when SocialController state is nullish by using a frozen constant instead of allocating a new array each call. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Prevents double-tap race conditions by disabling the follow/unfollow button during the controller call. Applies to TraderRow, TopTraderCard, and TraderProfileView. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Use Set<string> instead of single string for followLoadingIds to correctly handle multiple concurrent follow/unfollow operations. Fix Object.freeze type cast and add missing mock properties in tests. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…ow-cta Made-with: Cursor # Conflicts: # app/core/Engine/messengers/social-service-messenger.ts
| @@ -1,10 +1,13 @@ | |||
| import { useCallback, useState, useEffect } from 'react'; | |||
| import { useCallback, useEffect, useRef, useState } from 'react'; | |||
There was a problem hiding this comment.
Significant Code Duplication — useTopTraders vs useTraderProfile
Both hooks add this same logical sequence. I'll put them side by side using the diff content.
1. State setup — identical concept, only data structure differs
useTraderProfile manages a single trader:
const [optimisticFollow, setOptimisticFollow] = useState<boolean | null>(null);
const inflightRef = useRef(false);
const isFollowing = optimisticFollow ?? reduxFollowing;useTopTraders manages N traders, so it uses a map — but the concept is the same:
const [optimisticFollowState, setOptimisticFollowState] = useState<Record<string, boolean>>({});
const inflightIdsRef = useRef<Set<string>>(new Set());
// inside useMemo per-trader:
isFollowing: optimisticFollowState[entry.profileId] ?? currentFollowingThe only real difference is "one boolean" vs "a Record<id, boolean>". The logic for why optimistic state exists and how it interacts with Redux is identical.
2. toggleFollow body — nearly verbatim
useTraderProfile:
const toggleFollow = useCallback(async () => {
if (inflightRef.current) return; // ← guard
inflightRef.current = true;
const nextValue = !isFollowing;
setOptimisticFollow(nextValue); // ← optimistic set
try {
const { profileId } =
await Engine.context.AuthenticationController.getSessionProfile();
const opts = { addressOrUid: profileId, targets: [addressOrId] };
if (nextValue) {
await messenger.call('SocialController:followTrader', opts);
} else {
await messenger.call('SocialController:unfollowTrader', opts);
}
} catch (err) {
setOptimisticFollow(null); // ← rollback
Logger.error(err, '...');
} finally {
inflightRef.current = false; // ← clear guard
}
}, [isFollowing, addressOrId]);useTopTraders:
const toggleFollow = useCallback(async (addressOrId: string) => {
if (inflightIdsRef.current.has(addressOrId)) return; // ← guard
inflightIdsRef.current.add(addressOrId);
const nextValue = !currentlyFollowing;
setOptimisticFollowState(prev => ({ ...prev, [addressOrId]: nextValue })); // ← optimistic set
try {
const { profileId } =
await Engine.context.AuthenticationController.getSessionProfile();
const opts = { addressOrUid: profileId, targets: [addressOrId] };
if (nextValue) {
await messenger.call('SocialController:followTrader', opts);
} else {
await messenger.call('SocialController:unfollowTrader', opts);
}
} catch (err) {
setOptimisticFollowState(prev => { delete prev[addressOrId]; return prev; }); // ← rollback
Logger.error(err, '...');
} finally {
inflightIdsRef.current.delete(addressOrId); // ← clear guard
}
}, [...]);The four steps — guard → optimistic set → getSessionProfile + messenger call → rollback on error — are word-for-word the same. Only the state mutation shape differs.
3. Cleanup useEffect — same intent, different shape
useTraderProfile — single value, simple:
useEffect(() => {
if (optimisticFollow !== null && reduxFollowing === optimisticFollow) {
setOptimisticFollow(null); // clear once Redux agrees
}
}, [optimisticFollow, reduxFollowing]);useTopTraders — iterates over all entries, but same idea:
useEffect(() => {
setOptimisticFollowState(prev => {
for (const [id, value] of Object.entries(prev)) {
if (followingProfileIds.includes(id) === value) {
// remove it — Redux has caught up
}
}
});
}, [followingProfileIds]);Both effects exist to answer the same question: "Has Redux caught up to my optimistic value? If yes, discard it."
The Risk
Because this logic is duplicated:
- A bug in the guard logic (e.g., the inflight ref not being cleared in an edge case) has to be fixed in two places. Right now there's actually a subtle asymmetry: on error,
useTraderProfilesetsoptimisticFollowtonull(clearing it), whileuseTopTradersdeletes the key from the map. These are equivalent today but they'll diverge under future edits. - If a third surface (e.g., a Watchlist or a Notifications screen) needs follow/unfollow, this pattern gets copy-pasted a third time.
What the Shared Hook Would Look Like
The single-trader hook that both could call:
// app/components/hooks/useFollowToggle.ts
export function useFollowToggle(addressOrId: string): {
isFollowing: boolean;
toggleFollow: () => Promise<void>;
} {
const followingProfileIds = useSelector(selectFollowingProfileIds);
const reduxFollowing = followingProfileIds.includes(addressOrId);
const [optimistic, setOptimistic] = useState<boolean | null>(null);
const inflightRef = useRef(false);
const isFollowing = optimistic ?? reduxFollowing;
const toggleFollow = useCallback(async () => {
if (inflightRef.current) return;
inflightRef.current = true;
const next = !isFollowing;
setOptimistic(next);
try {
const { profileId } =
await Engine.context.AuthenticationController.getSessionProfile();
const opts = { addressOrUid: profileId, targets: [addressOrId] };
await (Engine.controllerMessenger.call as CallableFunction)(
next ? 'SocialController:followTrader' : 'SocialController:unfollowTrader',
opts,
);
} catch (err) {
setOptimistic(null);
Logger.error(err as Error, 'useFollowToggle failed');
} finally {
inflightRef.current = false;
}
}, [isFollowing, addressOrId]);
useEffect(() => {
if (optimistic !== null && reduxFollowing === optimistic) {
setOptimistic(null);
}
}, [optimistic, reduxFollowing]);
return { isFollowing, toggleFollow };
}useTraderProfile then becomes a one-liner call:
const { isFollowing, toggleFollow } = useFollowToggle(addressOrId);useTopTraders calls it once per trader in useMemo, or calls it at render and passes the result into the list — the multi-trader case needs a small adapter, but the core logic lives in one place.
The duplication is real and worth surfacing in review, even if the author chooses to defer the refactor to a follow-up.
… hooks Removed Redux dependencies and optimized follow state management by integrating useFollowToggle and useFollowToggleMany hooks. This change enhances code clarity and performance by eliminating unnecessary state management and improving the optimistic follow behavior for traders and profiles.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit be833fe. Configure here.
Changed the import statement in the `useTopTraders.test.ts` file from `@testing-library/react-hooks` to `@testing-library/react-native` to align with the testing framework used in the project.
🔍 Smart E2E Test Selection
click to see 🤖 AI reasoning detailsE2E Test Selection:
Tag Selection Rationale:
Tags NOT selected:
Performance Test Selection: |
|
|
✅ E2E Fixture Validation — Schema is up to date |
Description
Wire follow/unfollow persistence via SocialController state
Test plan
TSA-389
Changelog
CHANGELOG entry: Wired up follow/unfollow functionality
Related issues
Fixes:
Manual testing steps
Screenshots/Recordings
Before
After
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Medium Risk
Introduces new follow/unfollow wiring that calls
SocialControllerviaEngine.controllerMessengerand runs a new hydration request duringEngineService.start(), which could affect app startup behavior and follow state consistency if messaging/auth fails.Overview
Moves follow state from local component overrides to persisted
SocialController.followingProfileIdsin Redux, via a new shared hookuseFollowToggle/useFollowToggleManythat performs optimistic follow/unfollow and dispatchesSocialController:followTrader/SocialController:unfollowTraderusing the sessionprofileIdfromAuthenticationController.Adds engine-startup hydration (
hydrateSocialFollowing) to refresh the server following list by callingSocialController:updateFollowing(fire-and-forget, non-fatal on failure), plus a new selectorselectFollowingProfileIdsand expanded unit tests to cover seeding from controller state, optimistic updates, rejection reverts, and in-flight call de-duping.Reviewed by Cursor Bugbot for commit 4637abc. Bugbot is set up for automated code reviews on this repo. Configure here.