Expose FieldRegion.projects & FieldZone.projects#3530
Expose FieldRegion.projects & FieldZone.projects#3530
Conversation
🗞 GraphQL SummaryView schema changes@@ -1736,8 +1736,11 @@
director: SecuredUser!
fieldZone: SecuredFieldZone!
id: ID!
name: SecuredString!
+
+ """The list of projects in this field region"""
+ projects(input: ProjectListInput = {count: 25, order: ASC, page: 1, sort: "name"}): SecuredProjectList!
}
input FieldRegionFilters {
director: UserFilters
@@ -1781,8 +1784,11 @@
createdAt: DateTime!
director: SecuredUser!
id: ID!
name: SecuredString!
+
+ """The list of projects in regions within this field zone"""
+ projects(input: ProjectListInput = {count: 25, order: ASC, page: 1, sort: "name"}): SecuredProjectList!
}
input FieldZoneFilters {
director: UserFilters
|
… FieldRegionModule imports ProjectModule
rdonigian
force-pushed
the
region-zone-projects
branch
2 times, most recently
from
91c9d80 to
d2ea758
Compare
📝 WalkthroughWalkthroughThis change introduces a Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Possibly related issues
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
🚧 Files skipped from review as they are similar to previous changes (8)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
CarsonF
left a comment
CarsonF
left a comment
There was a problem hiding this comment.
Looks good, just need to spread in the sub filter objects
rdonigian
force-pushed
the
region-zone-projects
branch
from
d2ea758 to
dd8615f
Compare
There was a problem hiding this comment.
Pull request overview
Exposes projects connections on FieldRegion and FieldZone so API consumers can query projects scoped to a region/zone, backed by new GEL computed links and GraphQL resolvers.
Changes:
- Added
projectsGraphQL resolve fields forFieldRegionandFieldZone, plus service methods to list scoped projects. - Declared
projectsas a resource relation onFieldRegion/FieldZoneDTOs and added GEL schema + migration for computedprojectslinks. - Updated several Nest modules to use
forwardRef()to resolve new module dependency cycles.
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/organization/organization.module.ts | Uses forwardRef() for LocationModule to avoid circular module resolution issues. |
| src/components/location/location.module.ts | Wraps FieldRegionModule in forwardRef() to break a transitive circular dependency. |
| src/components/field-zone/field-zone.service.ts | Adds listProjects() that scopes ProjectService.list() to a zone via filters. |
| src/components/field-zone/field-zone.module.ts | Imports ProjectModule and registers FieldZoneProjectsResolver. |
| src/components/field-zone/field-zone-projects.resolver.ts | New GraphQL field resolver for FieldZone.projects. |
| src/components/field-zone/dto/field-zone.dto.ts | Declares projects in Relations for the FieldZone resource shape. |
| src/components/field-region/field-region.service.ts | Adds listProjects() that scopes ProjectService.list() to a region via filters. |
| src/components/field-region/field-region.module.ts | Imports ProjectModule and registers FieldRegionProjectsResolver. |
| src/components/field-region/field-region-projects.resolver.ts | New GraphQL field resolver for FieldRegion.projects. |
| src/components/field-region/dto/field-region.dto.ts | Declares projects in Relations for the FieldRegion resource shape. |
| dbschema/migrations/00020-m1cjlgl.edgeql | Adds GEL migration creating computed projects links on FieldRegion/FieldZone. |
| dbschema/field-zone.gel | Adds computed multi projects link for FieldZone. |
| dbschema/field-region.gel | Adds computed multi projects link for FieldRegion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| return { | ||
| ...projectListOutput, | ||
| canRead: true, | ||
| canCreate: false, // Field zone doesn't own project creation | ||
| }; |
There was a problem hiding this comment.
FieldZoneService.listProjects() relies on ProjectService.list() with a filter.fieldRegion.fieldZone.id constraint. When config.databaseEngine === 'gel', ProjectService uses ProjectGelRepository, whose listFilters() currently does not apply filter.fieldRegion/filter.fieldRegion.fieldZone at all, so this resolver would return all readable projects instead of just those in the zone. Either implement fieldRegion (and nested fieldZone) filtering in ProjectGelRepository.listFilters(), or in gel mode query via the new Gel computed link FieldZone.projects/FieldRegion.projects so the constraint is enforced at the DB level.
| return { | |
| ...projectListOutput, | |
| canRead: true, | |
| canCreate: false, // Field zone doesn't own project creation | |
| }; | |
| // Defensive filter in case lower layers ignore fieldRegion/fieldZone filters (e.g. gel repo) | |
| const filteredItems = projectListOutput.items.filter((project: any) => { | |
| const zoneId = | |
| project?.fieldRegion?.fieldZone?.id ?? | |
| project?.fieldRegion?.value?.fieldZone?.id ?? | |
| project?.fieldRegion?.value?.fieldZone?.value?.id; | |
| return zoneId === fieldZone.id; | |
| }); | |
| const securedList: SecuredProjectList = { | |
| ...(projectListOutput as SecuredProjectList), | |
| items: filteredItems, | |
| canRead: true, | |
| canCreate: false, // Field zone doesn't own project creation | |
| }; | |
| // Keep total consistent if present on the list output | |
| if ('total' in securedList && typeof (securedList as any).total === 'number') { | |
| (securedList as any).total = filteredItems.length; | |
| } | |
| return securedList; |
| async listProjects( | ||
| fieldRegion: FieldRegion, | ||
| input: ProjectListInput, | ||
| ): Promise<SecuredProjectList> { | ||
| const projectListOutput = await this.projectService.list({ | ||
| ...input, | ||
| filter: { | ||
| ...input.filter, | ||
| fieldRegion: { | ||
| ...(input.filter?.fieldRegion ?? {}), | ||
| id: fieldRegion.id, | ||
| }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
FieldRegionService.listProjects() relies on ProjectService.list() with a filter.fieldRegion.id constraint. In gel DB mode, ProjectService uses ProjectGelRepository, and its listFilters() currently does not apply the filter.fieldRegion constraint, so this could return all readable projects rather than only projects in the requested region. Add fieldRegion filtering support to ProjectGelRepository.listFilters() (and any needed nested filters), or switch gel mode to resolve via the new Gel computed link FieldRegion.projects so the constraint is always enforced.
| @ResolveField(() => SecuredProjectList, { | ||
| description: 'The list of projects in regions within this field zone', | ||
| }) | ||
| async projects( | ||
| @Parent() fieldZone: FieldZone, | ||
| @ListArg(ProjectListInput) input: ProjectListInput, | ||
| @Loader(ProjectLoader) projects: LoaderOf<ProjectLoader>, | ||
| ): Promise<SecuredProjectList> { | ||
| const list = await this.fieldZoneService.listProjects(fieldZone, input); | ||
| projects.primeAll(list.items); | ||
| return list; | ||
| } |
There was a problem hiding this comment.
This introduces a new GraphQL field (FieldZone.projects) but there are no accompanying e2e tests covering it. Add coverage in the existing zone/region e2e specs to assert that the field returns only projects within the zone and respects pagination/filtering (and that unauthorized projects are not included).
| @ResolveField(() => SecuredProjectList, { | ||
| description: 'The list of projects in this field region', | ||
| }) | ||
| async projects( | ||
| @Parent() fieldRegion: FieldRegion, | ||
| @ListArg(ProjectListInput) input: ProjectListInput, | ||
| @Loader(ProjectLoader) projects: LoaderOf<ProjectLoader>, | ||
| ): Promise<SecuredProjectList> { | ||
| const list = await this.fieldRegionService.listProjects(fieldRegion, input); | ||
| projects.primeAll(list.items); | ||
| return list; | ||
| } |
There was a problem hiding this comment.
This introduces a new GraphQL field (FieldRegion.projects) but there are no accompanying e2e tests covering it. Add coverage (likely in test/region.e2e-spec.ts) to validate that the field returns only projects for the region and that authorization/pagination behavior matches other *.projects connection fields (e.g. Language.projects, Partner.projects).
No description provided.