Gsn member profile - #313
Open
nikithaguduru wants to merge 12 commits into
Open
Conversation
Contributor
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
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
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.
Comment on lines
+777
to
+786
| 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)); |
Contributor
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} |
Contributor
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: [], |
Contributor
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.
nikithaguduru
force-pushed
the
gsn_member_profile
branch
from
August 6, 2026 18:32
f25032a to
29fbf92
Compare
noce-nick
requested changes
Aug 14, 2026
Member
There was a problem hiding this comment.
Undo all the changes in this file. We do always need func core tools installed and available on the build machine, bc the e2e-tests need func to be available at runtime when it's going through those serenity scenarios
| const member = await repository.getById(command.memberId); | ||
| const profile = member.profile; | ||
|
|
||
| if (command.actorMemberId && String(command.actorMemberId) !== String(command.memberId)) { |
Member
There was a problem hiding this comment.
This permission check is not expected to be in the application services. The application services automatically scopes a passport for the acting user on the initialized member unit of work you're using here. The permission checks should be enforced through the domain aggregate/entity classes and their setters.
When you call profile.name = command.profile.name, that is actually hitting the set name on the MemberProfile entity. so that setter is where we would want to actually enforce the expected domain permission check of permissions.isEditingOwnMember (which the member's visa will perform the Object ID comparison for you automatically). Let me know if you have any questions, we can go over this in more detail.
Member
There was a problem hiding this comment.
Remove this .test.ts file, we expect to be using .stories.tsx files to test the individual UI components/pages. I would expect a member-profile.container.stories.tsx here instead to replace this file and member-profile.container.test.tsx.
We similarly need accompanying .stories.tsx files for all the new pages and components that were added to support this new member functionality, i.e. member-home.stories.tsx, `member-profile.stories.tsx
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.
Summary by Sourcery
Introduce a member-facing community portal with self-service profile management and supporting GraphQL APIs.
New Features:
Bug Fixes:
Enhancements:
Tests: