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

Add rate endpoint #161

Merged
merged 12 commits into from
Jul 5, 2021
Merged

Add rate endpoint #161

merged 12 commits into from
Jul 5, 2021

Conversation

TimJentzsch
Copy link
Contributor

@TimJentzsch TimJentzsch commented Jun 18, 2021

Relevant issue: N/A

Description:

Adds a volunteer/rate endpoint to get the daily transcription rate of volunteers.
This will be used for both GrafeasGroup/buttercup#21 and GrafeasGroup/buttercup#22.

Checklist:

  • Code Quality
  • Pep-8
  • Tests (if applicable)
  • Success Criteria Met
  • Inline Documentation
  • Wiki Documentation (if applicable)

@TimJentzsch TimJentzsch added the help wanted Extra attention is needed label Jun 23, 2021
@itsthejoker itsthejoker changed the title WIP: Add rate endpoint Add rate endpoint Jun 25, 2021
@itsthejoker itsthejoker marked this pull request as ready for review June 25, 2021 21:58
@itsthejoker itsthejoker requested a review from a team as a code owner June 25, 2021 21:58
@TimJentzsch
Copy link
Contributor Author

Ideally we would make the response similar to the other paginated endpoints, i.e.:

{
  "count": <number of total results>,
  "next": <link to the next page or null>,
  "previous": <link to the previous page or null>,
  "results": <list of results>
}

If that's not possible, we can definitely work with the current version as well though.

Additionally, we should use our custom pagination:
https://github.com/GrafeasGroup/blossom/blob/master/api/pagination.py#L4-L9

This changes the rate pagination to use the custom pagination class we defined, instead of implementing a separate paginator.
This has the advantage of using the same response for other paginated requests and uses the same default parameters.
The time_frame can be used to control which time frame is used to group the rate data.
For example, you can ask for the daily or hourly transcribing rate.
@@ -58,6 +67,61 @@ def summary(self, request: Request, username: str = None) -> Response:
user = get_object_or_404(BlossomUser, username=username, is_volunteer=True)
return Response(self.serializer_class(user).data)

@swagger_auto_schema(
operation_summary=(
"Retrieve a count of transcriptions for a volunteer per UTC day."
Copy link
Member

Choose a reason for hiding this comment

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

What does the time_frame option resemble? Is it the number of days included, or the type of aggregation?

Copy link
Member

Choose a reason for hiding this comment

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

From what I gather, it is the type of aggregation. I would also reflect this in the summary of operation.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's basically the length of the time frame that is used to aggregate the rate.
If you specify "hour", it will return the hourly transcription rate.

I couldn't think of a better name, do you have any suggestions?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added a parameter description to clarify it a bit.

This can be used to not group the results at all and is useful for the history graph generation.
Additionally, the 'day' option now also returns the time as 00:00:00Z to be consistent with the other options.
@TimJentzsch TimJentzsch merged commit b69d430 into master Jul 5, 2021
@TimJentzsch TimJentzsch deleted the add-rate-endpoint branch July 5, 2021 19:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants