Account Details: Redirect form incognito profile to mainProfile - #12153
Merged
Merged
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThis PR adds a shared ActivityAccountFields GraphQL fragment (including mainProfile) and updates activity-related fragments to include mainProfile. UI components now normalize activity nodes to prefer mainProfile for individual/fromAccount/account/host before rendering. The People router queries an account's mainProfile.publicId and replaces the current subpath with that publicId when different. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 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 |
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
PeopleRouter, consider adding a guard to avoid callingreplaceSubpathwhen the currentidalready matches themainProfile.publicId, to prevent unnecessary router replacements on already-normalized URLs. - The repeated
mainProfileselection inContributionDrawer(forfromAccount,account,host,individual) could be extracted into a GraphQL fragment to keep the query DRY and easier to maintain when these fields change.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `PeopleRouter`, consider adding a guard to avoid calling `replaceSubpath` when the current `id` already matches the `mainProfile.publicId`, to prevent unnecessary router replacements on already-normalized URLs.
- The repeated `mainProfile` selection in `ContributionDrawer` (for `fromAccount`, `account`, `host`, `individual`) could be extracted into a GraphQL fragment to keep the query DRY and easier to maintain when these fields change.
## Individual Comments
### Comment 1
<location path="components/dashboard/sections/community/People.tsx" line_range="427-432" />
<code_context>
+ skip: isEmpty(id),
+ });
+
+ useEffect(() => {
+ const mainProfilePublicId = accountData?.account?.mainProfile?.publicId;
+ if (mainProfilePublicId) {
+ replaceSubpath(mainProfilePublicId);
+ }
+ }, [accountData, replaceSubpath]);
+
if (!isEmpty(id)) {
</code_context>
<issue_to_address>
**issue (bug_risk):** Add a guard and tighten dependencies to avoid redundant or looping route replacements.
The current effect reruns on every `accountData` change and always calls `replaceSubpath` when `mainProfilePublicId` is truthy, which can cause redundant replaces or even a redirect loop if the `id` is already equal. It also depends on the whole `accountData` object, so identity changes can trigger extra runs even when `publicId` hasn’t changed.
Instead, derive `mainProfilePublicId` once and depend only on the relevant primitives:
```ts
const mainProfilePublicId = accountData?.account?.mainProfile?.publicId;
useEffect(() => {
if (!mainProfilePublicId || mainProfilePublicId === id) {
return;
}
replaceSubpath(mainProfilePublicId);
}, [id, mainProfilePublicId, replaceSubpath]);
```
This avoids unnecessary navigation and limits the effect to meaningful changes.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
Comment on lines
+427
to
+432
| useEffect(() => { | ||
| const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; | ||
| if (mainProfilePublicId) { | ||
| replaceSubpath(mainProfilePublicId); | ||
| } | ||
| }, [accountData, replaceSubpath]); |
There was a problem hiding this comment.
issue (bug_risk): Add a guard and tighten dependencies to avoid redundant or looping route replacements.
The current effect reruns on every accountData change and always calls replaceSubpath when mainProfilePublicId is truthy, which can cause redundant replaces or even a redirect loop if the id is already equal. It also depends on the whole accountData object, so identity changes can trigger extra runs even when publicId hasn’t changed.
Instead, derive mainProfilePublicId once and depend only on the relevant primitives:
const mainProfilePublicId = accountData?.account?.mainProfile?.publicId;
useEffect(() => {
if (!mainProfilePublicId || mainProfilePublicId === id) {
return;
}
replaceSubpath(mainProfilePublicId);
}, [id, mainProfilePublicId, replaceSubpath]);This avoids unnecessary navigation and limits the effect to meaningful changes.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@components/dashboard/sections/community/People.tsx`:
- Around line 422-432: The code currently allows AccountDetails to render before
the account lookup finishes; update the effect and render gating so you only
redirect when the fetched mainProfilePublicId exists and differs from id, and
you prevent rendering AccountDetails while the query is still resolving.
Specifically, use the useQuery loading or equivalent state from
peopleRouterAccountQuery (keep variable names accountData and loading) and
change the useEffect to: if (mainProfilePublicId && mainProfilePublicId !== id)
replaceSubpath(mainProfilePublicId); and in the component render path (where
AccountDetails is rendered) return a loading/null state while loading is true
(or while accountData is undefined) to avoid showing incognito details; apply
the same guard for the other block referenced (lines 434-445) that renders
AccountDetails.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e08086b8-afee-49a7-ae22-9ce9bb98f88d
⛔ Files ignored due to path filters (2)
lib/graphql/types/v2/gql.tsis excluded by!lib/graphql/types/**lib/graphql/types/v2/graphql.tsis excluded by!lib/graphql/types/**
📒 Files selected for processing (3)
components/contributions/ContributionDrawer.tsxcomponents/contributions/ContributionTimeline.tsxcomponents/dashboard/sections/community/People.tsx
Comment on lines
+422
to
+432
| const { data: accountData } = useQuery(peopleRouterAccountQuery, { | ||
| variables: { id }, | ||
| skip: isEmpty(id), | ||
| }); | ||
|
|
||
| useEffect(() => { | ||
| const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; | ||
| if (mainProfilePublicId) { | ||
| replaceSubpath(mainProfilePublicId); | ||
| } | ||
| }, [accountData, replaceSubpath]); |
There was a problem hiding this comment.
Prevent incognito details from rendering before redirect completes.
AccountDetails still renders immediately when id exists (Line 434), so an incognito profile can render briefly before the useEffect redirect runs. Gate rendering while the lookup is resolving, and only redirect when the resolved mainProfilePublicId differs from id.
Suggested patch
- const { data: accountData } = useQuery(peopleRouterAccountQuery, {
+ const { data: accountData, loading: accountLoading } = useQuery(peopleRouterAccountQuery, {
variables: { id },
skip: isEmpty(id),
});
+ const mainProfilePublicId = accountData?.account?.mainProfile?.publicId;
+
useEffect(() => {
- const mainProfilePublicId = accountData?.account?.mainProfile?.publicId;
- if (mainProfilePublicId) {
+ if (mainProfilePublicId && mainProfilePublicId !== id) {
replaceSubpath(mainProfilePublicId);
}
- }, [accountData, replaceSubpath]);
+ }, [id, mainProfilePublicId, replaceSubpath]);
if (!isEmpty(id)) {
+ if (accountLoading || (mainProfilePublicId && mainProfilePublicId !== id)) {
+ return null;
+ }
return (
<div className="h-full">
<AccountDetailsAlso applies to: 434-445
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/dashboard/sections/community/People.tsx` around lines 422 - 432,
The code currently allows AccountDetails to render before the account lookup
finishes; update the effect and render gating so you only redirect when the
fetched mainProfilePublicId exists and differs from id, and you prevent
rendering AccountDetails while the query is still resolving. Specifically, use
the useQuery loading or equivalent state from peopleRouterAccountQuery (keep
variable names accountData and loading) and change the useEffect to: if
(mainProfilePublicId && mainProfilePublicId !== id)
replaceSubpath(mainProfilePublicId); and in the component render path (where
AccountDetails is rendered) return a loading/null state while loading is true
(or while accountData is undefined) to avoid showing incognito details; apply
the same guard for the other block referenced (lines 434-445) that renders
AccountDetails.
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
Comment on lines
+427
to
+432
| useEffect(() => { | ||
| const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; | ||
| if (mainProfilePublicId) { | ||
| replaceSubpath(mainProfilePublicId); | ||
| } | ||
| }, [accountData, replaceSubpath]); |
There was a problem hiding this comment.
Bug: The replaceSubpath function is recreated on every render and used in a useEffect dependency array, causing an infinite re-render loop and unnecessary router.replace() calls.
Severity: MEDIUM
Suggested Fix
Wrap the creation of the replaceSubpath function with React.useMemo to stabilize its reference across renders. This will prevent the useEffect hook from re-running unnecessarily. The change should be: const replaceSubpath = React.useMemo(() => makeReplaceSubpath(router), [router]);.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: components/dashboard/sections/community/People.tsx#L427-L432
Potential issue: In the `PeopleRouter` component, the `replaceSubpath` function is
created on every render because it is not memoized with `React.useMemo`. This unstable
function is then included in the dependency array of a `useEffect` hook. As a result,
the effect re-executes on every render, not just when `accountData` changes. This
triggers a `router.replace()` call, which causes a re-render, creating a new
`replaceSubpath` function and thus an infinite loop. This leads to unnecessary
re-renders and performance degradation. This pattern is inconsistent with other parts of
the codebase where similar functions are correctly memoized.
Also affects:
components/dashboard/sections/exports/index.tsxcomponents/dashboard/sections/Vendors.tsx
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/dashboard/sections/community/People.tsx (1)
422-445:⚠️ Potential issue | 🟠 Major | ⚡ Quick winGate details rendering until redirect resolution completes.
Line 434 renders
AccountDetailsbefore themainProfilelookup finishes, so an incognito profile can flash before redirect. Block render while loading (and while a redirect target differs fromid).Suggested patch
- const { data: accountData } = useQuery(peopleRouterAccountQuery, { + const { data: accountData, loading: accountLoading } = useQuery(peopleRouterAccountQuery, { variables: { id }, skip: isEmpty(id), }); + const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; useEffect(() => { - const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; if (mainProfilePublicId && mainProfilePublicId !== id) { replaceSubpath(mainProfilePublicId); } - }, [accountData, replaceSubpath, id]); + }, [id, mainProfilePublicId, replaceSubpath]); if (!isEmpty(id)) { + if (accountLoading || (mainProfilePublicId && mainProfilePublicId !== id)) { + return null; + } return ( <div className="h-full"> <AccountDetails🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/dashboard/sections/community/People.tsx` around lines 422 - 445, The AccountDetails can render before the mainProfile lookup/redirect completes; update the useQuery call to read the loading state (const { data: accountData, loading } = useQuery(...)) and prevent rendering AccountDetails while loading or while a redirect target differs from id (i.e. when accountData?.account?.mainProfile?.publicId exists and !== id). Concretely, add a guard before the AccountDetails return that returns null or a loader if loading || (mainProfilePublicId && mainProfilePublicId !== id) so render is blocked until lookup/redirect resolution finishes; reference useQuery/peopleRouterAccountQuery, accountData, loading, mainProfilePublicId, replaceSubpath, isEmpty, AccountDetails, subpath, and id.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@components/dashboard/sections/community/People.tsx`:
- Around line 422-445: The AccountDetails can render before the mainProfile
lookup/redirect completes; update the useQuery call to read the loading state
(const { data: accountData, loading } = useQuery(...)) and prevent rendering
AccountDetails while loading or while a redirect target differs from id (i.e.
when accountData?.account?.mainProfile?.publicId exists and !== id). Concretely,
add a guard before the AccountDetails return that returns null or a loader if
loading || (mainProfilePublicId && mainProfilePublicId !== id) so render is
blocked until lookup/redirect resolution finishes; reference
useQuery/peopleRouterAccountQuery, accountData, loading, mainProfilePublicId,
replaceSubpath, isEmpty, AccountDetails, subpath, and id.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 84360b18-8ea7-4f70-a36c-502aa4e51fb5
📒 Files selected for processing (2)
components/contributions/ContributionDrawer.tsxcomponents/dashboard/sections/community/People.tsx
kewitz
force-pushed
the
refact/incognito-people-redirect
branch
from
May 20, 2026 14:06
a542da1 to
e1c0ccc
Compare
kewitz
force-pushed
the
refact/incognito-people-redirect
branch
from
May 26, 2026 15:21
e1c0ccc to
001ea1f
Compare
Comment on lines
434
to
436
| if (!isEmpty(id)) { | ||
| return ( | ||
| <div className="h-full"> |
There was a problem hiding this comment.
Bug: A race condition on incognito profile pages initiates a query that may briefly show an error message before redirecting to the main profile.
Severity: LOW
Suggested Fix
Prevent the AccountDetails component from rendering or executing its query until the check for a main profile is complete. This can be done by showing a loading state within PeopleRouter and only rendering AccountDetails after confirming that the ID is not for an incognito profile that needs redirection.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: components/dashboard/sections/community/People.tsx#L434-L442
Potential issue: When a user navigates to an incognito profile URL, the `PeopleRouter`
component immediately renders the `AccountDetails` component using the incognito
profile's ID. This triggers a `communityAccountDetailQuery` with that ID. In parallel, a
`useEffect` hook asynchronously fetches the corresponding main profile ID and triggers a
redirect. This creates a race condition where the query for the incognito profile is
executed. If this API call returns an error, as is likely intended, an error message
will briefly flash on the screen before the user is redirected to the main profile,
creating a jarring user experience.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
components/dashboard/sections/community/People.tsx (1)
422-445:⚠️ Potential issue | 🟠 Major | ⚡ Quick winBlock
AccountDetailsrendering until redirect resolution completes.
AccountDetailsstill renders as soon asidexists, so an incognito profile can briefly render beforereplaceSubpathruns. Gate rendering while lookup is in progress and while a redirect target differs fromid.Suggested patch
- const { data: accountData } = useQuery(peopleRouterAccountQuery, { + const { data: accountData, loading } = useQuery(peopleRouterAccountQuery, { variables: { id }, skip: isEmpty(id), }); + const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; + useEffect(() => { - const mainProfilePublicId = accountData?.account?.mainProfile?.publicId; if (mainProfilePublicId && mainProfilePublicId !== id) { replaceSubpath(mainProfilePublicId); } - }, [accountData, replaceSubpath, id]); + }, [id, mainProfilePublicId, replaceSubpath]); if (!isEmpty(id)) { + if (loading || (mainProfilePublicId && mainProfilePublicId !== id)) { + return null; + } return ( <div className="h-full"> <AccountDetails🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@components/dashboard/sections/community/People.tsx` around lines 422 - 445, The AccountDetails component is rendering prematurely before the redirect resolution completes; update the useQuery call (peopleRouterAccountQuery) to also pull the loading flag, compute mainProfilePublicId from accountData, and prevent rendering when the query is still loading or when a redirect target differs from id (i.e., if loading || (mainProfilePublicId && mainProfilePublicId !== id) return null or a loader). Ensure you still call replaceSubpath(mainProfilePublicId) inside the existing useEffect when a differing mainProfilePublicId is found so the redirect happens before AccountDetails mounts.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@components/dashboard/sections/community/People.tsx`:
- Around line 422-445: The AccountDetails component is rendering prematurely
before the redirect resolution completes; update the useQuery call
(peopleRouterAccountQuery) to also pull the loading flag, compute
mainProfilePublicId from accountData, and prevent rendering when the query is
still loading or when a redirect target differs from id (i.e., if loading ||
(mainProfilePublicId && mainProfilePublicId !== id) return null or a loader).
Ensure you still call replaceSubpath(mainProfilePublicId) inside the existing
useEffect when a differing mainProfilePublicId is found so the redirect happens
before AccountDetails mounts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 71b0b8d7-4eed-46e5-b4b2-7cba36bfc339
⛔ Files ignored due to path filters (2)
lib/graphql/types/v2/gql.tsis excluded by!lib/graphql/types/**lib/graphql/types/v2/graphql.tsis excluded by!lib/graphql/types/**
📒 Files selected for processing (5)
components/contributions/ContributionDrawer.tsxcomponents/contributions/ContributionTimeline.tsxcomponents/dashboard/sections/community/AccountDetailActivitiesTab.tsxcomponents/dashboard/sections/community/People.tsxcomponents/dashboard/sections/community/queries.ts
hdiniz
approved these changes
May 27, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Resolves opencollective/opencollective#8761
Requires opencollective/opencollective-api#11755