Skip to content
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): Endpoint for parliamentary candidate list overview #16487

Merged
merged 2 commits into from
Oct 21, 2024

Conversation

juni-haukur
Copy link
Member

@juni-haukur juni-haukur commented Oct 21, 2024

...

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:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • Formatting passes locally with my changes
  • I have rebased against main before asking for a review

Summary by CodeRabbit

  • New Features

    • Introduced a new API endpoint to retrieve an overview of specific recommendation lists.
    • Added methods for fetching list summaries in both the server and client services.
  • Bug Fixes

    • Enhanced access control for fetching signature collection overviews.
  • Documentation

    • Updated API specifications to include details about the new endpoint and response schemas.

@juni-haukur juni-haukur requested a review from a team as a code owner October 21, 2024 11:55
Copy link
Contributor

coderabbitai bot commented Oct 21, 2024

Walkthrough

This pull request introduces several new methods and an API endpoint related to signature collections. A new query method signatureCollectionListOverview is added to the SignatureCollectionResolver, which retrieves an overview of a signature collection. Correspondingly, the SignatureCollectionService now includes a listOverview method for fetching the list details. Additionally, a new GET endpoint /Medmaelalistar/{ID}/Info is added to the API specification. The SignatureCollectionClientService also gains a getListOverview method to interact with this new API endpoint.

Changes

File Change Summary
libs/api/domains/signature-collection/src/lib/signatureCollection.resolver.ts - Added method: signatureCollectionListOverview to retrieve a summary of a signature collection.
- Added import: SignatureCollectionListSummary.
libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts - Added method: listOverview to fetch an overview of a specific list for a user.
- Added import: SignatureCollectionListSummary.
libs/clients/signature-collection/src/clientConfig.json - Added new endpoint: GET /Medmaelalistar/{ID}/Info for retrieving a list overview.
- Defined response schema: MedmaelalistiExtendedDTO.
libs/clients/signature-collection/src/lib/signature-collection.service.ts - Added method: getListOverview to call the new API endpoint and return a list summary.
- Added imports: ListSummary, mapListSummary.

Possibly related PRs

Suggested labels

automerge, high priority

Suggested reviewers

  • kksteini

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (4)
libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts (1)

192-200: LGTM: New method is well-structured and consistent.

The listOverview method is correctly implemented and maintains consistency with other methods in the class. It adheres to the coding guidelines for reusability across different NextJS apps.

Consider adding error handling to improve robustness:

async listOverview(
  user: User,
  listId: string,
): Promise<SignatureCollectionListSummary> {
  try {
    return await this.signatureCollectionClientService.getListOverview(
      user,
      listId,
    );
  } catch (error) {
    // Log the error
    console.error('Error fetching list overview:', error);
    // Rethrow or handle as appropriate
    throw new Error('Failed to fetch list overview');
  }
}

This addition would make the method more resilient and easier to debug.

libs/api/domains/signature-collection/src/lib/signatureCollection.resolver.ts (1)

212-222: LGTM: New query method is well-implemented.

The signatureCollectionListOverview method is correctly implemented with proper decorators for access control and auditing. It follows the established pattern of other query methods in this resolver and delegates to the service layer, which is a good practice.

Minor suggestion for consistency:
Consider adding a @CurrentSignee() parameter, as seen in other methods like signatureCollectionList. This might be useful if you need to perform any signee-specific checks in the service layer.

async signatureCollectionListOverview(
  @CurrentUser() user: User,
  @CurrentSignee() signee: SignatureCollectionSignee,
  @Args('input') input: SignatureCollectionListIdInput,
): Promise<SignatureCollectionListSummary> {
  return this.signatureCollectionService.listOverview(user, signee, input.listId)
}

This change would make the method signature more consistent with other methods in the resolver and potentially provide more context to the service layer if needed.

libs/clients/signature-collection/src/lib/signature-collection.service.ts (1)

566-575: LGTM: New getListOverview method is well-implemented.

The method follows the existing patterns in the class, correctly handles authentication, and uses async/await properly. Good job on parsing the listId to an integer for the API call.

Consider adding error handling to catch and properly handle any API errors:

async getListOverview(auth: User, listId: string): Promise<ListSummary> {
  try {
    const summary = await this.getApiWithAuth(
      this.listsApi,
      auth,
    ).medmaelalistarIDInfoGet({
      iD: parseInt(listId),
    })

    return mapListSummary(summary)
  } catch (error) {
    // Log the error or handle it appropriately
    throw new Error(`Failed to get list overview: ${error.message}`)
  }
}
libs/clients/signature-collection/src/clientConfig.json (1)

1079-1113: Endpoint addition looks good, consider enhancing the description.

The new endpoint "/Medmaelalistar/{ID}/Info" is well-structured and consistent with the existing API patterns. It follows RESTful conventions and provides appropriate documentation.

Consider adding an English translation or explanation in the description for better international developer experience. For example:

 "description": "Framboð geta sótt yfirlit fyrir sína eigin lista",
+"description": "Framboð geta sótt yfirlit fyrir sína eigin lista (Candidates can retrieve an overview of their own lists)",
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 4e91db4 and 63021ed.

