care icon indicating copy to clipboard operation
care copied to clipboard

Updated test cases for schedule api

Open nandkishorr opened this issue 1 month ago • 2 comments

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

@ohcnetwork/care-backend-maintainers @ohcnetwork/care-backend-admins

Summary by CodeRabbit

  • Bug Fixes

    • Corrected permission error message shown when listing schedules without proper permissions.
  • Tests

    • Expanded test coverage for schedule operations across resource types (facility, location, healthcare service), permission scenarios (including superuser), validations, and charge-item assignment flows.
    • Added test helpers and utilities to simplify schedule/availability setup and validation.

✏️ Tip: You can customize this high-level summary in your review settings.

nandkishorr avatar Nov 25 '25 07:11 nandkishorr

📝 Walkthrough

Walkthrough

A single-line permission message was corrected in the schedule viewset; extensive test infrastructure and many new/updated tests were added to cover schedules, availabilities, exceptions, resource types, permissions, and charge item definition behaviors.

Changes

Cohort / File(s) Change Summary
Permission error message correction
care/emr/api/viewsets/scheduling/schedule.py
Changed the permission denial message in can_read_resource_schedule from "update schedule" to "list schedule" when the can_list_schedule check fails. No control flow or logic altered.
Test infrastructure and coverage expansion
care/emr/tests/test_schedule_api.py
Large additions and updates: new imports (baker, ChargeItemDefinition, FacilityLocation, FacilityLocationOrganization, permission classes); new helpers (create_super_user(), get_set_charge_item_defintion_url(), create_healthcare_service(), create_facility_location(), generate_schedule_data(), generate_availiability()); facility/org/location fixtures adjusted; many tests extended/added for list/retrieve/create/update/delete of schedules, availabilities, exceptions, permission scenarios (including superuser paths), healthcare_service/location resource types, charge item definition assignment cases, and validation edge cases (dates/overlaps/slots).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

  • Reason: tiny, trivial change in production code but a large, heterogeneous set of test additions that require spot-checking behaviors and test setup correctness.
  • Pay special attention to:
    • The exact permission message text and that it aligns with other permission checks/messages.
    • New test helpers and baker usages for consistency with existing factories/fixtures.
    • Charge item definition assignment tests and their facility-scoping assumptions.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description check ⚠️ Warning The PR description is largely incomplete. It only includes the merge checklist template items but entirely omits the required 'Proposed Changes' and 'Associated Issue' sections that explain what was actually changed and why. Add 'Proposed Changes' section describing the permission message fix and test enhancements, and include 'Associated Issue' section linking to the related issue or explaining the motivation for these changes.
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (1 passed)
Check name Status Explanation
Title check ✅ Passed The title is partially related to the changeset—it mentions updating test cases for the schedule API, which is accurate for the test file changes, but omits the important fix to the permission error message in the viewset logic.
✨ Finishing touches
  • [ ] 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment
  • [ ] Commit unit tests in branch test/fix/schedule

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7c0eca6bdfbc7bb0c995d5337e0bcdec28b282b5 and bdad7a086c33c3178eaccbf34108e1e04212e12f.

📒 Files selected for processing (1)
  • care/emr/api/viewsets/scheduling/schedule.py (1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.py

📄 CodeRabbit inference engine (.cursorrules)

**/*.py: Prioritize readability and maintainability; follow Django's coding style guide (PEP 8 compliance). Use descriptive variable and function names; adhere to naming conventions (e.g., lowercase with underscores for functions and variables).

Files:

  • care/emr/api/viewsets/scheduling/schedule.py
🪛 Ruff (0.14.8)
care/emr/api/viewsets/scheduling/schedule.py

225-225: Avoid specifying long messages outside the exception class

(TRY003)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Test / Test
🔇 Additional comments (1)
care/emr/api/viewsets/scheduling/schedule.py (1)

225-225: Error message now correctly reflects the permission check.

The updated message properly indicates that the "list schedule" permission is required, aligning with the can_list_schedule authorization check above. One might wonder how this minor discrepancy went unnoticed, but at least it's fixed now.

Note: The static analysis hint (TRY003) about message length can be safely ignored here—the message is concise and appropriately descriptive.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

coderabbitai[bot] avatar Nov 25 '25 07:11 coderabbitai[bot]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 74.01%. Comparing base (e0b163e) to head (7c0eca6).

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #3377      +/-   ##
===========================================
+ Coverage    73.88%   74.01%   +0.13%     
===========================================
  Files          435      435              
  Lines        19760    19760              
  Branches      2143     2143              
===========================================
+ Hits         14599    14626      +27     
+ Misses        4715     4686      -29     
- Partials       446      448       +2     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:
  • :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

codecov[bot] avatar Nov 25 '25 07:11 codecov[bot]