Gsn member profile - #313
Conversation
Reviewer's GuideAdds a self-service member profile experience for community members, including front-end routing/layout, GraphQL schema and resolvers, and backend authorization for updating one’s own profile distinct from admin-driven updates. Sequence diagram for self-service member profile updatesequenceDiagram
actor User
participant MemberProfilePage
participant MemberProfileContainer
participant ApolloClient
participant GraphQLMemberResolvers
participant CommunityMemberService as CommunityMember.updateMemberProfile
participant MemberRepository as MemberRepository
User->>MemberProfilePage: Navigate to /:communityId/member/:memberId/profile
MemberProfilePage->>MemberProfileContainer: render(mode=self)
User->>MemberProfileContainer: submit MemberProfileFormValues
MemberProfileContainer->>ApolloClient: memberUpdateMyProfile(communityId, input)
ApolloClient->>GraphQLMemberResolvers: memberUpdateMyProfile(communityId, input)
GraphQLMemberResolvers->>GraphQLMemberResolvers: getActorMemberIdForCommunity(communityId)
GraphQLMemberResolvers->>CommunityMemberService: updateMemberProfile({ memberId: actorMemberId, actorMemberId, profile })
CommunityMemberService->>MemberRepository: getById(memberId)
MemberRepository-->>CommunityMemberService: member
CommunityMemberService->>CommunityMemberService: check actorMemberId === memberId
CommunityMemberService-->>GraphQLMemberResolvers: updated member
GraphQLMemberResolvers-->>ApolloClient: memberUpdateMyProfile.status, member
ApolloClient-->>MemberProfileContainer: mutation result
MemberProfileContainer->>MemberProfileContainer: message.success('Profile updated')
MemberProfileContainer-->>User: updated profile shown
Flow diagram for member self-service profile routingflowchart LR
A[Route /:communityId/member/:memberId/* in App] --> B[Member component]
B --> C[MemberSectionLayoutContainer]
C --> D[MemberSectionLayout]
D --> E[Route "" -> MemberHome]
D --> F[Route "profile/*" -> MemberProfilePage]
F --> G[MemberProfileContainer mode=self]
G --> H[memberMyProfile query]
G --> I[memberUpdateMyProfile mutation]
File-Level Changes
Possibly linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
There was a problem hiding this comment.
Hey - I've found 3 issues, and left some high level feedback:
- In MemberSectionLayoutContainer, if the current memberId isn’t found in membersForCurrentEndUser the component still renders MemberSectionLayout with an undefined memberData, which will likely blow up at runtime; consider explicitly handling the “member not found” case (e.g., by showing an error or redirect) instead of casting with
as MemberSectionLayoutContainerMemberFieldsFragment. - The member routes and links appear inconsistent: App.tsx registers the member route as
/:communityId/member/:memberId/*, but the Member pageLayouts use/community/:communityId/member/:memberIdand the header link goes to/community/accountswhile the accounts route is/accounts/*; aligning these paths will avoid broken navigation and menu highlighting issues. - The
buildMemberProfileSaveVariablestests passmemberObjectIdandcommunityIdinto both self and admin calls even though the self overload doesn’t declarememberObjectId, which will trigger excess property checking in TypeScript; adjust the helper’s types or the test call sites so the argument shapes match the overload signatures without relying on unsafe casting.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In MemberSectionLayoutContainer, if the current memberId isn’t found in membersForCurrentEndUser the component still renders MemberSectionLayout with an undefined memberData, which will likely blow up at runtime; consider explicitly handling the “member not found” case (e.g., by showing an error or redirect) instead of casting with `as MemberSectionLayoutContainerMemberFieldsFragment`.
- The member routes and links appear inconsistent: App.tsx registers the member route as `/:communityId/member/:memberId/*`, but the Member pageLayouts use `/community/:communityId/member/:memberId` and the header link goes to `/community/accounts` while the accounts route is `/accounts/*`; aligning these paths will avoid broken navigation and menu highlighting issues.
- The `buildMemberProfileSaveVariables` tests pass `memberObjectId` and `communityId` into both self and admin calls even though the self overload doesn’t declare `memberObjectId`, which will trigger excess property checking in TypeScript; adjust the helper’s types or the test call sites so the argument shapes match the overload signatures without relying on unsafe casting.
## Individual Comments
### Comment 1
<location path="packages/ocom/graphql/src/schema/types/member.resolvers.ts" line_range="777-786" />
<code_context>
},
+
+ // ...existing code...
+ memberUpdateMyProfile: async (_parent: unknown, args: MutationMemberUpdateMyProfileArgs, context: GraphContext) => {
+ try {
+ if (!context.applicationServices.verifiedUser?.verifiedJwt) {
+ return {
+ status: { success: false, errorMessage: 'Unauthorized' },
+ member: null,
+ };
+ }
+
+ const actorMemberId = await getActorMemberIdForCommunity(context, String(args.communityId));
+ if (!actorMemberId) {
+ return {
+ status: { success: false, errorMessage: 'Forbidden' },
+ member: null,
+ };
+ }
+
+ const command: MemberUpdateProfileCommand = {
+ memberId: actorMemberId,
+ actorMemberId,
</code_context>
<issue_to_address>
**issue (bug_risk):** Self-profile visibility flags are only partially mapped to the domain model.
In `memberUpdateMyProfile`, the `visibility` input is only applied to `showInterests` and `showEmail`, while `showProfile`, `showLocation`, and `showProperties` are ignored. If these flags are meant to be user-editable via self-update, they should also be included in the `MemberUpdateProfileCommand` payload; otherwise, self updates will diverge from admin-initiated updates in how visibility is handled.
</issue_to_address>
### Comment 2
<location path="packages/ocom/ui-community-route-accounts/src/components/member-section-layout.container.tsx" line_range="25" />
<code_context>
+ hasDataComponent={
</code_context>
<issue_to_address>
**issue (bug_risk):** Member lookup by route param may return undefined but is cast and passed as non-null.
If no member in `membersForCurrentEndUser` has an `id` matching the `memberId` route param, `find(...)` returns `undefined`, but it’s cast to `MemberSectionLayoutContainerMemberFieldsFragment` and passed to `MemberSectionLayout`. This will cause runtime errors when `memberData` is dereferenced. Please add a guard for the missing-member case (e.g., choose a fallback member, render an error state, or use `hasData`/`hasDataComponent` so the loader handles it) instead of relying on the cast.
</issue_to_address>
### Comment 3
<location path="packages/ocom/ui-community-shared/src/components/member-profile.container.tsx" line_range="56" />
<code_context>
+ name: 'Jane Doe',
+ email: 'jane@example.com',
+ bio: 'Hello there',
+ interests: [],
+ visibility: {
+ showEmail: true,
</code_context>
<issue_to_address>
**question (bug_risk):** Self-profile updates always send an empty interests array, potentially clearing existing interests.
In the self-mode branch of `buildMemberProfileSaveVariables`, `interests` is always set to `[]`. Because the GraphQL field is `[String!]!`, every self-profile save will clear any existing interests. If interests should be preserved, either include them in `MemberProfileFormValues` and map them through, or avoid setting the field at all so existing values remain unchanged.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| memberUpdateMyProfile: async (_parent: unknown, args: MutationMemberUpdateMyProfileArgs, context: GraphContext) => { | ||
| try { | ||
| if (!context.applicationServices.verifiedUser?.verifiedJwt) { | ||
| return { | ||
| status: { success: false, errorMessage: 'Unauthorized' }, | ||
| member: null, | ||
| }; | ||
| } | ||
|
|
||
| const actorMemberId = await getActorMemberIdForCommunity(context, String(args.communityId)); |
There was a problem hiding this comment.
issue (bug_risk): Self-profile visibility flags are only partially mapped to the domain model.
In memberUpdateMyProfile, the visibility input is only applied to showInterests and showEmail, while showProfile, showLocation, and showProperties are ignored. If these flags are meant to be user-editable via self-update, they should also be included in the MemberUpdateProfileCommand payload; otherwise, self updates will diverge from admin-initiated updates in how visibility is handled.
| <MemberSectionLayout | ||
| pageLayouts={props.pageLayouts} | ||
| // biome-ignore lint:useLiteralKeys | ||
| memberData={membersData?.membersForCurrentEndUser.find((member: MemberSectionLayoutContainerMemberFieldsFragment) => member.id === params['memberId']) as MemberSectionLayoutContainerMemberFieldsFragment} |
There was a problem hiding this comment.
issue (bug_risk): Member lookup by route param may return undefined but is cast and passed as non-null.
If no member in membersForCurrentEndUser has an id matching the memberId route param, find(...) returns undefined, but it’s cast to MemberSectionLayoutContainerMemberFieldsFragment and passed to MemberSectionLayout. This will cause runtime errors when memberData is dereferenced. Please add a guard for the missing-member case (e.g., choose a fallback member, render an error state, or use hasData/hasDataComponent so the loader handles it) instead of relying on the cast.
| name: values.name, | ||
| email: values.email, | ||
| bio: values.bio, | ||
| interests: [], |
There was a problem hiding this comment.
question (bug_risk): Self-profile updates always send an empty interests array, potentially clearing existing interests.
In the self-mode branch of buildMemberProfileSaveVariables, interests is always set to []. Because the GraphQL field is [String!]!, every self-profile save will clear any existing interests. If interests should be preserved, either include them in MemberProfileFormValues and map them through, or avoid setting the field at all so existing values remain unchanged.
f25032a to
29fbf92
Compare
Summary by Sourcery
Introduce a member-facing community portal with self-service profile management and supporting GraphQL APIs.
New Features:
Bug Fixes:
Enhancements:
Tests: