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

21929 - get reviews endpoint #2830

Merged
merged 6 commits into from
Jul 16, 2024
Merged

Conversation

ketaki-deodhar
Copy link
Collaborator

@ketaki-deodhar ketaki-deodhar commented Jul 15, 2024

Issue #: /bcgov/entity#21929

Description of changes:

  • "get reviews" endpoint
  • support pagination (added logic to handle limit and page)
  • add unit tests

image

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of the lear license (Apache 2.0).

@ketaki-deodhar ketaki-deodhar self-assigned this Jul 15, 2024
@ketaki-deodhar ketaki-deodhar marked this pull request as ready for review July 15, 2024 16:40
@ketaki-deodhar
Copy link
Collaborator Author

ketaki-deodhar commented Jul 15, 2024

WIP for unit tests. added 2 and they run successfully added tests for pagination

headers=create_header(jwt, [STAFF_ROLE], 'user'))

assert rv.status_code == HTTPStatus.OK
assert 'reviews' in rv.json
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you add some data to review table to verify this?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Working on it

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have an initial view of what the future arguments should look like?

  • sorting by different fields
  • filtering on different fields

@classmethod
def get_paginated_reviews(cls, page, limit):
"""Return paginated reviews."""
query = db.session.query(Review).order_by(Review.creation_date.asc())
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Yes, the default sort is oldest first. This is probably worth a comment, but then again, this code will be expanded soon to handle filtering and sorting by different fields.

Copy link
Collaborator

@severinbeauvais severinbeauvais left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. I agree with Vysakh that it would be nice to have some test data in here.

Vysakh, could we merge this now (which would allow Arwen to call this) and then work on the test data?

Copy link
Collaborator

@ArwenQin ArwenQin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, LGTM

@ketaki-deodhar ketaki-deodhar merged commit 608b226 into bcgov:main Jul 16, 2024
6 checks passed
@ketaki-deodhar ketaki-deodhar deleted the 21929 branch August 23, 2024 17:34
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.

4 participants