refactor: user authuser 2#8741
Conversation
…er and AnonymousUser, enhancing user management capabilities
WalkthroughReplaces the public/federated Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
|
View your CI Pipeline Execution ↗ for commit 29fa606
☁️ Nx Cloud last updated this comment at |
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
|
The latest updates on your projects.
|
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apis/api-users/src/schema/user/user.ts (1)
39-44:⚠️ Potential issue | 🟡 MinorLog message references old type name "User" instead of "AuthenticatedUser".
Line 41 still reads
"Federation: Error resolving User entity for userId: …". Since this entity is nowAuthenticatedUser, the log should match for grep-ability and consistency.Proposed fix
- `Federation: Error resolving User entity for userId: ${id}`, + `Federation: Error resolving AuthenticatedUser entity for userId: ${id}`,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-users/src/schema/user/user.ts` around lines 39 - 44, Log message in the catch block for resolving the federated user entity still uses the old type name "User"; update the string used in the console.error within the resolveReference (or equivalent) function so it references "AuthenticatedUser" (e.g., change `"Federation: Error resolving User entity for userId: ${id}"` to use "AuthenticatedUser") and scan for any other console.error/log occurrences in the same file (user.ts) that still mention "User" to make them consistent for grep-ability.
🧹 Nitpick comments (1)
apis/api-journeys-modern/src/schema/user/user.ts (1)
9-15: Remove the redundant emptyfieldsblock inAuthenticatedUserRef.implement().The empty
fields: (t) => ({})is unnecessary —implement()for an external ref only requiresexternalFields. This also makes the comment inside vacuous.♻️ Proposed fix
AuthenticatedUserRef.implement({ - externalFields: (t) => ({ id: t.id({ nullable: false }) }), - fields: (t) => ({ - // No additional fields needed - this is just the external reference - }) + externalFields: (t) => ({ id: t.id({ nullable: false }) }) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apis/api-journeys-modern/src/schema/user/user.ts` around lines 9 - 15, Remove the redundant empty fields block inside AuthenticatedUserRef.implement(): keep only the externalFields definition (externalFields: (t) => ({ id: t.id({ nullable: false }) })) and delete the fields: (t) => ({ /* ... */ }) entry and its comment so the implementation is just the externalFields for the external reference.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apis/api-gateway/schema.graphql`:
- Around line 2189-2191: AnonymousUser is defined only as an extension in
API_JOURNEYS and other subgraphs lack a primary `@key` owner, so pick one subgraph
to own the entity: remove extension: true from the chosen subgraph’s
AnonymousUser declaration and add a non-extension `@key`(fields: "id") to that
type (e.g., in API_USERS or API_JOURNEYS_MODERN), or alternatively ensure
AnonymousUser is never referenced across subgraphs; update the type declarations
for AnonymousUser and the `@join__type` annotations accordingly so one subgraph is
the primary owner with a `@key` on id while others remain extensions or shareable.
In `@apis/api-journeys/src/app/modules/user/user.graphql`:
- Around line 5-7: The AnonymousUser type extension incorrectly adds
`@key`(fields: "id") without marking the id field `@external` or providing a
__resolveReference resolver; remove the `@key` directive from the AnonymousUser
extension in this module unless you intend to make it a federated entity, in
which case mark the id field with `@external` and implement a __resolveReference
resolver for AnonymousUser that delegates to the owning service (matching the
entity resolution pattern used for AuthenticatedUser) so federation composition
stays consistent.
---
Outside diff comments:
In `@apis/api-users/src/schema/user/user.ts`:
- Around line 39-44: Log message in the catch block for resolving the federated
user entity still uses the old type name "User"; update the string used in the
console.error within the resolveReference (or equivalent) function so it
references "AuthenticatedUser" (e.g., change `"Federation: Error resolving User
entity for userId: ${id}"` to use "AuthenticatedUser") and scan for any other
console.error/log occurrences in the same file (user.ts) that still mention
"User" to make them consistent for grep-ability.
---
Duplicate comments:
In `@apis/api-journeys/schema.graphql`:
- Around line 2701-2711: The AnonymousUser extension erroneously declares `@key`
without marking its id as `@external`, which causes this subgraph to declare
AnonymousUser as a federation entity; update the AnonymousUser block to match
AuthenticatedUser by marking id as `@external` (i.e., change "id: ID!" to "id: ID!
`@external`") or remove the `@key` directive entirely if this subgraph should not
own the key; target the AnonymousUser type, its `@key`(fields: \"id\") directive,
and the id field in the schema definition to make the fix.
---
Nitpick comments:
In `@apis/api-journeys-modern/src/schema/user/user.ts`:
- Around line 9-15: Remove the redundant empty fields block inside
AuthenticatedUserRef.implement(): keep only the externalFields definition
(externalFields: (t) => ({ id: t.id({ nullable: false }) })) and delete the
fields: (t) => ({ /* ... */ }) entry and its comment so the implementation is
just the externalFields for the external reference.
There was a problem hiding this comment.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apis/api-gateway/schema.graphql`:
- Around line 2189-2191: The API_JOURNEYS_MODERN `@join__type` for AnonymousUser
currently has no key so it should not be resolving entity references; inspect
the api-journeys-modern schema and resolver code for any fields that return
AnonymousUser and if you find any, add a proper `@join__type`(..., key: "id") on
AnonymousUser in the api-journeys-modern subgraph and implement a
__resolveReference resolver in that service to return the entity (or
alternatively change those fields to return a type the subgraph already owns);
if no such fields exist, leave the join__type as extension-only and no resolver
is required.
|
I see you added the "on stage" label, I'll get this merged to the stage branch! |
|
Merge conflict attempting to merge this into stage. Please fix manually. |
Summary by CodeRabbit
Refactor
New Features