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

optimized the query performed to fetch skills in UserAssignedSerializer #1310

Closed
wants to merge 0 commits into from

Conversation

yaswanthsaivendra
Copy link
Contributor

@yaswanthsaivendra yaswanthsaivendra commented May 23, 2023

Proposed Changes

  • deleted add_skills method in UserAssignedSerializer
  • overided to_representation method .
  • Made use of prefetch_related method to fetch skills from a many to many relationship to skills. So that skills are prefetched using a single query . This way we can access the prefetched skills from memory rather than making multiple db requests.
  • prefetch_related (internally makes use of JOIN) to fetch all the related objects in a single query.

Associated Issue

Architecture changes

@coronasafe/code-reviewers

Merge Checklist

  • Tests added/fixed
  • Update docs in /docs
  • Linting Complete

@yaswanthsaivendra
Copy link
Contributor Author

@sainak
UserAssingedSerializer is used at 2 places.

  1. in FacilityUserViewSet, where I have made the changes to prefetch the skills.
  2. in PatientConsulationSerializer here i am not sure how it can be done.
    Can u suggest , what can be done here?

@sainak
Copy link
Member

sainak commented May 27, 2023

@yaswanthsaivendra in patient consultation view you can prefetch assigned_to__skills

@yaswanthsaivendra
Copy link
Contributor Author

Hey @sainak ,
I made the necessary changes , can u review it please!

@sainak
Copy link
Member

sainak commented Jun 1, 2023

@yaswanthsaivendra there are no changes in this pr
you should create a new branch for every pull request

@yaswanthsaivendra
Copy link
Contributor Author

@yaswanthsaivendra there are no changes in this pr you should create a new branch for every pull request

sorry, I messed a bit. Pushed the changes Now.

The worst thing, I did at the start is making changes in my master branch which leads to all this mess in the commits. Will avoid it in my further prs.

care/users/api/serializers/user.py Outdated Show resolved Hide resolved
@sonarcloud
Copy link

sonarcloud bot commented Jun 3, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@codecov-commenter
Copy link

Codecov Report

Patch coverage: 54.54% and project coverage change: -0.01 ⚠️

Comparison is base (831c216) 56.66% compared to head (dcbdca6) 56.66%.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1310      +/-   ##
==========================================
- Coverage   56.66%   56.66%   -0.01%     
==========================================
  Files         195      195              
  Lines        9861     9865       +4     
  Branches     1655     1656       +1     
==========================================
+ Hits         5588     5590       +2     
- Misses       4219     4221       +2     
  Partials       54       54              
Impacted Files Coverage Δ
care/facility/api/viewsets/facility_users.py 75.86% <50.00%> (-4.14%) ⬇️
care/facility/api/viewsets/patient_consultation.py 51.04% <50.00%> (-0.58%) ⬇️
care/users/api/serializers/user.py 59.47% <100.00%> (+0.50%) ⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Optimize the query to fetch the skills associated to a User in UserAssignedSerializer
4 participants