care icon indicating copy to clipboard operation
care copied to clipboard
verified publisher icon coronasafe

Test cases for observation definition apis

Open nandkishorr opened this issue 5 months ago • 4 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

  • Tests
    • Added comprehensive automated tests for Observation Definition workflows (create, retrieve, update, list), access control, validation, duplicate identifiers, filtering, and metrics formatting.
  • Bug Fixes
    • Improved handling when an observation lacks a facility to prevent errors and ensure consistent identifier/slug behavior.

nandkishorr avatar Jul 21 '25 21:07 nandkishorr

📝 Walkthrough

Walkthrough

Adds a new comprehensive test module for Observation Definition API and adjusts slug/facility handling inside validate_data to avoid AttributeError by using instance-level slug calculation when facility is absent.

Changes

Cohort / File(s) Summary
Observation Definition API tests
care/emr/tests/test_observation_definition_api.py		 New comprehensive test suite covering create, retrieve, update, list, filtering, access control, slug uniqueness (facility vs instance), permitted_data_type validation, facility-less scenarios, and metrics endpoint checks using DRF test client and model_bakery.
Observation Definition viewset change
care/emr/api/viewsets/observation_definition.py		 Adjusted validate_data to guard against missing model_obj.facility; when facility is falsy the code leaves facility as None and uses instance-level slug calculation instead of attempting to access facility.external_id. No other logic paths changed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Tester as Test Client
  participant API as Observation Definition API
  participant Auth as Permissions
  participant View as validate_data
  participant DB as DB

  rect rgb(232,244,253)
  note over Tester,API: Create / Update flow (high-level)
  Tester->>API: POST/PUT /observation-definitions
  API->>Auth: Check user permissions & facility context
  Auth-->>API: Allow / Deny
  API->>View: validate_data(model_obj)
  alt model_obj.facility exists
    View->>View: facility_id = str(model_obj.facility.external_id)
    View->>DB: calculate_slug_from_facility(...)
  else facility missing
    View->>View: facility = None
    View->>DB: calculate_slug_from_instance(...)
  end
  DB-->>API: Success / validation errors (slug dup, data type)
  API-->>Tester: 200/201 or 400/403
  end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

Tests arrive with checklists, sharp and neat,
Slugs now bow out when facilities retreat.
Permissions murmur, “Only some may pass,”
Errors tidy up where attributes trespass.
Small fix, big relief — thanks for doing the math.

Pre-merge checks and finishing touches

❌ Failed checks (2 warnings)
Check name Status Explanation Resolution
Description Check ⚠️ Warning The description only includes the merge checklist and omits the required “Proposed Changes,” “Associated Issue,” and optional “Architecture changes” sections from the repository’s template, leaving the purpose and scope of the PR undefined. Please fill out the “Proposed Changes” section with a brief summary of both test additions and the minor viewset fix, add an “Associated Issue” link or explanation, and include or remove the “Architecture changes” section according to the template.
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 clearly and concisely describes the main change, which is the addition of test cases for the Observation Definition API, making it immediately clear to reviewers what this PR focuses on.
✨ 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 nandkishorr/test/observation_definition
📜 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 2888b36b619562ae226e412aefbe6ec2d48f084b and ecfe8660aaa3ff990f8dac15e9531bf6f290405a.

📒 Files selected for processing (1)
  • care/emr/api/viewsets/observation_definition.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/observation_definition.py
⏰ 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/observation_definition.py (1)

74-75: Guard against missing facility—looks good.

The conditional check before accessing model_obj.facility.external_id correctly prevents an AttributeError when updating facility-less observation definitions. This aligns nicely with the slug calculation logic below, which already branches on facility presence.

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 Jul 21 '25 21:07 coderabbitai[bot]

Codecov Report

:x: Patch coverage is 0% with 2 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 53.87%. Comparing base (3d2a3cc) to head (22e69fe).

Files with missing lines Patch % Lines
care/emr/api/viewsets/observation_definition.py 0.00% 2 Missing :warning:
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop    #3157      +/-   ##
===========================================
- Coverage    53.87%   53.87%   -0.01%     
===========================================
  Files          434      434              
  Lines        19683    19684       +1     
  Branches      2129     2130       +1     
===========================================
  Hits         10604    10604              
- Misses        9051     9052       +1     
  Partials        28       28

: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 Jul 21 '25 21:07 codecov[bot]

Let's not merge this right now. New interpretation changes will be coming in soon :)

praffq avatar Aug 07 '25 14:08 praffq

Let's not merge this right now. New interpretation changes will be coming in soon :)

they can be added in a separate pr

sainak avatar Aug 29 '25 13:08 sainak