[Community][Admin] Member Management Page#198
Conversation
Reviewer's GuideImplements a full member management feature for community admins across domain, application services, GraphQL schema/resolvers, and UI, including listing members by community, adding/removing members, updating roles, and enforcing authorization and domain permissions with test coverage and Storybook stories. Sequence diagram for admin adding a member via Members pagesequenceDiagram
actor Admin
participant MembersPage as MembersPage
participant MemberAddModal as MemberAddModalContainer
participant Apollo as ApolloClient
participant GQL as GraphQLServer
participant MemberResolvers as memberResolvers
participant AppService as MemberApplicationService
participant DS as DataSources
participant DomainMember as MemberAggregate
Admin->>MembersPage: Open Members admin page
MembersPage->>Apollo: query AdminMemberListContainerMembersByCommunityId(communityId)
Apollo->>GQL: membersByCommunityId(communityId)
GQL->>MemberResolvers: Query.membersByCommunityId(args, context)
MemberResolvers->>AppService: Member.queryByCommunityId({ communityId })
AppService->>DS: MemberReadRepo.getByCommunityId(communityId)
DS-->>AppService: MemberEntityReference[]
AppService-->>MemberResolvers: MemberEntityReference[]
MemberResolvers-->>GQL: Member[]
GQL-->>Apollo: Member[]
Apollo-->>MembersPage: Member[]
Admin->>MembersPage: Click Add Member
MembersPage->>MemberAddModal: open = true
Admin->>MemberAddModal: Submit memberAdd form values
MemberAddModal->>Apollo: mutate AdminMemberAddContainerMemberAdd(input)
Apollo->>GQL: memberAdd(input)
GQL->>MemberResolvers: Mutation.memberAdd(args, context)
MemberResolvers->>AppService: Member.addMember(MemberAddCommand)
AppService->>DS: EndUserReadRepo.getByExternalId(userExternalId)
DS-->>AppService: EndUser(user)
AppService->>DS: EndUserReadRepo.getByExternalId(createdByExternalId)
DS-->>AppService: EndUser(createdBy)
AppService->>DS: CommunityReadRepo.getById(communityId)
DS-->>AppService: Community
AppService->>DS: MemberUnitOfWork.withScopedTransaction(callback)
DS->>DomainMember: getNewInstance(memberName, community)
DomainMember->>DomainMember: requestNewAccount()
DomainMember->>DomainMember: set account fields (createdBy, firstName, lastName, statusCode, user)
DomainMember->>DS: save(newMember)
DS-->>AppService: MemberEntityReference
AppService-->>MemberResolvers: MemberEntityReference
MemberResolvers-->>GQL: MemberMutationResult{ status.success = true, member }
GQL-->>Apollo: payload
Apollo-->>MemberAddModal: payload
MemberAddModal->>Apollo: refetch AdminMemberListContainerMembersByCommunityId
Apollo->>GQL: membersByCommunityId(communityId)
GQL-->>Apollo: Member[]
Apollo-->>MembersPage: update list
MemberAddModal-->>Admin: Show success message and close
Sequence diagram for admin removing a member and domain deletionsequenceDiagram
actor Admin
participant MembersPage as MembersPage
participant Apollo as ApolloClient
participant GQL as GraphQLServer
participant MemberResolvers as memberResolvers
participant AppService as MemberApplicationService
participant DS as DataSources
participant DomainMember as MemberAggregate
Admin->>MembersPage: Click Remove on member
MembersPage->>Apollo: mutate AdminMemberListContainerMemberRemove({ memberId })
Apollo->>GQL: memberRemove(input)
GQL->>MemberResolvers: Mutation.memberRemove(args, context)
MemberResolvers->>AppService: Member.removeMember(MemberRemoveCommand)
AppService->>DS: MemberUnitOfWork.withScopedTransaction(callback)
DS->>DomainMember: getById(memberId)
DomainMember->>DomainMember: requestDelete()
DomainMember->>DomainMember: check visa.canManageMembers or visa.isSystemAccount
alt authorized
DomainMember->>DomainMember: super.isDeleted = true
DomainMember->>DomainMember: addIntegrationEvent(MemberRemovedEvent)
else unauthorized
DomainMember-->>AppService: throw PermissionError
end
DomainMember->>DS: save(member)
DS-->>AppService: MemberEntityReference
AppService-->>MemberResolvers: MemberEntityReference
alt success
MemberResolvers-->>GQL: MemberMutationResult{ status.success = true }
else error
MemberResolvers-->>GQL: MemberMutationResult{ status.success = false, errorMessage }
end
GQL-->>Apollo: payload
Apollo-->>MembersPage: payload
MembersPage->>Apollo: refetch AdminMemberListContainerMembersByCommunityId
Apollo-->>MembersPage: updated members
MembersPage-->>Admin: Show success or error message
Updated class diagram for Member aggregate, events, and member application servicesclassDiagram
class MemberProps {
<<interface>>
string id
string memberName
string communityId
Date createdAt
Date updatedAt
}
class Passport {
<<value object>>
bool canManageMembers
bool isSystemAccount
bool determineIf(fn)
}
class Member {
+Member(props MemberProps, visa Passport)
+requestDelete() void
+loadCommunity() Promise~CommunityEntityReference~
+isDeleted bool
+communityId string
+role EndUserRoleEntityReference
}
class MemberRemovedEvent {
+props MemberRemovedProps
}
class MemberRemovedProps {
<<interface>>
string memberId
string communityId
}
class MemberAddedEvent {
+props MemberAddedProps
}
class MemberAddedProps {
<<interface>>
string memberId
string communityId
string memberName
}
class MemberApplicationService {
<<interface>>
+determineIfAdmin(command MemberDetermineIfAdminCommand) Promise~bool~
+queryByEndUserExternalId(command MemberQueryByEndUserExternalIdCommand) Promise~MemberEntityReference[]~
+queryByCommunityId(command MemberQueryByCommunityIdCommand) Promise~MemberEntityReference[]~
+addMember(command MemberAddCommand) Promise~MemberEntityReference~
+removeMember(command MemberRemoveCommand) Promise~MemberEntityReference~
+updateMemberRole(command MemberRoleUpdateCommand) Promise~MemberEntityReference~
}
class MemberAddCommand {
<<interface>>
string communityId
string memberName
string firstName
string lastName
string userExternalId
string createdByExternalId
}
class MemberRemoveCommand {
<<interface>>
string memberId
}
class MemberRoleUpdateCommand {
<<interface>>
string memberId
string roleId
}
class MemberQueryByCommunityIdCommand {
<<interface>>
string communityId
}
class MemberMutationResult {
<<GraphQL type>>
MutationStatus status
Member member
}
class MutationStatus {
bool success
string errorMessage
}
class MemberMutationResolverHelper {
+MemberMutationResolver(getMember Promise~MemberEntityReference~) Promise~MemberMutationResult~
}
MemberProps <|-- Member
Passport --> Member : visa
Member --> MemberRemovedEvent : emits
Member --> MemberAddedEvent : emits
MemberRemovedEvent --> MemberRemovedProps
MemberAddedEvent --> MemberAddedProps
MemberApplicationService --> MemberAddCommand
MemberApplicationService --> MemberRemoveCommand
MemberApplicationService --> MemberRoleUpdateCommand
MemberApplicationService --> MemberQueryByCommunityIdCommand
MemberMutationResult --> MutationStatus
MemberMutationResult --> Member
MemberMutationResolverHelper --> MemberMutationResult
MemberMutationResolverHelper --> MemberApplicationService
File-Level Changes
Assessment against linked issues
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 4 issues, and left some high level feedback:
- The new
member.resolvers.test.tsblock appears to duplicate several existing scenarios (including twotest.for(feature, ...)suites and repeatedQuerying members by community IDcases); consider consolidating into a single feature suite to avoid redundant tests and future drift. - In
update-member-role.ts,roleRepo.getByIdandmemberRepo.getByIdare assumed to succeed and any failure results in a genericMember not founderror; it would be safer and clearer to explicitly handle missing roles/members and surface distinct, accurate error messages. MemberAddModalaccepts acommunityIdprop but never uses it, and all context-specific work is done inMemberAddModalContainer; you can remove this unused prop (and related plumbing) to keep the presentational component’s API minimal.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The new `member.resolvers.test.ts` block appears to duplicate several existing scenarios (including two `test.for(feature, ...)` suites and repeated `Querying members by community ID` cases); consider consolidating into a single feature suite to avoid redundant tests and future drift.
- In `update-member-role.ts`, `roleRepo.getById` and `memberRepo.getById` are assumed to succeed and any failure results in a generic `Member not found` error; it would be safer and clearer to explicitly handle missing roles/members and surface distinct, accurate error messages.
- `MemberAddModal` accepts a `communityId` prop but never uses it, and all context-specific work is done in `MemberAddModalContainer`; you can remove this unused prop (and related plumbing) to keep the presentational component’s API minimal.
## Individual Comments
### Comment 1
<location path="packages/ocom/application-services/src/contexts/community/member/update-member-role.ts" line_range="9-12" />
<code_context>
+ roleId: string;
+}
+
+export const updateMemberRole = (dataSources: DataSources) => {
+ return async (command: MemberRoleUpdateCommand): Promise<Domain.Contexts.Community.Member.MemberEntityReference> => {
+ let memberToReturn: Domain.Contexts.Community.Member.MemberEntityReference | undefined;
+ await dataSources.domainDataSource.Community.Role.EndUserRole.EndUserRoleUnitOfWork.withScopedTransaction(async (roleRepo) => {
+ const role = await roleRepo.getById(command.roleId);
+ await dataSources.domainDataSource.Community.Member.MemberUnitOfWork.withScopedTransaction(async (memberRepo) => {
</code_context>
<issue_to_address>
**issue (bug_risk):** Handle the case where the role cannot be found before assigning it to the member.
If `roleRepo.getById(command.roleId)` returns `undefined` or throws, you may assign an invalid role or surface an unhelpful error. Please explicitly handle the "role not found" case (e.g., throw a clear domain error) before opening the member unit of work so callers can distinguish it from other failures.
</issue_to_address>
### Comment 2
<location path="packages/ocom/application-services/src/contexts/community/member/remove-member.ts" line_range="11-14" />
<code_context>
+ }
+
+ let memberToReturn: Domain.Contexts.Community.Member.MemberEntityReference | undefined;
+ await dataSources.domainDataSource.Community.Member.MemberUnitOfWork.withScopedTransaction(async (repo) => {
+ const newMember = await repo.getNewInstance(command.memberName, community as Domain.Contexts.Community.Community.CommunityEntityReference);
+ const newAccount = newMember.requestNewAccount();
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Clarify and tighten handling of a missing member in the remove-member flow.
Within the unit of work, `repo.getById(command.memberId)` is used before checking for a missing member. If `getById` can return `null`/`undefined`, `member.requestDelete()` will throw before the explicit `Member not found` check runs. Please either guard immediately after `getById` and throw a clear error, or guarantee that `getById` throws and remove the final `if (!memberToReturn)` as unreachable/redundant code.
</issue_to_address>
### Comment 3
<location path="apps/ui-community/src/components/layouts/admin/components/member-list.container.graphql" line_range="1" />
<code_context>
+query AdminMemberListContainerMembersByCommunityId($communityId: ObjectID!) {
+ membersByCommunityId(communityId: $communityId) {
+ ...AdminMemberListContainerMemberFields
</code_context>
<issue_to_address>
**issue (review_instructions):** This container GraphQL file is under `apps/ui-community/...`, which doesn’t match the specified `ui-<PortalName>/src/...` path pattern.
Per the instructions, container GraphQL files should live under a path like `ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql`.
This file is currently located at `apps/ui-community/src/components/layouts/admin/components/member-list.container.graphql`, which introduces an extra `apps/` segment and doesn’t strictly match the required pattern.
If the convention is still accurate, this file should be moved/renamed so its path matches the `ui-<PortalName>/src/...` pattern (for example, `ui-community/src/components/layouts/admin/components/member-list.container.graphql`).
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.container.graphql`
**Instructions:**
container graphql files should be found in the following path pattern: ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql
</details>
</issue_to_address>
### Comment 4
<location path="apps/ui-community/src/components/layouts/admin/components/member-add-modal.container.graphql" line_range="1" />
<code_context>
+mutation AdminMemberAddContainerMemberAdd($input: MemberAddInput!) {
+ memberAdd(input: $input) {
+ status {
</code_context>
<issue_to_address>
**issue (review_instructions):** This container GraphQL file’s path includes an `apps/` prefix, so it doesn’t strictly follow the specified `ui-<PortalName>/src/...` path pattern.
The convention states that container GraphQL files should be at:
`ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql`.
This file is at `apps/ui-community/src/components/layouts/admin/components/member-add-modal.container.graphql`, which doesn’t strictly adhere to that pattern because of the `apps/` prefix. If the documented convention is still current, consider relocating/renaming so the path conforms to the expected structure.
<details>
<summary>Review instructions:</summary>
**Path patterns:** `**/*.container.graphql`
**Instructions:**
container graphql files should be found in the following path pattern: ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql
</details>
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| export const updateMemberRole = (dataSources: DataSources) => { | ||
| return async (command: MemberRoleUpdateCommand): Promise<Domain.Contexts.Community.Member.MemberEntityReference> => { | ||
| let memberToReturn: Domain.Contexts.Community.Member.MemberEntityReference | undefined; | ||
| await dataSources.domainDataSource.Community.Role.EndUserRole.EndUserRoleUnitOfWork.withScopedTransaction(async (roleRepo) => { |
There was a problem hiding this comment.
issue (bug_risk): Handle the case where the role cannot be found before assigning it to the member.
If roleRepo.getById(command.roleId) returns undefined or throws, you may assign an invalid role or surface an unhelpful error. Please explicitly handle the "role not found" case (e.g., throw a clear domain error) before opening the member unit of work so callers can distinguish it from other failures.
| await dataSources.domainDataSource.Community.Member.MemberUnitOfWork.withScopedTransaction(async (repo) => { | ||
| const member = await repo.getById(command.memberId); | ||
| member.requestDelete(); | ||
| memberToReturn = await repo.save(member); |
There was a problem hiding this comment.
suggestion (bug_risk): Clarify and tighten handling of a missing member in the remove-member flow.
Within the unit of work, repo.getById(command.memberId) is used before checking for a missing member. If getById can return null/undefined, member.requestDelete() will throw before the explicit Member not found check runs. Please either guard immediately after getById and throw a clear error, or guarantee that getById throws and remove the final if (!memberToReturn) as unreachable/redundant code.
| @@ -0,0 +1,37 @@ | |||
| query AdminMemberListContainerMembersByCommunityId($communityId: ObjectID!) { | |||
There was a problem hiding this comment.
issue (review_instructions): This container GraphQL file is under apps/ui-community/..., which doesn’t match the specified ui-<PortalName>/src/... path pattern.
Per the instructions, container GraphQL files should live under a path like ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql.
This file is currently located at apps/ui-community/src/components/layouts/admin/components/member-list.container.graphql, which introduces an extra apps/ segment and doesn’t strictly match the required pattern.
If the convention is still accurate, this file should be moved/renamed so its path matches the ui-<PortalName>/src/... pattern (for example, ui-community/src/components/layouts/admin/components/member-list.container.graphql).
Review instructions:
Path patterns: **/*.container.graphql
Instructions:
container graphql files should be found in the following path pattern: ui-/src/components/layouts//components/.container.graphql
| @@ -0,0 +1,23 @@ | |||
| mutation AdminMemberAddContainerMemberAdd($input: MemberAddInput!) { | |||
There was a problem hiding this comment.
issue (review_instructions): This container GraphQL file’s path includes an apps/ prefix, so it doesn’t strictly follow the specified ui-<PortalName>/src/... path pattern.
The convention states that container GraphQL files should be at:
ui-<PortalName>/src/components/layouts/<AreaName>/components/<componentName>.container.graphql.
This file is at apps/ui-community/src/components/layouts/admin/components/member-add-modal.container.graphql, which doesn’t strictly adhere to that pattern because of the apps/ prefix. If the documented convention is still current, consider relocating/renaming so the path conforms to the expected structure.
Review instructions:
Path patterns: **/*.container.graphql
Instructions:
container graphql files should be found in the following path pattern: ui-/src/components/layouts//components/.container.graphql
Fixes #94
Summary by Sourcery
Add end-to-end support for community member management, including querying, adding, removing, and updating member roles from the admin interface.
New Features:
Enhancements: