Fixed N+1 queries issue in /api/v1/bed/

[dhruv-goyal-10]

Proposed Changes

• is_occupied is the property defined in the Bed Model, which was contributing to N + 1 queries issue, and I have annotated the attribute _is_occupied in the queryset, which basically corresponds to the same value as the is_occupied property in Bed Model. I have handled the various attribute cases, in BedSerializer, by defining is_occupied field as SerializerMethodField, hence there will be no change in API response.

• Have added location__facility in the select_related field, as this was also contributing to the N + 1 queries issue.

Associated Issue

• Solves Fix N+1 queries in /api/v1/bed/ #1338

Merge Checklist

• Tests added/fixed
• Update docs in /docs
• Linting Complete
• Any other necessary step

Only PR's with test cases included and passing lint and test pipelines will be reviewed

@coronasafe/care-backend-maintainers @coronasafe/care-backend-admins

[dhruv-goyal-10]

@sainak @rithviknishad @vigneshhari Can you please review this PR?

[sainak]

hey @dhruv-goyal-10 thanks for the pr,
after looking at the bed model i can see that the is_occupied property on the model is not being used
https://github.com/coronasafe/care/blob/6cb4b3f26c03bece09f818299f4dccfb321ab86b/care/facility/models/bed.py#L43-L45

so we can just annotate the queryset with is_occupied directly if self.action == list

[dhruv-goyal-10]

@sainak @rithviknishad @vigneshhari I have completed the changes as suggested by @sainak. Can you review my PR now?

[dhruv-goyal-10]

Hey @sainak! I have done changes suggested by you. Can you please have a look at my PR now?

[dhruv-goyal-10]

@sainak I later realised this, commenting out the is_occupied property will although not affect the results in case of list method, but it will affect the result in case of retrieve method (fetching a particular object using id), coz we aren't annotating is_occupied property in that case and it will always corresponds to False (the default value).
Isn't it?

[sainak]

@sainak I later realised this, commenting out the is_occupied property will although not affect the results in case of list method, but it will affect the result in case of retrieve method (fetching a particular object using id), coz we aren't annotating is_occupied property in that case and it will always corresponds to False (the default value).
Isn't it?

Currently we are not using bed retrieve on Fe but you are correct that will give inconsistent results, It won't affect the performance so lets remove the if condition

[dhruv-goyal-10]

@sainak Made the changes as suggested by you.
Also, thanks for an insightful discussion.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 61.99%. Comparing base (6cb4b3f) to head (a8372ba).
Report is 1 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1908      +/-   ##
==========================================
+ Coverage   61.94%   61.99%   +0.04%     
==========================================
  Files         221      221              
  Lines       12168    12168              
  Branches     1735     1735              
==========================================
+ Hits         7538     7543       +5     
+ Misses       4324     4316       -8     
- Partials      306      309       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[dhruv-goyal-10]

@sainak Do I need to write test cases for this PR to get merged?

[sainak]

Yes!
For this case create a bunch of records and checked the number of queries executed

[dhruv-goyal-10]

@sainak I have added the test case for this as suggested by you, Please have a look now!