-
-
Notifications
You must be signed in to change notification settings - Fork 158
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
Added endpoint for company leaderboard #2123
Conversation
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.
Could you check if the following suggestions make sense in this context:
Here Company is parent of Domain and DOmain is parent of Issue Try this logic it is db level optimised with pagination def company_leaderboard(self, request, *args, **kwargs):
paginator = PageNumberPagination()
companies = Company.objects.values().annotate(issue_count=Count('domain__issue')).order_by("-issue_count")
page = paginator.paginate_queryset(companies, request)
return paginator.get_paginated_response(page) |
This do seems to work . Thanks for the insight , this is my first time with working with this orm, i think i came up with the correct sql query and maybe i was not able to use the orm optimally thanks for the review @AtmegaBuzz . Working on it. But there is a problem where in the APIViewSet we dont have a def of get which is handling the logic to methods or acts as a filter. What should i do? should we change the viewset to Model's one and add the get in the url ? Also do we have to show all the companies in the leaderboard or only the top 100 or so? @DonnieBLT |
No worries @Uttkarsh-raj, every time you make a pr I will try to give an optimised orm query to give a new perspective of how powerful it is, so that you can learn from it. Api view has a get method and you can override it. Modelviewset doesn't have get instead it uses list and show. I think the api view is also fine if the current implementation works. |
@AtmegaBuzz I could find only the above methods in the APIView. And i tried overriding the get api which is not getting the required results. |
Looks good, good work @Uttkarsh-raj |
Thank you. Really appreciate the reviews. |
Added endpoints to get a scoreboard/ leaderboard for the companies according to the total number of issues.
Will help in implementing and tracking the leaderboard for the companies in the application.