fix for entered in error choices #3061
Conversation
📝 WalkthroughWalkthroughThis change refactors EMR viewset classes to use explicit mixins instead of a monolithic base class and adds model-level validators across multiple resource specs to prevent updates to records with a status of Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant API_ViewSet
participant ModelValidator
Client->>API_ViewSet: PATCH/PUT (update resource)
API_ViewSet->>ModelValidator: validate_status/validate_verification_status
ModelValidator-->>API_ViewSet: Raise error if status == "entered_in_error"
API_ViewSet-->>Client: Return error if update not allowed
API_ViewSet-->>Client: Proceed with update if allowed
Assessment against linked issues
Assessment against linked issues: Out-of-scope changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3061 +/- ##
===========================================
- Coverage 51.28% 51.17% -0.11%
===========================================
Files 251 251
Lines 11371 11444 +73
Branches 1280 1288 +8
===========================================
+ Hits 5832 5857 +25
- Misses 5518 5566 +48
Partials 21 21 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
care/emr/resources/medication/administration/spec.py (1)
161-176: Validation logic is sound and consistent.The
validate_statusmethod correctly implements the business rule to prevent updates on records marked as "entered_in_error". The logic matches the pattern used across other resource specs, which is... reassuring, I suppose.Consider adding a docstring to document the validation rule:
@model_validator(mode="after") def validate_status(self, info: ValidationInfo): + """Prevent updates to medication administration records with 'entered_in_error' status.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 162-162: Missing function or method docstring
(C0116)
care/emr/resources/consent/spec.py (1)
98-111: Validator implementation follows the established pattern correctly.The validation logic appropriately prevents updates to consent records with "entered_in_error" status. The implementation is consistent with similar validators across the codebase.
Adding a docstring would improve code documentation:
@model_validator(mode="after") def validate_status(self, info: ValidationInfo): + """Prevent updates to consent records with 'entered_in_error' status.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 99-99: Missing function or method docstring
(C0116)
care/emr/resources/encounter/spec.py (1)
93-105: Validation implementation is consistent and correct.The
validate_statusmethod properly prevents updates to encounter records with "entered_in_error" status. The use ofStatusChoices.entered_in_error.valuecorrectly references the imported enum.Consider adding a docstring for better documentation:
@model_validator(mode="after") def validate_status(self, info: ValidationInfo): + """Prevent updates to encounter records with 'entered_in_error' status.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 94-94: Missing function or method docstring
(C0116)
care/emr/resources/scheduling/slot/spec.py (1)
75-87: Add docstring for better code documentation.The validation logic correctly prevents updates to bookings with "entered_in_error" status, which aligns with the PR objectives. However, the method would benefit from a docstring explaining its purpose and behavior.
@model_validator(mode="after") def validate_status(self, info: ValidationInfo): + """Prevent updates to bookings that are marked as entered in error.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 76-76: Missing function or method docstring
(C0116)
care/emr/resources/medication/statement/spec.py (1)
59-71: Validation logic is solid, but please add documentation.The implementation correctly prevents updates to medication statements marked as "entered_in_error" and follows the same pattern as other resource specs in this PR. The error message is appropriately descriptive, though a docstring would improve maintainability.
@model_validator(mode="after") def validate_verification_status(self, info: ValidationInfo): + """Prevent updates to medication statements with entered_in_error status.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 60-60: Missing function or method docstring
(C0116)
care/emr/resources/allergy_intolerance/spec.py (1)
80-98: Validation implementation is correct and appropriately adapted.The validator correctly prevents updates to allergy intolerance records with "entered_in_error" verification status. The method name and error message are appropriately specific to the verification_status field, which is a nice touch compared to the generic status checks in other files. Just missing a docstring for completeness.
@model_validator(mode="after") def validate_verification_status(self, info: ValidationInfo): + """Prevent updates to allergy intolerance records with entered_in_error verification status.""" context = info.context or {}🧰 Tools
🪛 Pylint (3.3.7)
[convention] 81-81: Missing function or method docstring
(C0116)
care/emr/resources/condition/spec.py (1)
150-167: Validation logic is correct and follows established pattern.The implementation properly prevents updates to conditions with "entered_in_error" verification status and maintains consistency with the allergy intolerance validation approach. The error message accurately reflects the field being validated. As with the other files, a docstring would improve documentation.
@model_validator(mode="after") def validate_verification_status(self, info: ValidationInfo): + """Prevent updates to conditions with entered_in_error verification status.""" context = info.context or {}Consider abstracting the validation pattern.
While the validation logic is correctly implemented across all files, there's notable code duplication. Since this pattern appears in multiple resource specs, consider creating a base validation mixin or utility function to reduce duplication and improve maintainability.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 151-151: Missing function or method docstring
(C0116)
care/emr/api/viewsets/device.py (1)
54-61: Excellent architectural improvement with explicit mixin composition.This refactoring makes the viewset's capabilities crystal clear and provides better modularity than the previous monolithic inheritance. The removal of delete functionality aligns perfectly with the PR's goal of restricting operations on "entered_in_error" records.
One tiny suggestion though - perhaps consider adding a class docstring to document the viewset's purpose and capabilities?
class DeviceViewSet( EMRCreateMixin, EMRRetrieveMixin, EMRUpdateMixin, EMRListMixin, EMRBaseViewSet, EMRUpsertMixin, ): + """ + ViewSet for managing medical devices within facilities. + + Provides CRUD operations and device association management with + encounters, locations, and organizations. + """🧰 Tools
🪛 Pylint (3.3.7)
[convention] 54-54: Missing class docstring
(C0115)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (20)
care/emr/api/viewsets/condition.py(3 hunks)care/emr/api/viewsets/consent.py(2 hunks)care/emr/api/viewsets/device.py(2 hunks)care/emr/api/viewsets/medication_administration.py(2 hunks)care/emr/api/viewsets/medication_request.py(2 hunks)care/emr/api/viewsets/medication_statement.py(2 hunks)care/emr/resources/allergy_intolerance/spec.py(2 hunks)care/emr/resources/condition/spec.py(2 hunks)care/emr/resources/consent/spec.py(2 hunks)care/emr/resources/device/spec.py(2 hunks)care/emr/resources/encounter/spec.py(2 hunks)care/emr/resources/medication/administration/spec.py(2 hunks)care/emr/resources/medication/request/spec.py(2 hunks)care/emr/resources/medication/statement/spec.py(2 hunks)care/emr/resources/scheduling/slot/spec.py(2 hunks)care/emr/tests/test_allergy_intolerance_api.py(7 hunks)care/emr/tests/test_consent_api.py(2 hunks)care/emr/tests/test_device_api.py(2 hunks)care/emr/tests/test_diagnosis_api.py(5 hunks)care/emr/tests/test_symptom_api.py(5 hunks)
🧰 Additional context used
🧠 Learnings (1)
care/em 8000 r/resources/allergy_intolerance/spec.py (1)
Learnt from: DraKen0009
PR: ohcnetwork/care#2931
File: care/emr/migrations/0023_allergyintolerance_allergy_intolerance_type.py:16-16
Timestamp: 2025-03-19T14:53:14.299Z
Learning: For the AllergyIntolerance model, field validations are handled by Pydantic specs (AllergyIntoleranceWriteSpec) using enum types rather than in the Django model definition. The field `allergy_intolerance_type` is validated through the AllergyIntoleranceTypeOptions enum which contains "allergy" and "intolerance" options.
🧬 Code Graph Analysis (18)
care/emr/tests/test_diagnosis_api.py (1)
care/emr/resources/condition/spec.py (1)
VerificationStatusChoices(29-35)
care/emr/resources/device/spec.py (4)
care/emr/resources/consent/spec.py (1)
validate_status(99-111)care/emr/resources/encounter/spec.py (1)
validate_status(94-105)care/emr/resources/medication/administration/spec.py (1)
validate_status(162-176)care/emr/resources/scheduling/slot/spec.py (1)
validate_status(76-87)
care/emr/resources/consent/spec.py (4)
care/emr/resources/encounter/spec.py (1)
validate_status(94-105)care/emr/resources/medication/administration/spec.py (1)
validate_status(162-176)care/emr/resources/device/spec.py (1)
validate_status(71-81)care/emr/resources/scheduling/slot/spec.py (1)
validate_status(76-87)
care/emr/tests/test_consent_api.py (1)
care/emr/resources/consent/spec.py (1)
ConsentStatusChoices(22-27)
care/emr/resources/encounter/spec.py (5)
care/emr/resources/consent/spec.py (1)
validate_status(99-111)care/emr/resources/medication/administration/spec.py (1)
validate_status(162-176)care/emr/resources/device/spec.py (1)
validate_status(71-81)care/emr/resources/scheduling/slot/spec.py (1)
validate_status(76-87)care/emr/resources/encounter/constants.py (1)
StatusChoices(4-13)
care/emr/resources/medication/statement/spec.py (3)
care/emr/resources/allergy_intolerance/spec.py (1)
validate_verification_status(81-98)care/emr/resources/condition/spec.py (1)
validate_verification_status(151-167)care/emr/resources/medication/request/spec.py (1)
validate_verification_status(200-211)
care/emr/resources/scheduling/slot/spec.py (4)
care/emr/resources/consent/spec.py (1)
validate_status(99-111)care/emr/resources/encounter/spec.py (1)
validate_status(94-105)care/emr/resources/medication/administration/spec.py (1)
validate_status(162-176)care/emr/resources/device/spec.py (1)
validate_status(71-81)
care/emr/tests/test_symptom_api.py (1)
care/emr/resources/allergy_intolerance/spec.py (1)
VerificationStatusChoices(22-27)
care/emr/resources/medication/administration/spec.py (4)
care/emr/resources/consent/spec.py (1)
validate_status(99-111)care/emr/resources/encounter/spec.py (1)
validate_status(94-105)care/emr/resources/device/spec.py (1)
validate_status(71-81)care/emr/resources/scheduling/slot/spec.py (1)
validate_status(76-87)
care/emr/tests/test_allergy_intolerance_api.py (1)
care/emr/resources/allergy_intolerance/spec.py (1)
VerificationStatusChoices(22-27)
care/emr/tests/test_device_api.py (1)
care/emr/resources/device/spec.py (1)
DeviceStatusChoices(16-19)
care/emr/resources/allergy_intolerance/spec.py (4)
care/emr/resources/condition/spec.py (2)
validate_verification_status(151-167)VerificationStatusChoices(29-35)care/emr/resources/medication/request/spec.py (1)
validate_verification_status(200-211)care/emr/resources/medication/statement/spec.py (1)
validate_verification_status(60-71)care/emr/resources/base.py (1)
is_update(41-42)
care/emr/resources/condition/spec.py (1)
care/emr/resources/allergy_intolerance/spec.py (2)
validate_verification_status(81-98)VerificationStatusChoices(22-27)
care/emr/api/viewsets/medication_statement.py (2)
care/emr/api/viewsets/base.py (7)
EMRBaseViewSet(229-271)EMRCreateMixin(70-108)EMRListMixin(111-124)EMRQuestionnaireResponseMixin(274-275)EMRRetrieveMixin(63-67)EMRUpdateMixin(127-179)EMRUpsertMixin(201-226)care/emr/api/viewsets/encounter_authz_base.py (1)
EncounterBasedAuthorizationBase(8-44)
care/emr/api/viewsets/medication_request.py (2)
care/emr/api/viewsets/base.py (7)
EMRBaseViewSet(229-271)EMRCreateMixin(70-108)EMRListMixin(111-124)EMRQuestionnaireResponseMixin(274-275)EMRRetrieveMixin(63-67)EMRUpdateMixin(127-179)EMRUpsertMixin(201-226)care/emr/api/viewsets/encounter_authz_base.py (1)
EncounterBasedAuthorizationBase(8-44)
care/emr/api/viewsets/device.py (1)
care/emr/api/viewsets/base.py (6)
EMRUpsertMixin(201-226)EMRCreateMixin(70-108)EMRRetrieveMixin(63-67)EMRUpdateMixin(127-179)EMRListMixin(111-124)EMRBaseViewSet(229-271)
care/emr/resources/medication/request/spec.py (3)
care/emr/resources/allergy_intolerance/spec.py (1)
validate_verification_status(81-98)care/emr/resources/condition/spec.py (1)
validate_verification_status(151-167)care/emr/resources/medication/statement/spec.py (1)
validate_verification_status(60-71)
care/emr/api/viewsets/condition.py (1)
care/emr/api/viewsets/base.py (7)
EMRBaseViewSet(229-271)EMRCreateMixin(70-108)EMRListMixin(111-124)EMRQuestionnaireResponseMixin(274-275)EMRRetrieveMixin(63-67)EMRUpdateMixin(127-179)EMRUpsertMixin(201-226)
🪛 Pylint (3.3.7)
care/emr/resources/device/spec.py
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 71-71: Missing function or method docstring
(C0116)
care/emr/resources/consent/spec.py
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 99-99: Missing function or method docstring
(C0116)
care/emr/resources/encounter/spec.py
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 94-94: Missing function or method docstring
(C0116)
care/emr/resources/medication/statement/spec.py
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 60-60: Missing function or method docstring
(C0116)
care/emr/resources/scheduling/slot/spec.py
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 76-76: Missing function or method docstring
(C0116)
care/emr/api/viewsets/consent.py
[error] 7-14: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/resources/medication/administration/spec.py
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 162-162: Missing function or method docstring
(C0116)
care/emr/tests/test_device_api.py
[convention] 177-177: Missing function or method docstring
(C0116)
care/emr/resources/allergy_intolerance/spec.py
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 81-81: Missing function or method docstring
(C0116)
care/emr/api/viewsets/medication_administration.py
[error] 4-12: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/resources/condition/spec.py
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 151-151: Missing function or method docstring
(C0116)
care/emr/api/viewsets/medication_statement.py
[error] 4-12: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/api/viewsets/medication_request.py
[error] 6-14: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/api/viewsets/device.py
[convention] 54-54: Missing class docstring
(C0115)
care/emr/resources/medication/request/spec.py
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
[convention] 200-200: Missing function or method docstring
(C0116)
care/emr/api/viewsets/condition.py
[error] 7-15: Unable to import 'care.emr.api.viewsets.base'
(E0401)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Test / Test
🔇 Additional comments (36)
care/emr/tests/test_diagnosis_api.py (5)
504-508: LGTM! Explicit status setting improves test reliability.The explicit
verification_status=VerificationStatusChoices.provisional.valueensures the test consistently creates diagnoses that can be updated, preventing any potential flakiness from random status selection.
536-540: Consistent with other update tests.Good pattern consistency with the previous test method.
569-573: Maintaining the established pattern.Follows the same explicit status setting approach.
602-606: Well, at least you're being consistent.Same explicit status setting pattern continues across all update tests.
633-637: Final test method follows suit.Completes the consistent pattern of explicit status setting for update scenarios.
care/emr/tests/test_consent_api.py (2)
223-225: Explicit status setting for update test consistency.Setting
status=ConsentStatusChoices.active.valueensures the consent can be updated in the test, aligning with the new validation rules that prevent updates to "entered_in_error" records.
237-239: Consistent pattern maintained.Follows the same explicit status approach as the previous test method.
care/emr/tests/test_device_api.py (3)
171-171: Explicit device status for update test.Setting
status=DeviceStatusChoices.active.valueensures the device can be updated, preventing test failures due to the new validation rules.
178-178: Maintaining consistency across update tests.Same explicit status approach as the previous test.
187-189: Complete parameter specification for care plan test.Includes both
care_typeand explicitstatussetting for comprehensive test setup.care/emr/resources/device/spec.py (2)
4-5: Required imports for validation functionality.The
model_validatorandValidationInfoimports are necessary for the new status validation logic.🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
70-82: Solid validation logic to enforce business rules.The
validate_statusmethod correctly prevents updates to devices with "entered_in_error" status. The implementation:
- Properly checks for update context
- Validates the existing model object's status
- Raises an appropriate error message
- Follows the established pattern from other resource specs
This enforces the core business rule mentioned in the PR objectives.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 71-71: Missing function or method docstring
(C0116)
care/emr/resources/medication/administration/spec.py (1)
4-5: Import additions look correct.The added imports for
model_validatorandValidationInfoare necessary for the new validation functionality and follow the established import patterns in the codebase.🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/consent/spec.py (1)
6-6: Import addition is appropriate.The
ValidationInfoimport is correctly placed and necessary for the new validator functionality.🧰 Tools
🪛 Pylint (3.3.7)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/encounter/spec.py (1)
5-6: Import changes are correct and necessary.The additions to the pydantic imports and the new
ValidationInfoimport are properly placed and required for the validation functionality.🧰 Tools
🪛 Pylint (3.3.7)
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/tests/test_allergy_intolerance_api.py (1)
544-546: Test data updates are necessary and well-implemented.The explicit setting of
verification_status=VerificationStatusChoices.confirmed.valuein update test methods ensures compatibility with the new validation rules that prevent updates to "entered_in_error" records. I suppose it's fortunate that someone thought to update the tests accordingly.Also applies to: 578-580, 612-614, 645-647, 678-680, 717-717, 763-763
care/emr/resources/scheduling/slot/spec.py (1)
4-5: LGTM! Imports added correctly for the new validation logic.The addition of
model_validatorandValidationInfoimports is necessary for the new status validation functionality.🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/medication/statement/spec.py (1)
4-5: LGTM! Imports properly added for validation functionality.The pydantic imports are correctly added to support the new model-level validation.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/allergy_intolerance/spec.py (1)
4-5: LGTM! Required imports added for validation enhancement.The pydantic imports are properly included to support the new verification status validation.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-4: Unable to import 'pydantic'
(E0401)
[error] 5-5: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/condition/spec.py (1)
5-6: LGTM! Imports added correctly for validation functionality.The necessary pydantic imports are properly included for the new model validation.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
care/emr/resources/medication/request/spec.py (2)
5-6: LGTM: Clean import additions for validation functionality.The imports align perfectly with the validation pattern used throughout the codebase.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 5-5: Unable to import 'pydantic'
(E0401)
[error] 6-6: Unable to import 'pydantic_core.core_schema'
(E0401)
199-211: Solid validation implementation following established patterns.The
validate_verification_statusmethod correctly prevents updates to medication requests marked as "entered_in_error", which is exactly what this PR aims to achieve. The implementation is consistent with similar validators in other resource specs.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 200-200: Missing function or method docstring
(C0116)
care/emr/tests/test_symptom_api.py (5)
506-510: Excellent foresight in test data preparation.Setting
verification_status=VerificationStatusChoices.confirmed.valueensures this test won't inadvertently trip over the new validation rules that prevent updates to "entered_in_error" records. Nicely done.
538-542: Consistent test data management.Same smart approach here - explicitly setting the verification status to a safe value for update operations.
571-575: Maintaining test reliability with proper status setup.Good practice to explicitly set verification status even in permission-denied scenarios to ensure consistent test behavior.
604-608: Thorough consistency in test data preparation.Appreciate the attention to detail in setting verification status across all update test scenarios.
635-639: Complete coverage of test scenario adjustments.Even for closed encounter tests, the verification status is properly set. This demonstrates a comprehensive understanding of the validation changes.
care/emr/api/viewsets/consent.py (2)
7-14: Clean modularization of viewset functionality.The explicit mixin imports provide much clearer visibility into what capabilities this viewset actually uses, rather than inheriting everything from a monolithic base class.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 7-14: Unable to import 'care.emr.api.viewsets.base'
(E0401)
28-37: Well-structured inheritance hierarchy.The explicit mixin composition gives you precise control over the viewset's capabilities. The addition of
EMRUpsertMixinis particularly nice for bulk operations.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 28-28: Missing class docstring
(C0115)
care/emr/api/viewsets/device.py (1)
18-18: Helpful addition of upsert capability.Adding
EMRUpsertMixinprovides convenient bulk create/update functionality to the device viewset.care/emr/api/viewsets/medication_request.py (1)
6-14: Nicely modularized viewset inheritanceThe refactoring to use explicit mixins instead of the monolithic
EMRModelViewSetmakes the capabilities much clearer. I notice the delete functionality is conspicuously absent, which I'm sure aligns perfectly with the PR objectives.Also applies to: 46-53
🧰 Tools
🪛 Pylint (3.3.7)
[error] 6-14: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/api/viewsets/medication_statement.py (1)
4-12: Consistent refactoring pattern maintainedGood to see the same explicit mixin approach here. The inheritance order remains consistent with other viewsets in the PR.
Also applies to: 34-41
🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-12: Unable to import 'care.emr.api.viewsets.base'
(E0401)
care/emr/api/viewsets/condition.py (2)
7-15: Both viewsets updated uniformlyThe refactoring has been applied consistently to both
SymptomViewSetandDiagnosisViewSet. The placement ofValidateEncounterMixinat the beginning of the inheritance chain is appropriate for ensuring validation occurs early in the method resolution order.Also applies to: 71-76, 120-125
🧰 Tools
🪛 Pylint (3.3.7)
[error] 7-15: Unable to import 'care.emr.api.viewsets.base'
(E0401)
71-125:✅ Verification successful
Verify integration with entered_in_error validators
While the viewset refactoring looks solid, just want to make sure the model-level validators mentioned in the PR objectives are properly integrated with these viewsets through the pydantic specs.
Run the following script to verify the validators are in place:
🏁 Script executed:
#!/bin/bash # Description: Check for entered_in_error validators in the condition spec files # Search for validation methods in condition spec ast-grep --pattern 'def validate_verification_status($_) { $$$ }' # Also check for any references to entered_in_error rg -A 5 "entered_in_error" care/emr/resources/condition/Length of output: 957
Sure, let’s peek into the spec to confirm the validator is wired up as expected.
🏁 Script executed:
#!/bin/bash # Show any Pydantic validator on verification_status rg -C5 '@validator\("verification_status"\)' care/emr/resources/condition/spec.py # Show context around entered_in_error checks rg -C5 'entered_in_error' care/emr/resources/condition/spec.pyLength of output: 808
entered_in_error validation already enforced in ConditionSpec
I’ve confirmed in care/emr/resources/condition/spec.py that the update logic checks for
verification_status == VerificationStatusChoices.entered_in_error.value
and raises a ValueError if so. Since your viewsets use ConditionUpdateSpec, this guard will run on all updates. No further integration work needed here.🧰 Tools
🪛 Pylint (3.3.7)
[convention] 95-95: Missing function or method docstring
(C0116)
[convention] 99-99: Missing function or method docstring
(C0116)
[convention] 116-116: Missing class docstring
(C0115)
care/emr/api/viewsets/medication_administration.py (2)
4-12: LGTM! Explicit mixins improve code clarity.The refactoring from a monolithic base class to explicit mixins is a solid architectural improvement. It makes the viewset's capabilities much more transparent, though I suppose someone could argue that having everything spelled out like this is a bit... verbose compared to the previous approach.
🧰 Tools
🪛 Pylint (3.3.7)
[error] 4-12: Unable to import 'care.emr.api.viewsets.base'
(E0401)
35-44:✅ Verification successful
Verify the mixin order for proper method resolution.
The explicit mixin inheritance looks good and follows composition over inheritance principles. However, please ensure that the order of mixins is correct for Django's method resolution order, particularly with
EncounterBasedAuthorizationBasefirst andEMRBaseViewSettoward the end.
🏁 Script executed:
#!/bin/bash # Description: Verify that other viewsets in the EMR module follow the same mixin pattern # Expected: Similar refactoring patterns across related viewsets echo "Checking for similar refactoring patterns in related EMR viewsets..." rg -A 10 "class.*ViewSet" care/emr/api/viewsets/ | grep -E "(EMRCreateMixin|EMRUpdateMixin|EMRBaseViewSet)" -A 5 -B 5Length of output: 17323
Mixin order is consistent across EMR viewsets
I verified that MedicationAdministrationViewSet follows the same mixin sequence as other encounter-based viewsets (e.g. medication_statement.py, medication_request.py):
EncounterBasedAuthorizationBase → EMRQuestionnaireResponseMixin → EMRCreateMixin → EMRRetrieveMixin → EMRUpdateMixin → EMRListMixin → EMRBaseViewSet → EMRUpsertMixinNo adjustments required.
🧰 Tools
🪛 Pylint (3.3.7)
[convention] 35-35: Missing class docstring
(C0115)
[refactor] 35-35: Too few public methods (1/2)
(R0903)
Proposed Changes
Associated Issue
Architecture changes
Merge Checklist
/docsOnly 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
New Features
Refactor
Bug Fixes
Tests