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

Improving typing of PaginatedResponse. #13913

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

dennisoelkers
Copy link
Member

Description

Motivation and Context

This PR is removing the capability of the PaginatedResponse class to define a custom key which is used to index the entity list wrapped in the response. This makes it possible to get rid of the @JsonValue method and use a simple AutoValue-class instead, enabling swagger to generate proper typings.

The benefit of this is that the response format for paginated endpoints is always identical and frontend clients can make use of the generated types.

A future refactoring could include typing the context field properly by adding a generic parameter to the class.

/jenkins-pr-deps Graylog2/graylog-plugin-enterprise#4313

How Has This Been Tested?

Screenshots (if appropriate):

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Refactoring (non-breaking change)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.

@dennisoelkers dennisoelkers force-pushed the refactor/improve-typing-of-paginated-response branch from 8e02512 to 19adc6d Compare November 9, 2022 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant