FieldRegion & FieldZone filtering & sorting #3503
Conversation
📝 WalkthroughWalkthroughThis change introduces enhanced and more granular filtering and sorting capabilities for the FieldRegion, FieldZone, and Project entities. It adds nested filter fields and corresponding sorting logic, enabling queries based on related entities such as directors and zones. The updates affect DTOs, repositories, and filter/sorter definitions, supporting complex, relation-aware queries. Changes
✨ Finishing Touches
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/components/field-zone/field-zone.repository.ts (1)
135-137: Same concern as raised forFieldRegionRepository.list: protect againstundefinedfilters to avoid brittle assumptions infieldZoneFilters.
🧹 Nitpick comments (9)
src/components/project/project-filters.query.ts (1)
111-117: Add a description for the newfieldRegionfilter to keep API docs consistentMost of the existing filters include
descriptiontext so the generated GraphQL schema is self-documenting.
Adding the same here keeps the public API consistent and discoverable.- fieldRegion: filter.sub(() => fieldRegionFilters)((sub) => + fieldRegion: filter.sub(() => fieldRegionFilters, { + description: 'Only projects whose field region matches these filters', + })((sub) =>src/components/project/dto/list-projects.dto.ts (1)
138-140: Document thefieldRegionfilter fieldOther
FilterFields have a description, which shows up in the GraphQL docs and helps clients know what the field does. Consider adding one for parity.- @FilterField(() => FieldRegionFilters) + @FilterField(() => FieldRegionFilters, { + description: 'Only projects whose field region matches these filters', + })src/components/project/project.repository.ts (1)
461-476: Repeated sorter pattern could be abstracted
primaryLocation.*,primaryPartnership.*, and nowfieldRegion.*all share the same “match or return null” union pattern. Extracting this into a small helper (e.g.defineRelationshipSorter(rel, label, sorters)) would DRY up the repository and make adding the next nested sorter trivial.No code change required in this PR.
src/components/field-zone/dto/list-field-zone.dto.ts (1)
18-20: Missing descriptions on new filter fields
idanddirectorlackdescriptionmetadata, unlike similar filters elsewhere. Adding them improves generated docs.@IdField({ optional: true, description: 'Filter by zone id' }) ... @FilterField(() => UserFilters, { description: 'Only zones directed by users matching these filters', })src/components/field-region/dto/list-field-region.dto.ts (1)
16-24: Add descriptions for new properties & verify public exposure
- Provide
descriptiontext forid,director, andfieldZoneto align with other DTOs.- The
filterfield is now public (internal flag removed). Ensure this breaking-schema change is intentional.- @IdField({ optional: true }) + @IdField({ optional: true, description: 'Filter by region id' }) ... - @FilterField(() => UserFilters) + @FilterField(() => UserFilters, { + description: 'Only regions directed by users matching these filters', + }) ... - @FilterField(() => FieldZoneFilters) + @FilterField(() => FieldZoneFilters, { + description: 'Only regions whose field zone matches these filters', + })src/components/field-region/field-region.repository.ts (2)
141-157: Alias shadowing in nested filter match blocks can lead to hard-to-trace bugsBoth sub-filters reuse the
"node"alias for the inner entity while the outer entity is silently renamed to"outer".
If another developer later nests additional filters/sorters that also expect"node"to be the outer alias, queries will silently break.
Consider using distinct aliases (directorNode,fieldZoneNode, …) or a helper that guarantees alias uniqueness.
159-180: Duplicate sorter scaffolding – opportunity for a shared helperThe two blocks are 100 % identical aside from relationship name and delegated sorters.
Extracting a higher-order helper (nestedSorter('director', userSorters)/nestedSorter('zone', fieldZoneSorters)) would:
- eliminate duplication,
- enforce a uniform pattern across repositories, and
- make future maintenance (e.g., tweaking
with/matchstructure) one-shot.No functional issue, purely maintainability.
src/components/field-zone/field-zone.repository.ts (2)
156-165: DRY up director sub-filter logicThe
directorsub-filter here duplicates the pattern already defined infieldRegionFiltersand (potentially) other repositories.
Extract a shared helper (e.g.,directorSubFilter()) so any change to the relationship pattern or aliasing is done once.
167-178: Shared nested sorter helper would trim repetitionMirrors the comment on
fieldRegionSorters: consider a reusablenestedSorterutility to generate these boilerplate blocks and ensure consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
src/components/field-region/dto/list-field-region.dto.ts(2 hunks)src/components/field-region/field-region.repository.ts(3 hunks)src/components/field-zone/dto/list-field-zone.dto.ts(2 hunks)src/components/field-zone/field-zone.repository.ts(3 hunks)src/components/project/dto/list-projects.dto.ts(2 hunks)src/components/project/project-filters.query.ts(2 hunks)src/components/project/project.repository.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
src/components/project/dto/list-projects.dto.ts (1)
src/common/filter-field.ts (1)
FilterField(11-31)
src/components/project/project-filters.query.ts (3)
src/components/field-region/field-region.repository.ts (1)
fieldRegionFilters(141-157)src/core/database/query/filters.ts (1)
sub(243-272)src/core/database/query/matching.ts (1)
ACTIVE(33-33)
src/components/field-region/dto/list-field-region.dto.ts (4)
src/components/field-zone/dto/list-field-zone.dto.ts (1)
InputType(22-30)src/components/user/dto/list-users.dto.ts (1)
InputType(35-41)src/common/id-field.ts (2)
IdField(9-19)ID(24-25)src/common/filter-field.ts (1)
FilterField(11-31)
⏰ Context from checks skipped due to timeout of 90000ms (12)
- GitHub Check: E2E Tests (neo4j 3/6)
- GitHub Check: E2E Tests (neo4j 6/6)
- GitHub Check: E2E Tests (neo4j 1/6)
- GitHub Check: E2E Tests (neo4j 4/6)
- GitHub Check: E2E Tests (neo4j 2/6)
- GitHub Check: E2E Tests (neo4j 5/6)
- GitHub Check: Unit
- GitHub Check: Clean
- GitHub Check: lint
- GitHub Check: Generate (base)
- GitHub Check: Generate (head)
- GitHub Check: Analyze (javascript)
🔇 Additional comments (2)
src/components/field-zone/dto/list-field-zone.dto.ts (1)
15-20: Exposing the filter publicly – confirm this is intentional
FieldZoneFilterswas previously internal; removing{ internal: true }makes it part of the public GraphQL schema.
Double-check that this is the desired API change and that clients are prepared for the newfilterargument.src/components/field-region/field-region.repository.ts (1)
118-123: Handle undefinedfilterargument to avoid runtime blows
input.filtermay legitimately beundefined.
fieldRegionFilters(undefined)relies on internal defaults that could change and would currently yield a runtimeTypeErrorif the util starts touching properties. Guard at the call-site to make the contract explicit.- .apply(fieldRegionFilters(input.filter)) + .apply(fieldRegionFilters(input.filter ?? {}))
🗞 GraphQL SummaryView schema changes@@ -1736,11 +1736,18 @@
id: ID!
name: SecuredString!
}
+input FieldRegionFilters {
+ director: UserFilters
+ fieldZone: FieldZoneFilters
+ id: ID
+}
+
input FieldRegionListInput {
"""The number of items to return in a single page"""
count: Int! = 25
+ filter: FieldRegionFilters
"""The order in which to sort the list"""
order: Order! = ASC
@@ -1774,11 +1781,17 @@
id: ID!
name: SecuredString!
}
+input FieldZoneFilters {
+ director: UserFilters
+ id: ID
+}
+
input FieldZoneListInput {
"""The number of items to return in a single page"""
count: Int! = 25
+ filter: FieldZoneFilters
"""The order in which to sort the list"""
order: Order! = ASC
@@ -4845,8 +4858,9 @@
input ProjectFilters {
"""Only projects created within this time range"""
createdAt: DateTimeFilter
+ fieldRegion: FieldRegionFilters
"""
Only projects that the requesting user is an active member of. false does nothing.
"""
|
Part of #3500