-
Notifications
You must be signed in to change notification settings - Fork 61
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(signature-collection): Admin area summary report endpoint #16230
Conversation
WalkthroughThis pull request introduces a new input type class, Changes
Possibly related PRs
Suggested reviewers
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
Documentation and Community
|
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: 1
🧹 Outside diff range and nitpick comments (5)
libs/api/domains/signature-collection/src/lib/dto/areaSummaryReport.input.ts (1)
6-13
: LGTM: Properties are well-defined with appropriate decorators.The
areaId
andcollectionId
properties are correctly decorated with@Field()
for GraphQL schema exposure and@IsString()
for type validation. The use of TypeScript's definite assignment assertion (!
) clearly indicates that these fields are required.Consider adding a comment or using a more explicit decorator like
@Field({ nullable: false })
to make it clearer that these fields are required in the GraphQL schema. This would improve code readability and self-documentation.libs/api/domains/signature-collection/src/lib/models/areaSummaryReport.model.ts (2)
4-8
: LGTM: Well-structured class with proper GraphQL decorators.The
SignatureCollectionAreaSummaryReport
class is well-defined, extendingSignatureCollectionArea
and using appropriate GraphQL decorators. This structure promotes code reuse and adheres to TypeScript and GraphQL best practices.For consistency with TypeScript best practices, consider adding an explicit type annotation to the
lists
field:@Field(() => [SignatureCollectionListSummary]) lists!: SignatureCollectionListSummary[];
10-29
: LGTM: Well-defined class with appropriate GraphQL field decorators.The
SignatureCollectionListSummary
class is well-structured with clear and descriptive field names. The use of non-nullable types (!
) and appropriate GraphQL decorators ensures data integrity and proper GraphQL schema generation.To improve code organization and readability, consider grouping related fields together. For example:
@ObjectType() export class SignatureCollectionListSummary { @Field(() => String) candidateName!: string @Field(() => String) listName!: string @Field(() => String) partyBallotLetter!: string @Field(() => Number) nrOfSignatures!: number @Field(() => Number) nrOfDigitalSignatures!: number @Field(() => Number) nrOfPaperSignatures!: number }This grouping makes it easier to understand the structure of the data at a glance.
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.service.ts (1)
202-211
: LGTM: New method is well-integrated and follows existing patterns.The
getAreaSummaryReport
method is correctly implemented and follows the existing patterns in the class. It properly uses TypeScript for type definitions and delegates to thesignatureCollectionClientService
.For consistency with other methods in the class, consider destructuring the
input
parameter:async getAreaSummaryReport( { collectionId, areaId }: SignatureCollectionAreaSummaryReportInput, user: User, ): Promise<SignatureCollectionAreaSummaryReport> { return await this.signatureCollectionClientService.getAreaSummaryReport( user, collectionId, areaId, ) }This change would make the method signature more consistent with others in the class and improve readability.
libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts (1)
336-353
: LGTM with suggestions: NewgetAreaSummaryReport
method added correctly.The new method is well-structured and follows the existing patterns in the class. It correctly uses
getApiWithAuth
for authentication and properly handles the API call. However, there are a few suggestions for improvement:
Consider adding more specific error handling. Instead of returning an empty object, it might be helpful to return an error message or throw a custom error.
The method could benefit from some inline documentation, especially explaining the purpose of the
AreaSummaryReport
.Consider adding input validation for
collectionId
andareaId
to ensure they are valid before parsing to integers.Here's a suggested improvement:
/** * Retrieves the area summary report for a given collection and area. * @param auth - The authentication object. * @param collectionId - The ID of the collection. * @param areaId - The ID of the area. * @returns A promise that resolves to the AreaSummaryReport. * @throws Error if the API call fails. */ async getAreaSummaryReport( auth: Auth, collectionId: string, areaId: string ): Promise<AreaSummaryReport> { if (!collectionId || !areaId) { throw new Error('Invalid collectionId or areaId'); } try { const res = await this.getApiWithAuth( this.adminApi, auth ).adminMedmaelasofnunIDSvaediInfoSvaediIDGet({ iD: parseInt(collectionId, 10), svaediID: parseInt(areaId, 10), }); return mapAreaSummaryReport(res); } catch (error) { console.error('Failed to fetch area summary report:', error); throw new Error('Failed to fetch area summary report'); } }These changes improve error handling, add documentation, and include input validation.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- libs/api/domains/signature-collection/src/lib/dto/areaSummaryReport.input.ts (1 hunks)
- libs/api/domains/signature-collection/src/lib/models/areaSummaryReport.model.ts (1 hunks)
- libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.resolver.ts (2 hunks)
- libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.service.ts (2 hunks)
- libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts (2 hunks)
- libs/clients/signature-collection/src/lib/types/areaSummaryReport.dto.ts (1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
libs/api/domains/signature-collection/src/lib/dto/areaSummaryReport.input.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/api/domains/signature-collection/src/lib/models/areaSummaryReport.model.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.resolver.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
libs/clients/signature-collection/src/lib/types/areaSummaryReport.dto.ts (1)
Pattern
libs/**/*
: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
🔇 Additional comments (17)
libs/api/domains/signature-collection/src/lib/dto/areaSummaryReport.input.ts (3)
1-2
: LGTM: Import statements are concise and appropriate.The import statements are correctly importing only the necessary items from 'class-validator' and '@nestjs/graphql', which aligns with good practices for effective tree-shaking and bundling.
4-5
: LGTM: Class definition is well-structured and follows GraphQL conventions.The
SignatureCollectionAreaSummaryReportInput
class is correctly decorated with@InputType()
, adhering to GraphQL conventions. The class name is descriptive and follows PascalCase, which is a good practice for TypeScript classes.
1-13
: LGTM: Code adheres to coding guidelines forlibs/**/*
.This input type class follows the coding guidelines for the
libs
directory:
- It's a reusable component that can be used across different NextJS apps.
- TypeScript is effectively used for defining props and exporting types.
- The small, focused nature of the class supports effective tree-shaking and bundling practices.
Great job on maintaining consistency with the project's coding standards!
libs/api/domains/signature-collection/src/lib/models/areaSummaryReport.model.ts (2)
1-2
: LGTM: Imports are correctly structured and support tree-shaking.The import statements are well-organized, importing only the necessary decorators from '@nestjs/graphql' and the base class from a local file. This approach supports effective tree-shaking and follows TypeScript best practices.
1-29
: Overall, excellent implementation adhering to project guidelines and best practices.This file demonstrates a high-quality implementation of GraphQL object types for signature collection summary reports. It effectively uses TypeScript and GraphQL decorators, promotes code reuse through inheritance, and follows best practices for defining reusable components in the
libs
directory. The structure supports effective tree-shaking and bundling, making it suitable for use across different NextJS apps.libs/clients/signature-collection/src/lib/types/areaSummaryReport.dto.ts (6)
1-1
: LGTM: Import statement is correct and follows best practices.The import statement correctly imports the necessary DTO types from the generated file, promoting type safety and consistency across the codebase.
3-9
: LGTM: AreaSummaryReport interface is well-defined and follows TypeScript best practices.The interface clearly defines the structure for an area summary report with appropriate property names and types. The use of the nested ListSummary interface for the 'lists' property is a good practice for maintaining a clear and organized data structure.
11-18
: LGTM: ListSummary interface is well-defined and follows TypeScript best practices.The interface clearly defines the structure for a list summary with appropriate property names and types. The separation of different signature counts (total, digital, and paper) provides a good level of detail for the data structure.
20-29
: LGTM: mapListSummary function is well-implemented and follows TypeScript best practices.The function effectively maps the MedmaelalistiExtendedDTO to a ListSummary object. It handles potential undefined values appropriately using optional chaining and nullish coalescing operators. The function is pure and reusable, aligning with the coding guidelines for the libs directory.
31-41
: LGTM: mapAreaSummaryReport function is well-implemented and follows TypeScript best practices.The function effectively maps the SvaediExtendedDTO to an AreaSummaryReport object. It handles potential undefined values appropriately using optional chaining and nullish coalescing operators. The use of the mapListSummary function for mapping nested lists promotes code reuse. The function is pure and reusable, aligning with the coding guidelines for the libs directory.
1-41
: LGTM: Overall structure and implementation adhere to coding guidelines.The file structure and content align well with the coding guidelines for the libs directory:
- The interfaces and functions are reusable across different NextJS apps.
- TypeScript is used effectively for defining props and exporting types.
- The code structure should allow for effective tree-shaking and bundling.
The implementation demonstrates good practices in terms of type safety, reusability, and maintainability.
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.service.ts (2)
25-27
: LGTM: New imports are appropriate and consistent.The new imports for
SignatureCollectionAreaInput
,SignatureCollectionAreaSummaryReportInput
, andSignatureCollectionAreaSummaryReport
are correctly added and follow the existing import style in the file. These imports are necessary for the new functionality being introduced.
Line range hint
1-211
: Overall: Changes align well with coding guidelines and existing patterns.The modifications to this file, including the new imports and the
getAreaSummaryReport
method, adhere to the coding guidelines forlibs/**/*
files:
- The new functionality is reusable across different NextJS apps.
- TypeScript is used effectively for defining props and exporting types.
- The changes don't introduce any obvious issues with tree-shaking or bundling.
The new method is well-integrated into the existing
SignatureCollectionAdminService
class, following established patterns and maintaining consistency with other methods.libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts (2)
26-29
: LGTM: New imports added correctly.The new imports for
AreaSummaryReport
andmapAreaSummaryReport
are correctly added and follow the existing import style in the file.
Line range hint
1-353
: Overall assessment: Changes are well-implemented and adhere to guidelines.The modifications to this file, including the new imports and the
getAreaSummaryReport
method, are consistent with the existing code style and patterns. The changes adhere to the coding guidelines for files in thelibs/**/*
pattern, ensuring:
- Reusability of components and hooks across different NextJS apps.
- Proper TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices.
The new functionality enhances the service without introducing any apparent issues. With the suggested improvements implemented, these changes are ready to be merged.
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.resolver.ts (2)
38-40
: LGTM: New imports for area summary report functionality.The new imports are correctly placed and follow the established naming conventions. They set the stage for the new area summary report functionality.
Line range hint
1-277
: Summary: New area summary report functionality adheres to coding guidelines.The changes introduce a new query for generating area summary reports, which appears to be a reusable component that could be utilized across different NextJS apps. The use of TypeScript for defining types and the structure of the new method align with the existing patterns in the file.
The additions comply with the coding guidelines for
libs/**/*
:
- The new functionality is likely reusable across different NextJS apps.
- TypeScript is used appropriately for defining types.
- The changes don't seem to negatively impact tree-shaking or bundling practices.
Overall, the implementation is sound, with a minor suggestion to add authorization decorators for consistency and security.
libs/api/domains/signature-collection/src/lib/signatureCollectionAdmin.resolver.ts
Show resolved
Hide resolved
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16230 +/- ##
=======================================
Coverage 36.96% 36.96%
=======================================
Files 6778 6786 +8
Lines 139856 139846 -10
Branches 39769 39779 +10
=======================================
+ Hits 51697 51698 +1
+ Misses 88159 88148 -11
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 21 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 5 Total Test Services: 0 Failed, 5 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (2) |
libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts
Outdated
Show resolved
Hide resolved
libs/clients/signature-collection/src/lib/signature-collection-admin.service.ts
Outdated
Show resolved
Hide resolved
…-admin.service.ts Co-authored-by: kksteini <[email protected]>
…-admin.service.ts Co-authored-by: kksteini <[email protected]>
...
Attach a link to issue if relevant
What
Specify what you're trying to achieve
Why
Specify why you need to achieve this
Screenshots / Gifs
Attach Screenshots / Gifs to help reviewers understand the scope of the pull request
Checklist:
Summary by CodeRabbit
New Features
Documentation