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

Added endpoint for company leaderboard #2123

Merged
merged 4 commits into from
Jun 4, 2024

Conversation

Uttkarsh-raj
Copy link
Contributor

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.

Screenshot (116)

Copy link
Contributor

@arkid15r arkid15r left a 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:

website/api/views.py Outdated Show resolved Hide resolved
website/api/views.py Outdated Show resolved Hide resolved
@AtmegaBuzz
Copy link
Collaborator

AtmegaBuzz commented May 31, 2024

@Uttkarsh-raj

  1. @action decorator can be only used inside modelviewset and not api view
  2. as i already told you, you can easily chain foreignkeys to fetch the values from parent

Here Company is parent of Domain and DOmain is parent of Issue
Company -> DOmain -> Issue
so the parents above the hierarchy can access their child values using __
if 'X' has 5 level child then you have to add 5 _ eg "domain__issue__x__y__zfield"

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)

@Uttkarsh-raj
Copy link
Contributor Author

Uttkarsh-raj commented Jun 3, 2024

  1. @action decorator can be only used inside modelviewset and not api view
  2. as i already told you, you can easily chain foreignkeys to fetch the values from parent
    Try this logic it is db level optimised with pagination

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

@AtmegaBuzz
Copy link
Collaborator

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.

@Uttkarsh-raj
Copy link
Contributor Author

Uttkarsh-raj commented Jun 3, 2024

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.


class APIView(View):

    @classmethod
    def as_view(cls, **initkwargs):

    @property
    def allowed_methods(self):

    @property
    def default_response_headers(self):

    def http_method_not_allowed(self, request, *args, **kwargs):

    def permission_denied(self, request, message=None, code=None):

    def throttled(self, request, wait):

    def get_authenticate_header(self, request):

    def get_parser_context(self, http_request):

    def get_renderer_context(self):

    def get_exception_handler_context(self):

    def get_view_name(self):

    def get_view_description(self, html=False):

    def get_format_suffix(self, **kwargs):

    def get_renderers(self):

    def get_parsers(self):

    def get_authenticators(self):

    def get_permissions(self):

    def get_throttles(self):

    def get_content_negotiator(self):
 
    def get_exception_handler(self):

    def perform_content_negotiation(self, request, force=False):

    def perform_authentication(self, request):

    def check_permissions(self, request):

    def check_object_permissions(self, request, obj):

    def check_throttles(self, request):

    def determine_version(self, request, *args, **kwargs):

    def initialize_request(self, request, *args, **kwargs):

    def initial(self, request, *args, **kwargs):

    def finalize_response(self, request, response, *args, **kwargs):

    def handle_exception(self, exc):

    def raise_uncaught_exception(self, exc):

    def dispatch(self, request, *args, **kwargs):

    def options(self, request, *args, **kwargs):

@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.
Thank you.

@AtmegaBuzz
Copy link
Collaborator

Screenshot_20240604-000223.png

See here, api view inherits get api view, post api ... That's why you didn't see it cause it's inside the parent classes

@AtmegaBuzz
Copy link
Collaborator

AtmegaBuzz commented Jun 4, 2024

Looks good, good work @Uttkarsh-raj

@DonnieBLT DonnieBLT enabled auto-merge (squash) June 4, 2024 12:57
@DonnieBLT DonnieBLT merged commit b65d85e into OWASP-BLT:main Jun 4, 2024
8 checks passed
@Uttkarsh-raj
Copy link
Contributor Author

Looks good, good work @Uttkarsh-raj

Thank you. Really appreciate the reviews.

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