📒 Files selected for processing (4)
  • libs/api/domains/signature-collection/src/lib/signatureCollection.resolver.ts (2 hunks)
  • libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts (2 hunks)
  • libs/clients/signature-collection/src/clientConfig.json (1 hunks)
  • libs/clients/signature-collection/src/lib/signature-collection.service.ts (2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
libs/api/domains/signature-collection/src/lib/signatureCollection.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/signatureCollection.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/clientConfig.json (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.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."
🔇 Additional comments (5)
libs/api/domains/signature-collection/src/lib/signatureCollection.service.ts (2)

21-21: LGTM: Import statement is correctly placed and necessary.

The new import for SignatureCollectionListSummary is appropriately placed with other imports and is required for the new listOverview method. This adheres to the coding guideline of TypeScript usage for defining types.


Line range hint 1-200: Overall, the changes are well-implemented and align with the PR objectives.

The additions to this file (new import and listOverview method) are consistent with the existing code structure and contribute to the goal of providing an overview of parliamentary candidate lists. The changes adhere to the coding guidelines for reusability and TypeScript usage.

To further improve the implementation:

  1. Consider adding unit tests for the new listOverview method to ensure its functionality and maintain code quality.
  2. Update the documentation (if any) to reflect the new capability added to the SignatureCollectionService.

These suggestions will enhance the overall quality and maintainability of the code.

To ensure comprehensive test coverage, run the following command to check for existing tests and identify any gaps:

libs/api/domains/signature-collection/src/lib/signatureCollection.resolver.ts (2)

34-34: LGTM: New import statement is correct and necessary.

The import of SignatureCollectionListSummary is correctly added and is required for the new query method. It follows TypeScript best practices and the file path seems appropriate for the project structure.


Line range hint 1-222: Summary: Changes align well with PR objectives.

The new query method signatureCollectionListOverview and its associated import successfully implement the functionality described in the PR objectives. The changes are consistent with the existing code patterns and follow good practices for GraphQL resolvers in TypeScript.

The new endpoint provides the required overview of the list of parliamentary candidates, as intended. The implementation is clean, well-structured, and properly integrated into the existing SignatureCollectionResolver class.

libs/clients/signature-collection/src/lib/signature-collection.service.ts (1)

26-26: LGTM: New import statement is correctly placed and follows best practices.

The import of ListSummary and mapListSummary is appropriately added and aligns with TypeScript best practices.

@albinagu albinagu added the automerge Merge this PR as soon as all checks pass label Oct 21, 2024
Copy link

codecov bot commented Oct 21, 2024

Codecov Report

Attention: Patch coverage is 25.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 36.79%. Comparing base (afc168b) to head (7781dc1).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...collection/src/lib/signature-collection.service.ts 25.00% 3 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##             main   #16487   +/-   ##
=======================================
  Coverage   36.79%   36.79%           
=======================================
  Files        6845     6845           
  Lines      141716   141720    +4     
  Branches    40363    40373   +10     
=======================================
+ Hits        52138    52140    +2     
- Misses      89578    89580    +2     
Flag Coverage Δ
api 3.37% <ø> (ø)
application-system-api 41.37% <25.00%> (-0.01%) ⬇️
application-template-api-modules 27.81% <ø> (+0.01%) ⬆️
application-ui-shell 21.37% <ø> (ø)
judicial-system-backend 55.19% <ø> (ø)
license-api 42.44% <ø> (-0.08%) ⬇️
nest-core 43.54% <ø> (ø)
services-auth-ids-api 51.42% <ø> (+0.01%) ⬆️
services-user-notification 46.93% <ø> (ø)
services-user-profile 61.84% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
...collection/src/lib/signature-collection.service.ts 7.01% <25.00%> (+0.32%) ⬆️

... and 2 files with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update afc168b...7781dc1. Read the comment docs.

@datadog-island-is
Copy link

Datadog Report

All test runs 4303c85 🔗

8 Total Test Services: 0 Failed, 8 Passed
🔻 Test Sessions change in coverage: 3 decreased, 17 no change

Test Services
Service Name Failed Known Flaky New Flaky Passed Skipped Total Time Code Coverage Change Test Service View
api 0 0 0 4 0 2.83s 1 no change Link
application-system-api 0 0 0 120 2 3m 13.38s 1 no change Link
application-template-api-modules 0 0 0 123 0 2m 36.57s 1 no change Link
application-ui-shell 0 0 0 74 0 38.28s 1 no change Link
license-api 0 0 0 80 0 27.5s 1 no change Link
services-auth-ids-api 0 0 0 152 0 1m 20.8s 1 no change Link
services-user-notification 0 0 0 51 0 1m 25.56s 1 no change Link
services-user-profile 0 0 0 131 0 30.21s 1 decreased (-0.02%) Link

🔻 Code Coverage Decreases vs Default Branch (3)

  • api-domains-signature-collection - jest 12.25% (-0.09%) - Details
  • clients-signature-collection - jest 22.04% (-0.03%) - Details
  • services-user-profile - jest 52.54% (-0.02%) - Details

@kodiakhq kodiakhq bot merged commit e405c4c into main Oct 21, 2024
49 checks passed
@kodiakhq kodiakhq bot deleted the feat/candidate-list-overview branch October 21, 2024 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this PR as soon as all checks pass
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants