-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: change validator in observatory service #2770
Conversation
WalkthroughThis pull request introduces a comprehensive refactoring of the observatory service's data validation and type structures. The changes primarily focus on replacing contract and schema files with more robust, type-safe data transfer objects (DTOs) using the Changes
Sequence DiagramsequenceDiagram
participant Client
participant Action
participant DTO
participant Repository
participant Database
Client->>Action: Request with parameters
Action->>DTO: Validate parameters
DTO-->>Action: Validated parameters
Action->>Repository: Query with validated parameters
Repository->>Database: Execute query
Database-->>Repository: Return results
Repository-->>Action: Return results
Action-->>Client: Return validated results
Possibly related PRs
Poem
✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (41)
api/src/pdc/services/observatory/dto/occupation/Occupation.ts (3)
1-10
: Consider clarifying imports or grouping them for readability.It can be beneficial to group related imports (e.g., all superstruct-related imports together and domain types together) and add a brief comment clarifying their source or usage, which can help future maintainers quickly identify library vs. local abstractions.
12-21
: Add inline documentation for each property.Currently, there is no inline documentation or JSDoc describing the purpose of each property, such as
observe
ordirection
. This can make the codebase more maintainable and self-explanatory, especially for new contributors.
23-23
: Rename the constant or the type alias to avoid potential confusion.You have an exported const
Occupation
and also exporttype Occupation = Infer<typeof Occupation>
. Although valid in TypeScript, having the same identifier for both can lead to confusion. Consider renaming one for clarity, such asOccupationStruct
orOccupationType
.api/src/pdc/services/observatory/actions/occupation/OccupationAction.ts (2)
7-7
: Clarify name for multiple results.
ResultInterface
is declared as an array. If multiple records are expected, consider renaming it to clarify that it's a collection of results (e.g.,OccupationResultListInterface
).
21-21
: Validate theOccupation
schema.Adding
Occupation
to thevalidate
middleware ensures parameter validation before proceeding. Confirm thatOccupation
covers all required fields (e.g., date ranges, codes). If optional or extended fields are needed, include them in the schema to prevent run-time errors.api/src/pdc/services/observatory/interfaces/FluxRepositoryProviderInterface.ts (3)
36-37
: Use a descriptive error message in the abstract method.Throwing an error without context can make debugging difficult. Consider specifying which method is unimplemented.
- throw new Error(); + throw new Error('Method not implemented: getFlux in FluxRepositoryInterfaceResolver');
42-43
: Add context to the thrown error for getEvolFlux.Including a short explanation of the missing implementation helps future debugging.
- throw new Error(); + throw new Error('Method not implemented: getEvolFlux in FluxRepositoryInterfaceResolver');
48-49
: Improve error messaging for getBestFlux as well.Like the other abstract methods, a more descriptive error aids troubleshooting.
- throw new Error(); + throw new Error('Method not implemented: getBestFlux in FluxRepositoryInterfaceResolver');api/src/pdc/services/observatory/actions/flux/EvolFluxAction.ts (1)
6-14
: Ensure comprehensive type definitions.These newly introduced properties in
ResultInterface
look consistent with the EvolFlux DTO. However, consider adding short doc comments for each property to clarify their business meaning and potential edge cases.api/src/pdc/services/observatory/dto/flux/EvolFlux.ts (1)
12-21
: Leverage consistent naming for clarity.This data structure effectively captures the fields needed for EvolFlux. Confirm that your
indic
set is exhaustive (e.g., "trips" & "journeys" may overlap conceptually). If needed, consider clarifying them further in doc comments or refactoring for clarity.api/src/pdc/services/observatory/providers/FluxRepositoryProvider.ts (1)
131-132
: BestFlux now references new interfaces.Ensure the newly introduced
BestFluxParamsInterface
covers all logic needed by the top-10 results. Check if any corner cases—like missing or invalidlimit
—are handled.api/src/pdc/services/observatory/actions/occupation/EvolOccupationAction.ts (2)
6-13
: Clarify the distinction between journeys and trips
Thejourneys?
andtrips?
properties might be confusing if they represent similar concepts. Consider unifying them or providing more descriptive naming to reduce ambiguity. This helps maintain clarity for downstream consumers ofResultInterface
.
28-28
: Consider adding error handling for repository calls
Thehandle
method simply delegates tothis.repository.getEvolOccupation(params)
. In the event the repository layer throws an exception or returns invalid data, this might go unhandled. Consider adding error handling or logging to improve fault tolerance and debuggability.api/src/pdc/services/observatory/interfaces/KeyfiguresRepositoryProviderInterface.ts (1)
4-4
: Ensure naming consistency.Renaming
KeyFigures
toKeyfiguresParamsInterface
helps unify the naming convention. Confirm that existing references toParamsInterface
(if any) are updated to reflect the new name, preventing any confusion or mismatches across DTO usage.api/src/pdc/services/observatory/dto/KeyFigures.ts (1)
22-22
: Rename the exported struct or type to avoid potential confusion.
Having bothKeyFigures
as a constant (in the value space) and a type (in the type space) can sometimes be confusing. Consider renaming one to preserve clarity, for example:-export const KeyFigures = object({ ... }); -export type KeyFigures = Infer<typeof KeyFigures>; +export const KeyFiguresStruct = object({ ... }); +export type KeyFigures = Infer<typeof KeyFiguresStruct>;api/src/pdc/services/observatory/interfaces/IncentiveCampaignsRepositoryProviderInterface.ts (1)
Line range hint
8-14
: Improve error-handling for production readiness.Throwing a generic
Error
in the abstract class is acceptable for a placeholder, but consider using a more descriptive error or logging mechanism in future implementations. This helps with debugging and user feedback.api/src/pdc/services/observatory/dto/IncentiveCampaigns.ts (2)
4-8
: Consider adding field-level documentation.
While the object definition withoptional
validators is correct, adding short doc comments or TSDoc for each field helps improve clarity for future maintainers.
10-10
: Name collision between constant and type.
Defining both the constant and the type asIncentiveCampaigns
might cause confusion in references. Consider using a suffix likeIncentiveCampaignsStruct
for the constant orIncentiveCampaignsType
for the type.api/src/pdc/services/observatory/actions/incentiveCampaigns/CampaignsAction.ts (2)
7-35
: Enhance typed documentation for the result interface.
TheResultInterface
array adequately types all required campaign fields. Consider adding doc comments at the property level to describe each field’s purpose.
52-52
: Include error-handling strategy for repository calls.
When retrieving data via the repository, consider adding try/catch logic or fallback handling to deal with potential failures or empty results.api/src/pdc/services/observatory/dto/flux/Flux.ts (2)
1-9
: Consider renaming or restructuring imports for consistency.
Importing everything from multiple paths (e.g.,@/lib/superstruct
and@/pdc/providers/superstruct/shared
) is fine, but you might consider consolidating your imports or re-exporting them in a single file to simplify maintenance and ensure consistency across your codebase.
21-21
: Avoid naming collisions between the struct and the inferred type.
Using the same identifierFlux
for both the struct and the type can be confusing. As a best practice, consider naming them distinctly, for example:-export const Flux = object({ +export const FluxStruct = object({ ... }); -export type Flux = Infer<typeof Flux>; +export type FluxType = Infer<typeof FluxStruct>;api/src/pdc/services/observatory/actions/flux/FluxAction.ts (3)
6-16
: Confirm whether you need a dedicated interface or a domain-specific name.
ThisResultInterface
is an array of objects with territory codes, lat/lng, and trip data. If it’s going to be reused in multiple places, consider a clearer domain-specific name (e.g.,FluxQueryResult
) and use an interface if you anticipate extension. Keeping it as a type is perfectly valid but can hamper extension if future properties need to be optional.
23-23
: Add custom error handling or messages.
Using theFlux
struct in the “validate“ middleware is good for type safety. If you need user-friendly or localized error handling, consider customizing the superstruct validation error messages.
31-31
: Check parameter naming across the codebase.
Thepublic async handle(params: Flux)
signature is concise, but if you have multiple domain objects, consider using more descriptive parameter names (e.g.,fluxParams
). This can reduce confusion for new contributors.api/src/pdc/services/observatory/interfaces/LocationRepositoryProviderInterface.ts (1)
6-6
: Consider simplifying type naming to maintain consistency.While re-exporting both interfaces in one statement is convenient, the suffix
Interface
for bothLocationParamsInterface
andLocationResultInterface
might become cumbersome as the code evolves. Consider renaming them to more concise identifiers that still convey meaning, such asLocationDto
andLocationResultDto
, to maintain consistency across DTOs.Example diff illustrating a possible rename:
-import type { Location as LocationParamsInterface } from "@/pdc/services/observatory/dto/Location.ts"; +import type { Location as LocationDto } from "@/pdc/services/observatory/dto/Location.ts"; -export type { LocationParamsInterface, LocationResultInterface }; +export type { LocationDto, LocationResultInterface };api/src/pdc/services/observatory/interfaces/IncentiveRepositoryProviderInterface.ts (1)
1-2
: Consider simplifying the alias to improve clarity.Renaming the alias from
"Incentive"
to reflect parameter usage (e.g."IncentiveParams"
) makes the code more self-documenting for maintainers.Apply this diff to rename the alias for the new import:
- import type { Incentive as IncentiveParamsInterface } from "@/pdc/services/observatory/dto/Incentive.ts"; + import type { IncentiveParams as IncentiveParamsInterface } from "@/pdc/services/observatory/dto/Incentive.ts";api/src/pdc/services/observatory/actions/incentive/IncentiveAction.ts (1)
31-31
: Error handling consideration in repository call.This
handle
method delegates entirely torepository.getIncentive(params)
. If there's a possibility of repository failure, consider adding exception handling or fallback logic to ensure graceful handling of errors.api/src/pdc/services/observatory/actions/flux/BestFluxAction.ts (2)
6-12
: Result interface looks consistent.
ExportingResultInterface
as an array of objects referencingBestFlux["code"]
is type-safe and coherent with the new DTO pattern. Consider verifying if any additional fields might be necessary based on the repository return structure.
27-27
: Consider adding repository error handling.
IfgetBestFlux
throws an error or returns unexpected data, the action might fail silently. Wrapping the call in a try/catch (with appropriate logging or error transformation) can improve error resilience.api/src/pdc/services/observatory/dto/distribution/JourneysByDistances.ts (1)
12-20
: Validate default or fallback values for missing fields.While
month
,trimester
,semester
, anddirection
are marked optional, consider whether default values or descriptive error messages would help maintain consistent data. This might reduce the likelihood of runtime issues when these properties are missing or null.api/src/pdc/services/observatory/dto/Location.ts (1)
12-20
: Use specialized types if partial date segments are domain-specific.You have separate optional fields for
month
,trimester
, andsemester
. If these alternative time granularities are part of a domain concept (e.g., user can only select one time segment at a time), consider a union-based approach or a single field with enumerated time segments to reduce potential conflicts.api/src/pdc/services/observatory/dto/flux/BestFlux.ts (1)
13-21
: Consider unifying shared fields across DTO objects.
year
,type
,code
, possiblydirection
, andlimit
appear repeatedly in different DTOs. Extracting a base schema or leveraging composition (e.g.,extend()
) may improve maintainability by avoiding duplication.api/src/pdc/services/observatory/dto/occupation/EvolOccupation.ts (2)
19-20
: Check default value range
Usingdefaulted(CoerceNumberMinMax(integer(), 1, 5), 2)
ensurespast
remains between 1 and 5 with a default of 2. If you anticipate values outside this range in the future, consider making this range more flexible or documenting its rationale.
12-21
: Consider adding descriptive JSDoc
Documenting each field (especiallypast
andindic
) can help future collaborators understand the intended usage and constraints without searching through multiple files.api/src/pdc/services/observatory/actions/location/LocationAction.ts (1)
6-9
: Clear definition for the result structure
DefiningResultInterface
as an array of{ hex: string; count: number }
objects clarifies the response format. Ensure consistent naming conventions, e.g., ishex
always a valid hex code or a reference to a location ID?api/src/pdc/services/observatory/actions/distribution/JourneysByHoursAction.ts (1)
8-18
: Add a numeric range guard forhour
if appropriate
Thehour
field is typed tonumber
. If the domain expects a 24-hour format, consider enforcing a range of0–23
.export type ResultInterface = { code: string; libelle: string; direction: Infer<typeof Direction>; hours: [ { - hour: number; + // If your domain expects hours within the 24-hour clock + hour: number; // Possibly enforce range 0..23 using a custom struct journeys: number; }, ]; }[];api/src/pdc/services/observatory/actions/distribution/JourneysByDistancesAction.ts (1)
8-16
: Consider enumerating distance classes
Ifdist_classes
is restricted to a known set of categories, enumerating them can help catch invalid inputs earlier.export type ResultInterface = { code: string; libelle: string; direction: Infer<typeof Direction>; distances: { - dist_classes: string; + // Alternatively, define a narrower set of valid categories + dist_classes: 'SHORT' | 'MEDIUM' | 'LONG' | string; journeys: number; }[]; }[];api/src/pdc/services/observatory/interfaces/DistributionRepositoryProviderInterface.ts (1)
7-12
: Consider standardizing naming patterns
You’re importingJourneysByDistances
asJourneysByDistancesParamsInterface
and likewise forJourneysByHours
. For simplicity and consistency, you may want to rename one or the other so the type alias clearly indicates it’s for parameters.import type { - JourneysByDistances as JourneysByDistancesParamsInterface, + JourneysByDistances as JourneysByDistancesDTO, } from "@/pdc/services/observatory/dto/distribution/JourneysByDistances.ts";api/src/pdc/providers/superstruct/shared/index.ts (2)
34-38
: Validate the choice of year constraints and ID range.
Year
is capped at the current year, which might be too restrictive if future dates are needed.- The
Id
struct is fine at 32-bit range.Month
,Trimester
, andSemester
are clear uses of min–max constraints.
39-42
: Clarify naming or add inline documentation for territorial patterns.
Country
uses a 5-digit or "X" pattern, which might be confusing without context.Department
,Insee
, andSiren
patterns appear correct for local administrative codes but would benefit from inline comments to explain usage.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (66)
api/src/pdc/providers/superstruct/shared/index.ts
(2 hunks)api/src/pdc/services/observatory/ObservatoryServiceProvider.ts
(2 hunks)api/src/pdc/services/observatory/actions/distribution/JourneysByDistancesAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/distribution/JourneysByHoursAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/flux/BestFluxAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/flux/EvolFluxAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/flux/FluxAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/incentive/IncentiveAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/incentiveCampaigns/CampaignsAction.ts
(2 hunks)api/src/pdc/services/observatory/actions/infra/AiresCovoiturageAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/keyfigures/KeyfiguresAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/location/LocationAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/occupation/BestTerritoriesAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/occupation/EvolOccupationAction.ts
(1 hunks)api/src/pdc/services/observatory/actions/occupation/OccupationAction.ts
(1 hunks)api/src/pdc/services/observatory/config/ajv.ts
(0 hunks)api/src/pdc/services/observatory/config/index.ts
(0 hunks)api/src/pdc/services/observatory/contracts/distribution/journeysByDistances.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/distribution/journeysByDistances.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/distribution/journeysByHours.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/distribution/journeysByHours.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getBestFlux.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getBestFlux.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getEvolFlux.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getEvolFlux.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getFlux.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/flux/getFlux.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/incentive/getIncentive.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/incentive/getIncentive.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/incentiveCampaigns/campaigns.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/incentiveCampaigns/campaigns.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/infra/airesCovoiturage.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/infra/airesCovoiturage.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/keyfigures/getKeyfigures.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/keyfigures/getKeyfigures.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/location/location.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/location/location.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getBestTerritories.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getBestTerritories.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getEvolOccupation.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getEvolOccupation.schema.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getOccupation.contract.ts
(0 hunks)api/src/pdc/services/observatory/contracts/occupation/getOccupation.schema.ts
(0 hunks)api/src/pdc/services/observatory/dto/Incentive.ts
(1 hunks)api/src/pdc/services/observatory/dto/IncentiveCampaigns.ts
(1 hunks)api/src/pdc/services/observatory/dto/KeyFigures.ts
(1 hunks)api/src/pdc/services/observatory/dto/Location.ts
(1 hunks)api/src/pdc/services/observatory/dto/distribution/JourneysByDistances.ts
(1 hunks)api/src/pdc/services/observatory/dto/distribution/JourneysByHours.ts
(1 hunks)api/src/pdc/services/observatory/dto/flux/BestFlux.ts
(1 hunks)api/src/pdc/services/observatory/dto/flux/EvolFlux.ts
(1 hunks)api/src/pdc/services/observatory/dto/flux/Flux.ts
(1 hunks)api/src/pdc/services/observatory/dto/infra/AiresCovoiturage.ts
(1 hunks)api/src/pdc/services/observatory/dto/occupation/BestTerritories.ts
(1 hunks)api/src/pdc/services/observatory/dto/occupation/EvolOccupation.ts
(1 hunks)api/src/pdc/services/observatory/dto/occupation/Occupation.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/DistributionRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/FluxRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/IncentiveCampaignsRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/IncentiveRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/InfraRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/KeyfiguresRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/LocationRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/interfaces/OccupationRepositoryProviderInterface.ts
(1 hunks)api/src/pdc/services/observatory/providers/FluxRepositoryProvider.ts
(4 hunks)api/src/pdc/services/observatory/providers/OccupationRepositoryProvider.ts
(1 hunks)
💤 Files with no reviewable changes (28)
- api/src/pdc/services/observatory/contracts/distribution/journeysByHours.schema.ts
- api/src/pdc/services/observatory/config/index.ts
- api/src/pdc/services/observatory/contracts/flux/getEvolFlux.schema.ts
- api/src/pdc/services/observatory/contracts/flux/getFlux.schema.ts
- api/src/pdc/services/observatory/contracts/occupation/getOccupation.schema.ts
- api/src/pdc/services/observatory/contracts/incentiveCampaigns/campaigns.contract.ts
- api/src/pdc/services/observatory/contracts/location/location.schema.ts
- api/src/pdc/services/observatory/contracts/occupation/getEvolOccupation.schema.ts
- api/src/pdc/services/observatory/contracts/incentiveCampaigns/campaigns.schema.ts
- api/src/pdc/services/observatory/config/ajv.ts
- api/src/pdc/services/observatory/contracts/occupation/getBestTerritories.schema.ts
- api/src/pdc/services/observatory/contracts/distribution/journeysByDistances.schema.ts
- api/src/pdc/services/observatory/contracts/flux/getBestFlux.schema.ts
- api/src/pdc/services/observatory/contracts/incentive/getIncentive.schema.ts
- api/src/pdc/services/observatory/contracts/infra/airesCovoiturage.contract.ts
- api/src/pdc/services/observatory/contracts/distribution/journeysByDistances.contract.ts
- api/src/pdc/services/observatory/contracts/occupation/getOccupation.contract.ts
- api/src/pdc/services/observatory/contracts/distribution/journeysByHours.contract.ts
- api/src/pdc/services/observatory/contracts/incentive/getIncentive.contract.ts
- api/src/pdc/services/observatory/contracts/flux/getFlux.contract.ts
- api/src/pdc/services/observatory/contracts/keyfigures/getKeyfigures.contract.ts
- api/src/pdc/services/observatory/contracts/keyfigures/getKeyfigures.schema.ts
- api/src/pdc/services/observatory/contracts/flux/getEvolFlux.contract.ts
- api/src/pdc/services/observatory/contracts/infra/airesCovoiturage.schema.ts
- api/src/pdc/services/observatory/contracts/location/location.contract.ts
- api/src/pdc/services/observatory/contracts/occupation/getBestTerritories.contract.ts
- api/src/pdc/services/observatory/contracts/flux/getBestFlux.contract.ts
- api/src/pdc/services/observatory/contracts/occupation/getEvolOccupation.contract.ts
🧰 Additional context used
🪛 Biome (1.9.4)
api/src/pdc/providers/superstruct/shared/index.ts
[error] 5-5: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead.
See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.
(lint/suspicious/noGlobalIsNan)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Test integration
- GitHub Check: CodeQL (javascript)
🔇 Additional comments (89)
api/src/pdc/services/observatory/actions/occupation/OccupationAction.ts (5)
1-1
: ConfirmFeature
reference usage.Ensure
Feature
from@/deps.ts
is indeed appropriate for representing thegeom
field. If it references a GeoJSON Feature, confirm that all downstream usage aligns with that specification for consistency.
5-5
: Validate import reusability.Bringing in the
Occupation
type here is a good approach. Confirm that any references to the old types or schemas for validation are fully removed to avoid confusion or dead code.
8-14
: Increase type detail if needed.The properties match the
Occupation
essentials, plusgeom
as aFeature
. Verify thatterritory: Occupation["code"]
andlibelle
fulfill all expected fields. In case additional properties (e.g., region, population details) are required in the future, extend this interface accordingly.
17-18
: Ensure router registration aligns with service name and method.Renaming the
service
to"observatory"
andmethod
to"getOccupation"
is consistent. Confirm that the route or call site in your codebase has been updated to match these values, preventing routing or invocation mismatches.
29-29
: Leverage explicit type guarantees in the repository.Switching the
handle
method to use theOccupation
type directly is a solid improvement. Ensure the repository’sgetOccupation
method returns the shape defined byResultInterface
to maintain type safety end-to-end.api/src/pdc/services/observatory/providers/OccupationRepositoryProvider.ts (1)
101-101
: ValidateindicParam
usage to avoid potential SQL injectionThe usage of
raw(indicParam)
here introduces a potential injection surface. Ensure thatcheckIndicParam
thoroughly restrictsindicParam
to one of the known safe indicators (e.g.,"journeys"
,"trips"
, etc.). Additionally, confirm that casting tofloat
matches the column type in the database to prevent runtime errors.api/src/pdc/services/observatory/interfaces/FluxRepositoryProviderInterface.ts (8)
3-4
: Consistent naming convention.Importing the
ResultInterface
asBestFluxResultInterface
aligns perfectly with the new naming scheme. No issues found here.
6-7
: Usage of EvolFluxResultInterface is clear.This import follows the same approach as the best flux structure, maintaining consistency across flux result interfaces.
8-8
: Good separation of concerns for FluxResultInterface.No issues: the naming is consistent with the rest of the codebase.
9-11
: Proper referencing of parameter interfaces.Importing
BestFluxParamsInterface
,EvolFluxParamsInterface
, andFluxParamsInterface
from the DTO folder is clear and aligns with the refactored architecture.
14-19
: Re-exporting the newly introduced types is convenient.This approach simplifies the import statements in other parts of the application. No issues found.
24-25
: Method signature is consistent with refactored DTO types.Updating to
getFlux(params: FluxParamsInterface) → Promise<FluxResultInterface>
accurately reflects the new type definitions.
27-28
: Method signature for evol flux is correct.Aligns well with the
EvolFluxParamsInterface
andEvolFluxResultInterface
naming pattern.
30-31
: Method signature for best flux is consistent.Properly updated to use
BestFluxParamsInterface
andBestFluxResultInterface
.api/src/pdc/services/observatory/actions/flux/EvolFluxAction.ts (3)
4-4
: Good import usage for the EvolFlux DTO.This import aligns well with the new DTO-based validation approach, ensuring structured parameters for the evol flux action.
17-18
: Middleware configuration looks consistent.Defining the service, method, and the validation middleware with
EvolFlux
is well-structured. Just ensure that any existing consumer or route references togetEvolFlux
are updated accordingly.Also applies to: 21-21
29-29
: Validate downstream usage of updated parameter type.Changing the parameter interface from
ParamsInterface
toEvolFlux
is appropriate for strongly typed validation. Verify that external or legacy references tohandle(params)
do not break.api/src/pdc/services/observatory/dto/flux/EvolFlux.ts (2)
1-2
: Well-organized imports.Using
superstruct
ensures a robust validation model. The extra re-exports from"@/pdc/providers/superstruct/shared/index.ts"
appear consistent with the broader data validation approach.
23-23
: Strong typed export.Exporting
type EvolFlux = Infer<typeof EvolFlux>
is a clean pattern, ensuring all references remain synchronized with definition updates.api/src/pdc/services/observatory/providers/FluxRepositoryProvider.ts (4)
7-11
: Interfaces ensure type consistency.These new interfaces (
BestFluxParamsInterface
,EvolFluxParamsInterface
, etc.) align well with the new validation approach. Confirm that any removed interfaces aren’t referenced elsewhere to avoid compilation errors.Also applies to: 14-14
24-26
: Method overloading approach is coherent.The
table
method’s flexible signature for different param interfaces is neat. Just ensure that the logic withingetTableName
gracefully handles all interface variations without introducing runtime confusion.
33-34
: Refined signature for getFlux.The update to
FluxParamsInterface
clarifies the expected data shape. Validate any references in custom analyses or scripts that consumed the oldGetFluxParamsInterface
.
80-81
: Correct alignment with EvolFlux changes.Switching to
EvolFluxParamsInterface
is consistent with the new DTO. Confirm that all usage references the new type and that no extraneous properties are expected in the request.api/src/pdc/services/observatory/actions/occupation/EvolOccupationAction.ts (3)
4-4
: Validate the new DTO import linkage
ImportingEvolOccupation
ensures typed usage in subsequent logic. Verify that the location"@/pdc/services/observatory/dto/occupation/EvolOccupation.ts"
is the correct source and that the DTO correctly reflects the domain requirements.
16-17
: Correct service and method naming
The addition ofservice: "observatory"
andmethod: "getEvolOccupation"
is well-aligned with the file’s purpose. No issues found.
20-20
: Ensure the validation schema matches domain constraints
Using theEvolOccupation
DTO within the"validate"
middleware is a good approach. Confirm thatEvolOccupation
includes all necessary validations (e.g., minimum/maximum constraints, required properties) to safeguard against invalid input.api/src/pdc/services/observatory/interfaces/KeyfiguresRepositoryProviderInterface.ts (1)
3-3
: Confirm usage across the codebase.The renamed
ResultInterface
toKeyfiguresResultInterface
aligns with the new location. However, ensure that all references toKeyfiguresResultInterface
throughout the codebase have been updated and remain consistent with the changes in the underlying action file.✅ Verification successful
Interface rename is properly handled across the codebase ✅
The renamed
KeyfiguresResultInterface
is consistently used across all its references in the codebase, with proper imports and type declarations in both the provider implementation and interface definition files.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all references to the renamed KeyfiguresResultInterface rg "KeyfiguresResultInterface" -A 2Length of output: 2201
api/src/pdc/services/observatory/dto/KeyFigures.ts (2)
1-10
: Imports look consistent and properly scoped.
Everything in these import statements seems correct, and the usage ofoptional
fromsuperstruct
is appropriate.
12-20
: Consider validating mutually exclusive fields or providing defaults where needed.
The approach of markingmonth
,trimester
,semester
, anddirection
as optional fields is valid, but if your domain logic requires exactly one or some combination at any given time, consider adding corresponding validation checks or a default value for clarity.api/src/pdc/services/observatory/actions/keyfigures/KeyfiguresAction.ts (4)
3-6
: New imports are appropriately referencing superstruct and KeyFigures.
No issues are apparent, and the references align well with your updated DTO structure.
8-21
: Verify the shape of your result and consider boolean usage for has_incentive.
TheResultInterface
references some fields fromKeyFigures
and uses numeric fields for others (e.g.,has_incentive
). If your domain logic treats incentives as a boolean, consider changing the type to a boolean to reflect intent more clearly. Otherwise, leaving this as a numeric field (e.g., a bit indicator) is acceptable if well-documented.
24-25
: Handler config is clear and uses the correct middlewares.
The service name, method, and permission-based middleware align well with your updated approach. No action needed.Also applies to: 28-28
36-36
: Wrap repository calls in error handling if needed.
While this is functionally correct, confirm whether the framework handles repository errors or exceptions. If not, consider a try/catch block to handle repository-level errors gracefully.api/src/pdc/services/observatory/interfaces/IncentiveCampaignsRepositoryProviderInterface.ts (1)
1-2
: Verify consistency of newly introduced imports.Good job using
import type
to optimize bundling. Ensure that any remaining references to the now-removedcampaigns.contract.ts
have been replaced. This helps avoid confusion and prevents unintentional usage of outdated definitions.To confirm no leftover imports of
campaigns.contract.ts
exist, run:✅ Verification successful
Import changes are consistent and complete
The verification confirms that all references to
campaigns.contract.ts
have been successfully removed, and the new import sources exist in their correct locations. The migration is complete and consistent.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash rg 'campaigns.contract'Length of output: 23
Script:
#!/bin/bash # Check if the new import sources exist echo "Checking new import sources:" fd "CampaignsAction.ts|IncentiveCampaigns.ts" echo -e "\nChecking for any variations of campaigns contract:" rg -i "contract.*campaigns|campaigns.*contract" echo -e "\nVerifying the usage of imported types:" ast-grep --pattern 'type { $_ as CampaignsResultInterface }' ast-grep --pattern 'type { $_ as CampaignsParamsInterface }'Length of output: 752
api/src/pdc/services/observatory/dto/IncentiveCampaigns.ts (1)
1-2
: Good use of superstruct imports.
These imports look correct for defining and inferring structured types with thesuperstruct
library.api/src/pdc/services/observatory/actions/incentiveCampaigns/CampaignsAction.ts (2)
1-5
: Imports appear valid and cohesive.
The newly added imports forFeature
andIncentiveCampaigns
correctly introduce the needed dependencies.
38-42
: Middleware validation switch is aligned with new DTO.
Switching toIncentiveCampaigns
for validation in the@handler
decorator matches the updated DTO usage.api/src/pdc/services/observatory/dto/flux/Flux.ts (1)
11-19
: Clarify identical field types.
Currently, bothtype
andobserve
are typed asTerritoryType
. Verify thatobserve
indeed needs the same type constraints astype
, as it might cause confusion or erroneous usage if these fields serve different semantic roles. If they are truly the same domain concept, consider documenting the reason for that duplication.api/src/pdc/services/observatory/actions/flux/FluxAction.ts (2)
19-20
: Kudos on explicit service and method naming.
Definingservice: "observatory"
andmethod: "getFlux"
clarifies your intent and fosters maintainability. This approach is consistent with the rest of your codebase.
4-4
: Verify correlation with DTO.
ImportingFlux
fromdto/flux/Flux.ts
is consistent with the new validators. Just ensure that no references to the old contracts or schema files remain elsewhere in the codebase.✅ Verification successful
Import verified as correct, old references cleared
The import ofFlux
from the new DTO location is consistent with the codebase structure, and no active references to the old contract or schema files remain in use. While the old files still exist in theshared/observatory/flux/
directory, they are not imported anywhere in the active codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Searching for legacy references to old flux contract or schema. rg "getFlux\.contract" -A 5 rg "getFlux\.schema" -A 5Length of output: 57
Script:
#!/bin/bash # Search for flux-related files and their content echo "=== Flux-related files ===" fd -e ts -e js flux -I --type f echo -e "\n=== Searching for schema/contract patterns ===" rg -i "flux.*schema|flux.*contract|flux.*validate" -g '*.{ts,js}' echo -e "\n=== Checking Flux-related imports ===" rg "^import.*['\"].*flux.*['\"]" -g '*.{ts,js}'Length of output: 2983
api/src/pdc/services/observatory/interfaces/LocationRepositoryProviderInterface.ts (1)
3-4
: Ensure backward compatibility with old location contract references.These new imports replace references to the old contract-based definitions. Please verify that all consumer code within the codebase has transitioned to the new DTO format and that no references to
location.contract.ts
remain. If any remain, consider providing migration or fallback logic to ensure a smooth transition and backward compatibility.Below is a script that searches for any lingering references to
location.contract.ts
:✅ Verification successful
No backward compatibility concerns found
The codebase has fully transitioned to the new DTO format, with no remaining references to
location.contract.ts
. The old contract file exists but is unused, suggesting it can be safely removed.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to the old location contract rg "location\.contract"Length of output: 25
Script:
#!/bin/bash # Comprehensive search for location contract references echo "=== Searching for location contract file ===" fd -e ts -e js "location\.contract" echo -e "\n=== Searching for imports containing location contract ===" rg -i "import.*location.*contract" echo -e "\n=== Searching for any mention of LocationContract ===" rg -i "LocationContract" echo -e "\n=== Checking all Location type imports ===" rg "import.*Location.*from"Length of output: 1692
Script:
#!/bin/bash # Check the content of the contract file echo "=== Content of location.contract.ts ===" cat shared/observatory/location/location.contract.ts echo -e "\n=== Check if contract file is referenced without import statements ===" rg -i "location\.contract" -g "!*.md"Length of output: 1055
api/src/pdc/services/observatory/interfaces/OccupationRepositoryProviderInterface.ts (2)
4-6
: Confirm that all references to the old contract import paths are fully replaced.Switching from
contracts
todto
for parameter interfaces is a good step toward cleaner architecture. Make sure all files referencing them are updated to this new path and naming, especially if there are test files, service files, or older modules still reliant on thecontracts
directory.Consider running this script to ensure there are no leftover references:
✅ Verification successful
Migration from contracts to dto is complete and correct ✅
The codebase shows no references to the old contract imports, and all relevant files are properly using the new DTO imports. The contract files found in the shared directory are for different functionality (monthly operations) and are unrelated to this migration.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify references to old contract-based parameter imports. rg "getBestTerritories.contract|getEvolOccupation.contract|getOccupation.contract" -A 5Length of output: 89
Script:
#!/bin/bash # Search for any remaining imports from contracts directory echo "=== Checking for remaining contract imports ===" rg "from.*contracts.*occupation" -g "*.ts" echo -e "\n=== Checking for interface names in old locations ===" # Search for interface names that might still be in contracts rg "BestTerritories|EvolOccupation|Occupation" -g "*contract*.ts" echo -e "\n=== Verifying new DTO imports ===" # Verify the new imports are being used rg "from.*dto.*occupation" -g "*.ts"Length of output: 1995
1-3
: Ensure consistent usage of newly aliased result interfaces throughout the codebase.These lines correctly replace the old
contracts
imports with the newactions/occupation
ones. As a follow-up, please verify that no references to the old contract files remain in other parts of the codebase to avoid confusion or broken imports.You can run this script to confirm that all relevant references have been updated correctly:
✅ Verification successful
No other files need to be updated - this is a localized change.
The new imports in
OccupationRepositoryProviderInterface.ts
are correctly aliased and localized to this interface file. A thorough search of the codebase shows no other files were importing or using these occupation-related interfaces, so no additional updates are required.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for references to the old contract-based imports for best, evol, and standard occupation results. rg "getBestTerritories.contract|getEvolOccupation.contract|getOccupation.contract" -A 5Length of output: 89
Script:
#!/bin/bash # Search for any remaining contract-related imports and new action paths echo "=== Searching for any contract-related imports ===" rg -g "*.ts" "import.*contract" -A 2 echo -e "\n=== Checking usage of new action paths ===" rg -g "*.ts" "from.*observatory/actions/occupation/(BestTerritoriesAction|EvolOccupationAction|OccupationAction)" -A 2 echo -e "\n=== Looking for potential partial imports ===" rg -g "*.ts" "from.*observatory/(contracts|actions).*Occupation" -A 2Length of output: 66512
api/src/pdc/services/observatory/interfaces/IncentiveRepositoryProviderInterface.ts (1)
1-2
: Verify removal of references to the old contract file.If the old
getIncentive.contract.ts
file was removed, confirm that no code references it anymore.✅ Verification successful
All references to the old contract file have been successfully removed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any lingering references to the old contract file. rg -A 5 'getIncentive\.contract\.ts'Length of output: 38
Script:
#!/bin/bash # Try multiple search patterns to be thorough echo "=== Searching for any getIncentive contract references ===" rg -i "getincentive.*contract" echo -e "\n=== Looking for contract files in observatory service ===" fd "contract" api/src/pdc/services/observatory echo -e "\n=== Checking for getIncentive references ===" rg "getIncentive" api/src/pdc/services/observatoryLength of output: 1010
api/src/pdc/services/observatory/dto/Incentive.ts (3)
1-10
: Imports are consistent with superstruct usage.The imports neatly gather all relevant types from the shared folder, facilitating type-safe validation.
12-20
: Optional property alignment.
direction
is marked as optional here. However, verify that any downstream usage or result interface properly acknowledges thatdirection
may be absent.
22-22
: Neat approach with inferred type.Using
Infer<typeof Incentive>
directly provides a convenient TS interface without duplicating the schema. This is a best practice for strongly typed contracts.api/src/pdc/services/observatory/actions/incentive/IncentiveAction.ts (3)
3-6
: Imports reflect new structure usage.These imports align well with the new
Incentive
definition and utilize the superstructInfer
type for consistent typing.
19-23
: Middleware usage looks good.Applying
hasPermissionMiddleware
and validating the parameters asIncentive
ensures robust type safety and access control.
9-16
: Potential mismatch in optional field.
direction
is required here asInfer<typeof Direction>
but set as optional in theIncentive
object. This may lead to undefined errors if not handled.If
direction
is truly optional, consider marking it optional here:export type ResultInterface = { code: Incentive["code"]; libelle: string; - direction: Infer<typeof Direction>; + direction?: Infer<typeof Direction>; collectivite: number; operateur: number; autres: number; }[];✅ Verification successful
Potential mismatch in optional field confirmed.
The review comment is correct. The
direction
field is optional in theIncentive
type (defined indto/Incentive.ts
) but required inResultInterface
. This mismatch could lead to undefined errors if not handled properly. The suggested fix to makedirection
optional inResultInterface
is appropriate.🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find Incentive type definition echo "=== Searching for Incentive type definition ===" ast-grep --pattern 'type Incentive = {$$$}' echo -e "\n=== Searching for Incentive interface definition ===" ast-grep --pattern 'interface Incentive {$$$}' echo -e "\n=== Searching for ResultInterface usages ===" rg "ResultInterface" -A 5 -B 5Length of output: 66641
Script:
#!/bin/bash # Search for files containing both Incentive and direction echo "=== Searching for files containing both Incentive and direction ===" rg "Incentive.*direction|direction.*Incentive" -A 5 -B 5 echo -e "\n=== Searching for type/interface definitions with direction field ===" ast-grep --pattern 'type Incentive = { $$$ direction$$$ $$$ }' ast-grep --pattern 'interface Incentive { $$$ direction$$$ $$$ }'Length of output: 503
Script:
#!/bin/bash # View the full content of IncentiveAction.ts echo "=== Content of IncentiveAction.ts ===" cat api/src/pdc/services/observatory/actions/incentive/IncentiveAction.ts echo -e "\n=== Searching for Direction type definition ===" rg "type Direction|interface Direction|enum Direction" -A 5 -B 5Length of output: 3449
Script:
#!/bin/bash echo "=== Content of Incentive.ts ===" cat api/src/pdc/services/observatory/dto/Incentive.tsLength of output: 625
api/src/pdc/services/observatory/ObservatoryServiceProvider.ts (3)
5-5
: Retain ValidatorMiddleware only if necessary.The
ValidatorMiddleware
import is included and registered in the middlewares array. Check that it aligns with your refactored validation approach (e.g., if you’ve deprecated certain validations). Consider removing this middleware if it is no longer needed.
61-61
: Empty class is acceptable if minimalism is intended.Ensuring the
ObservatoryServiceProvider
class is intentionally empty is fine if the@serviceProvider
decorator handles all required configuration. Just confirm that there's no additional setup or teardown logic you want to implement.
2-2
: Confirm path correctness for imports.All the imported modules appear to be referencing their correct absolute paths. However, please ensure that these aliases (e.g., "@/ilos/…") are correctly set up in the project’s configuration (tsconfig paths or similar) so that they resolve properly at build time.
To confirm, run the following script to locate any broken imports or references:
✅ Verification successful
Import path is correctly configured
The import path
@/ilos/common/index.ts
is valid and properly configured. This is a Deno project (confirmed by deno.json in api/ directory) where this import alias pattern is used consistently across the codebase. The same import pattern is successfully used in 100+ files, indicating it's a well-established and working configuration.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify validity of the import paths used in this file. rg -A 2 -B 2 'from "@/' .Length of output: 102613
Script:
#!/bin/bash # Search for TypeScript config files that might contain path mappings echo "=== TypeScript Config Files ===" fd tsconfig.json echo -e "\n=== Contents of TypeScript Config Files ===" fd tsconfig.json -x cat {}Length of output: 4306
Script:
#!/bin/bash # Search for various config files in the api directory echo "=== Config Files in api/ ===" cd api 2>/dev/null && find . -maxdepth 2 -type f -name "*.json" -o -name "*.jsonc" || echo "No api directory found" echo -e "\n=== Contents of Found Config Files ===" cd api 2>/dev/null && find . -maxdepth 2 -type f -name "*.json" -o -name "*.jsonc" -exec cat {} \; || echo "No api directory found"Length of output: 307
api/src/pdc/services/observatory/actions/flux/BestFluxAction.ts (2)
4-4
: Good import usage.
ImportingBestFlux
from its DTO location aligns with the new validation approach.
15-19
: Validation middleware update seems correct.
UsingBestFlux
in the"validate"
middleware ensures type-checked parameters with superstruct. Make sure that all required fields are present and that defaults/optionals are properly handled in the DTO.api/src/pdc/services/observatory/dto/infra/AiresCovoiturage.ts (2)
1-2
: Imports are appropriate.
The imports fromsuperstruct
and the shared territory types align with the new DTO structure.
4-9
: DTO definition is clear and minimal.
DefiningAiresCovoiturage
with type and code ensures consistent validation. If there are hidden constraints or required fields, consider adding them to the object schema. Otherwise, this straightforward approach is acceptable.api/src/pdc/services/observatory/dto/distribution/JourneysByHours.ts (3)
1-2
: Imports conform to the superstruct-based pattern.
This aligns well with the refactored validation approach seen in other files.
12-20
: Schema design effectively captures optional fields.
Markingmonth
,trimester
,semester
, anddirection
as optional makes sense if not all data requires them. Ensure consumers handle these possibly undefined fields.
22-22
: Exporting inferred type.
Deriving the TypeScript type from the schema ensures consistency between runtime and compile-time checks. Good practice.api/src/pdc/services/observatory/dto/distribution/JourneysByDistances.ts (2)
22-22
: Appreciation for clean, well-structured type inference.Using
Infer<typeof JourneysByDistances>
fosters type safety and clarity, ensuring that all consumers of theJourneysByDistances
schema align with the actual structure. Good job!
1-10
: Check import paths and shared modules usage.These imports utilize path aliases like
@/lib/superstruct
and@/pdc/providers/superstruct/shared
. Make sure they are correctly mapped in the project's TypeScript configuration (paths
in tsconfig) and that all modules are properly exported from the shared index file.✅ Verification successful
Import paths and shared modules are correctly configured
The imports are valid as both source files exist and export the required symbols:
@/lib/superstruct/index.ts
->api/src/lib/superstruct/index.ts
@/pdc/providers/superstruct/shared/index.ts
->api/src/pdc/providers/superstruct/shared/index.ts
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the import paths resolve properly ast-grep --pattern 'import $_ from "@/lib/superstruct$_"' ast-grep --pattern 'import $_ from "@/pdc/providers/superstruct$_"'Length of output: 53693
Script:
#!/bin/bash # Find and check tsconfig.json for path mappings echo "=== Checking tsconfig.json for path mappings ===" fd tsconfig.json echo -e "\n=== Checking for superstruct files ===" # Look for potential superstruct files fd -t f -e ts superstruct echo -e "\n=== Checking for specific imports ===" # Search for specific imported symbols rg -l "export.*\b(Direction|Month|Semester|TerritoryCode|TerritoryType|Trimester|Year)\b"Length of output: 857
Script:
#!/bin/bash # Check api tsconfig.json echo "=== Checking api/tsconfig.json ===" fd tsconfig.json api/ echo -e "\n=== Checking exports from shared index ===" cat api/src/pdc/providers/superstruct/shared/index.ts echo -e "\n=== Checking lib/superstruct location ===" fd -t f index.ts api/src/lib/superstructLength of output: 2319
api/src/pdc/services/observatory/dto/Location.ts (2)
22-22
: Straightforward type export pattern.Exporting both the struct and its inferred type side by side is a neat way to keep the schema and TypeScript definitions in sync. Looks good!
1-10
: Ensurezoom
aligns with front-end usage.The
zoom
property is constrained from 0 to 8, with a default of 5. Confirm that any front-end components or dashboards are prepared to handle values outside this default range. Additionally, verify that theCoerceNumberMinMax
function correctly handles edge cases (e.g., negative or large values).✅ Verification successful
Zoom property usage aligns with constraints
The zoom property is consistently used within the defined range (0-8) across the codebase. Front-end components use appropriate zoom levels (default: 5, max: 8) that match the DTO constraints.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search references to the `Location` type or `zoom` property to ensure consistent usage. rg -A 3 "Location" rg -A 3 "zoom"Length of output: 22262
api/src/pdc/services/observatory/dto/occupation/BestTerritories.ts (2)
1-10
: Maintain uniform type naming across files.All these new DTO objects (e.g.,
Location
,BestTerritories
, etc.) follow a consistent naming pattern, which is excellent. Keep this convention in mind for future additions to maintain clarity in your codebase.
23-23
: Validate usage and references to newBestTerritories
type.Ensure that any older references to a previous
ParamsInterface
are updated to the newBestTerritories
structure, and that consumers handle thelimit
property correctly.✅ Verification successful
All references to
BestTerritories
type are consistent and properly handled.
- Type is correctly used across backend services and frontend components
- No legacy
ParamsInterface
found in the observatory context- The
limit
property is properly handled with a value of 10 in the Dashboard component🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Locate references to BestTerritories or the older interface it replaces rg -A 5 "BestTerritories" rg -A 5 "ParamsInterface"Length of output: 66035
api/src/pdc/services/observatory/dto/flux/BestFlux.ts (2)
24-24
: Well-defined type inference.Defining
BestFlux
withInfer
ensures consistent type checking across the codebase. This is a great approach to keep your domain models type-safe.
1-10
: Confirm consistent domain usage for numeric fields.The
limit
property ranges from 5 to 100 by default. Ensure that this domain limit matches the actual usage scenario in your application and that error-handling for out-of-range values remains consistent and user-friendly.✅ Verification successful
Limit property range is correctly defined and consistently used
The limit range of 5-100 is properly enforced through schema validation and consistently used across the observatory flux components. The range is appropriate for displaying "top N" results in tables, with UI components using reasonable defaults (e.g., top 10). Error handling is built into the schema validation.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check references to the `limit` field to ensure correct usage. rg -A 3 "limit"Length of output: 88821
api/src/pdc/services/observatory/dto/occupation/EvolOccupation.ts (2)
1-2
: Imports are neatly organized
The references tosuperstruct
and shared definitions are clearly laid out, ensuring consistent usage of typed validators throughout the project.
16-18
: Optional fields are well-defined
Markingmonth
,trimester
, andsemester
as optional is a good way to handle partial data—even if some timeframe elements are missing, the structure remains valid.api/src/pdc/services/observatory/interfaces/InfraRepositoryProviderInterface.ts (1)
1-2
: Interface usage looks correct
The updated imports forAiresCovoiturageParamsInterface
andAiresCovoiturageResultInterface
align with the new DTO-based approach. Ensure these changes are reflected in dependent repository implementations.api/src/pdc/services/observatory/actions/location/LocationAction.ts (2)
12-16
: Handler configuration is concise
Using theobservatory
service with methodgetLocation
and validating against theLocation
DTO is coherent with the new refactoring approach.
24-24
: Properly typed method signature
SwappingParamsInterface
withLocation
leverages the new DTO for validation. Ensure tests are updated accordingly to match the new parameter structure.api/src/pdc/services/observatory/actions/occupation/BestTerritoriesAction.ts (3)
6-10
: Robust result typing
DefiningResultInterface
to referenceBestTerritories["code"]
ensures consistency with the input type—helpful for preventing mismatches in territory codes.
13-17
: Handler metadata is aligned
Theservice: "observatory"
andmethod: "getBestTerritories"
combination, alongside validatingBestTerritories
, follows the same structured approach used in other actions. This creates a uniform pattern across the observatory module.
25-25
: DTO-based parameter usage
ReplacingParamsInterface
withBestTerritories
is a major step toward improved clarity and type safety. Confirm that the repository methodgetBestTerritories
is thoroughly tested with various valid and invalid inputs.api/src/pdc/services/observatory/actions/infra/AiresCovoiturageAction.ts (4)
1-1
: Import ofFeature
from@/deps.ts
looks good
The usage ofFeature
provides a convenient GeoJSON feature type.
5-5
: Dependency onAiresCovoiturage
DTO is consistent with new refactoring
The switch to a DTO-based approach aligns well with your recent structural changes.
23-24
: New @handler configuration aligns with the updated service name
Switching from the spread-based config to explicit fields forservice
andmethod
is consistent and improves clarity.Also applies to: 27-27
35-35
: Verify repository return type
Ensure thatthis.repository.getAiresCovoiturage(params)
always returns an array matchingResultInterface
.✅ Verification successful
Return type is correctly implemented
The repository methodgetAiresCovoiturage
returns an array of rows that exactly matches theResultInterface
definition, with all required fields and correct types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the getAiresCovoiturage repository method usage and definition rg 'getAiresCovoiturage'Length of output: 464
Script:
#!/bin/bash # Get the interface definition, implementation and ResultInterface echo "=== Interface Definition ===" ast-grep --pattern 'getAiresCovoiturage($_) : $_' api/src/pdc/services/observatory/interfaces/InfraRepositoryProviderInterface.ts echo -e "\n=== Implementation ===" ast-grep --pattern 'async getAiresCovoiturage($_) { $$$ }' api/src/pdc/services/observatory/providers/InfraRepositoryProvider.ts echo -e "\n=== ResultInterface ===" rg -A 5 "interface ResultInterface"Length of output: 28930
Script:
#!/bin/bash # Search for AiresCovoiturage-related files and definitions echo "=== AiresCovoiturage Files ===" rg -l "AiresCovoiturage" echo -e "\n=== AiresCovoiturage Type Definitions ===" rg "interface.*AiresCovoiturage" -A 5 echo -e "\n=== getAiresCovoiturage Implementation ===" rg -A 10 "getAiresCovoiturage.*\{" api/src/pdc/services/observatory/providers/InfraRepositoryProvider.tsLength of output: 1630
Script:
#!/bin/bash echo "=== DTO Definition ===" cat api/src/pdc/services/observatory/dto/infra/AiresCovoiturage.ts echo -e "\n=== Interface Contract ===" cat api/src/pdc/services/observatory/interfaces/InfraRepositoryProviderInterface.ts echo -e "\n=== Implementation ===" cat api/src/pdc/services/observatory/providers/InfraRepositoryProvider.tsLength of output: 3102
Script:
#!/bin/bash echo "=== ResultInterface Definition ===" cat api/src/pdc/services/observatory/actions/infra/AiresCovoiturageAction.tsLength of output: 1321
api/src/pdc/services/observatory/actions/distribution/JourneysByHoursAction.ts (3)
4-6
: Usage ofDirection
andJourneysByHours
from superstruct
Importing these types from your shared index is a solid approach to keep your code DRY and consistent.
21-22
: Clear definition forservice: "observatory"
andmethod: "journeysByHours"
Using explicit service/method naming and referencing the new DTOJourneysByHours
clarifies the action’s responsibility.Also applies to: 25-25
33-33
: Verify repository shape consistency
Double-check thatthis.repository.getJourneysByHours(params)
returns an array matching the newly definedResultInterface
.✅ Verification successful
Repository shape is consistent ✅
The repository'sgetJourneysByHours
method correctly returns theResultInterface
type defined in the action, ensuring type safety and consistency across the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Confirm getJourneysByHours usage rg 'getJourneysByHours'Length of output: 486
Script:
#!/bin/bash # Get ResultInterface definition and related implementations ast-grep --pattern 'interface ResultInterface { $$$ }' echo "=== Repository Interface ===" ast-grep --pattern 'interface DistributionRepositoryProviderInterface { $$$ getJourneysByHours($_): $_ $$$ }' echo "=== Repository Implementation ===" ast-grep --pattern 'async getJourneysByHours($_): $_ { $$$ }'Length of output: 10655
Script:
#!/bin/bash # Search for ResultInterface in observatory service echo "=== Observatory ResultInterface ===" rg -A 5 "ResultInterface" api/src/pdc/services/observatory/ echo "=== JourneysByHours Implementation ===" rg -A 10 "getJourneysByHours" api/src/pdc/services/observatory/providers/DistributionRepositoryProvider.ts echo "=== Interface Definition ===" rg -A 5 "getJourneysByHours" api/src/pdc/services/observatory/interfaces/DistributionRepositoryProviderInterface.tsLength of output: 56454
api/src/pdc/services/observatory/actions/distribution/JourneysByDistancesAction.ts (3)
3-3
: Superstruct-based imports look appropriate
ImportingInfer
and theJourneysByDistances
DTO is consistent with the broader refactoring effort.Also applies to: 5-6
19-20
: Explicit @handler config is concise and clear
No issues noted. The updated references to "observatory" are self-explanatory.Also applies to: 23-23
31-31
: Confirm return value fromgetJourneysByDistances
Check that the repository method strictly returns the structure described inResultInterface
.✅ Verification successful
Return value structure verified
The repository method strictly returns the structure defined inResultInterface
: an array of objects containing code, libelle, direction, and distances array with dist_classes and journeys counts.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify usage of getJourneysByDistances rg 'getJourneysByDistances'Length of output: 510
Script:
#!/bin/bash # Find ResultInterface definition and examine implementation echo "=== ResultInterface definition ===" ast-grep --pattern 'interface ResultInterface' echo -e "\n=== getJourneysByDistances implementation ===" ast-grep --pattern 'async getJourneysByDistances($_) { $$$ }' echo -e "\n=== Interface method signature ===" ast-grep --pattern 'getJourneysByDistances($_): $_'Length of output: 439
Script:
#!/bin/bash # Search for interface definitions and implementations with context echo "=== ResultInterface definition ===" rg -A 10 "interface ResultInterface" echo -e "\n=== Repository interface definition ===" rg -A 10 "interface DistributionRepositoryProviderInterface" echo -e "\n=== Implementation with context ===" rg -A 10 "class DistributionRepositoryProvider"Length of output: 47087
Script:
#!/bin/bash # Search for JourneysByDistances related code echo "=== JourneysByDistances interface and types ===" rg -A 10 "JourneysByDistances(Interface)?" echo -e "\n=== getJourneysByDistances implementation ===" rg -A 15 "async getJourneysByDistances"Length of output: 18275
api/src/pdc/services/observatory/interfaces/DistributionRepositoryProviderInterface.ts (1)
3-3
: Updated references forJourneysByDistancesResultInterface
andJourneysByHoursResultInterface
The newly added imports reference the updated action files. Everything looks aligned with the new approach.Also applies to: 6-6
api/src/pdc/providers/superstruct/shared/index.ts (2)
1-1
: Imports look good.
All required modules fromsuperstruct
are properly imported.
43-44
: Territory validation approach looks consistent.
- Union of codes (
TerritoryCode
) aligns well with existing typed validations.TerritoryType
enumerations cover key territory segments.
export const BestTerritories = object({ | ||
year: Year, | ||
type: TerritoryType, | ||
code: TerritoryCode, | ||
observe: TerritoryType, | ||
month: optional(Month), | ||
trimester: optional(Trimester), | ||
semester: optional(Semester), | ||
limit: defaulted(CoerceNumberMinMax(integer(), 5, 100), 10), | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Assess necessity of observe
as a separate property.
observe: TerritoryType
looks redundant if type
also references a territory classification. If they fulfill different roles, consider clarifying naming or adding explanatory comments to guide future maintainers.
export type ResultInterface = { | ||
id_lieu: string; | ||
nom_lieu: string; | ||
com_lieu: string; | ||
type: string; | ||
date_maj: Date; | ||
nbre_pl: number; | ||
nbre_pmr: number; | ||
duree: number; | ||
horaires: string; | ||
proprio: string; | ||
lumiere: boolean; | ||
geom: Feature; | ||
}[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Make fields optional in ResultInterface
to match database schema
The interface should be updated to make fields optional since the database schema allows NULL values for all fields except id
. Additionally, several fields undergo type conversion:
date_maj?: Date
nbre_pl?: number
nbre_pmr?: number
duree?: number
horaires?: string
proprio?: string
lumiere?: boolean
geom?: Feature
🔗 Analysis chain
Confirm optional vs. required fields in ResultInterface
All fields are declared as required. If any of these fields might be optional (e.g., date_maj
or geom
in edge cases), consider making them optional in the type or validating them more thoroughly.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Attempt to locate usage of ResultInterface properties
# to confirm if they're always populated or sometimes undefined/null.
ast-grep --pattern $'geom: $_'
rg 'date_maj'
Length of output: 1444
Script:
#!/bin/bash
# Search for database schema and null checks
rg -A 5 "CREATE TABLE.*aires" api/src/db/
rg -A 2 "\?\." api/src/pdc/services/observatory/
rg "null.*undefined" api/src/pdc/services/observatory/
# Search for other fields to see their usage
ast-grep --pattern 'nbre_pl: $_'
ast-grep --pattern 'nbre_pmr: $_'
ast-grep --pattern 'duree: $_'
ast-grep --pattern 'horaires: $_'
ast-grep --pattern 'proprio: $_'
ast-grep --pattern 'lumiere: $_'
Length of output: 3849
Script:
#!/bin/bash
# Get complete table definition
rg -A 20 "CREATE TABLE.*aires_covoiturage" api/src/db/
# Find data transformation code
rg -A 5 "ResultInterface|AiresCovoiturage" api/src/pdc/services/observatory/
rg -A 5 "map.*aires" api/src/pdc/services/observatory/
Length of output: 64067
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
api/src/pdc/providers/superstruct/shared/index.ts (2)
2-13
: Add input validation for min/max parameters.The
CoerceNumberMinMax
function implementation looks good, but consider adding validation for themin
andmax
parameters to ensure they form a valid range.export const CoerceNumberMinMax = (type: Struct<number, null>, min: number, max: number) => { + if (min > max) { + throw new Error(`Invalid range: min (${min}) cannot be greater than max (${max})`); + } return coerce(size(type, min, max), string(), (v) => {
46-47
: Add JSDoc comments for territory types.Consider adding documentation to explain the territory hierarchy and provide examples:
+/** + * Represents valid territory code formats including Country, Department, Insee, and Siren codes. + * @example + * // Country code: "12345" + * // Department code: "01", "2A" + * // Insee code: "01234" + * // Siren code: "123456789" + */ export const TerritoryCode = union([Country, Department, Insee, Siren]); + +/** + * Represents different levels of territorial administration in France. + * - com: Commune + * - epci: Établissement public de coopération intercommunale + * - aom: Autorité organisatrice de la mobilité + * - dep: Département + * - reg: Région + * - country: Pays + */ export const TerritoryType = enums(["com", "epci", "aom", "dep", "reg", "country"]);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
api/src/pdc/providers/superstruct/shared/index.ts
(2 hunks)
🔇 Additional comments (2)
api/src/pdc/providers/superstruct/shared/index.ts (2)
37-41
: Consider implications of dynamic Year validation.The
Year
validator usesnew Date().getFullYear()
which could lead to inconsistent validation behavior:
- Different servers might have different system times
- Validation rules could change as time passes
Consider using a fixed max year or implementing a centralized time provider.
For example:
-export const Year = CoerceNumberMinMax(integer(), 2020, new Date().getFullYear()); +// Define max year explicitly or use a centralized time provider +const MAX_YEAR = 2025; // Update periodically +export const Year = CoerceNumberMinMax(integer(), 2020, MAX_YEAR);
42-45
: Verify territory code patterns against official documentation.The regular expressions for territory codes appear to follow French administrative patterns, but let's verify them against official documentation:
- Country:
^[0-9X]{5}$
- Department:
^[0-9][0-9A-B]{1}[0-9]{0,1}$
- Insee:
^[0-9][0-9A-B][0-9]{3}$
- Siren:
^[0-9]{9}$
✅ Verification successful
Territory code patterns are correctly implemented for French administrative system
The patterns are specifically designed for French administrative territory management:
- Country pattern using 5 chars (digits + X) is intentional, with "XXXXX" representing France in the system
- Department, INSEE, and SIREN patterns correctly follow official French administrative code formats
- Patterns are validated and used consistently throughout the codebase with French geo APIs
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for existing territory code validation patterns and documentation # Look for related validation patterns in the codebase echo "Searching for existing territory code validation patterns..." rg -l "(?:\b(?:Country|Department|Insee|Siren)\b.*?pattern.*?/\^.*?\$/)" --type ts # Look for documentation files that might contain official patterns echo "Searching for documentation files..." fd -e md -e txt -e yaml "territory|codes|patterns|validation" api/specs/Length of output: 372
Script:
#!/bin/bash # Search for pattern definitions with context echo "=== Pattern definitions with context ===" rg -B 2 -A 2 "(?:Country|Department|Insee|Siren)\s*=\s*pattern" --type ts # Look for test files echo -e "\n=== Related test files ===" fd -e test.ts -e spec.ts . | xargs rg -l "Country|Department|Insee|Siren" # Search for documentation comments near patterns echo -e "\n=== Documentation comments ===" rg -B 5 "(?:Country|Department|Insee|Siren)\s*=\s*pattern" --type tsLength of output: 2756
Script:
#!/bin/bash # Check test files for pattern validation examples echo "=== Test file contents ===" rg -A 5 -B 5 "(?:Country|Department|Insee|Siren)" api/src/pdc/providers/geo/tests/*.spec.ts # Check GeoProvider implementation echo -e "\n=== GeoProvider implementation ===" cat api/src/pdc/providers/geo/GeoProvider.ts # Look for any documentation about these codes echo -e "\n=== Related documentation ===" fd README -e md | xargs rg -l "territory|code|insee|siren"Length of output: 8729
Script:
#!/bin/bash # Check README contents echo "=== README contents ===" cat README.md api/src/db/README.md notebooks/README.md # Look for any country code usage echo -e "\n=== Country code usage ===" rg -B 3 -A 3 "Country.*=.*pattern" --type ts rg -B 3 -A 3 "country" --type ts api/src/pdc/Length of output: 66531
#fixes (issues)
Description
Checklist
/api/specs
Merge
Je squash la PR et vérifie que le message de commit utilise la convention d'Angular :
Voir la convention de message de commit...
Types de commit
Le scope (optionnel) précise le module ou le composant impacté par le commit.
Summary by CodeRabbit
Based on the comprehensive summary, here are the updated release notes:
New Features
Improvements
Technical Changes
Performance