Test cases for the roles.py

[nandkishorr]

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 API tests for role management, covering listing, retrieval, creation, updating, and deletion of roles with various user permissions.
  • Ensured proper enforcement of permissions and data integrity for role-related API operations.

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

Adds a new Django REST Framework test module that validates role-management APIs (list, retrieve, create, update, delete), exercising permission enforcement, validation/error paths, and DB state transitions for regular users and superusers.

Changes

Cohort / File(s)	Change Summary
Role API tests care/emr/tests/test_roles_api.py	Added a comprehensive test suite (RoleApiTestCase) covering role listing and filtering, retrieval (including permissions), superuser-only create/update/delete flows, validation errors (duplicate/empty name, missing permissions), system-role protections, and DB cascade checks for RolePermission records.

Sequence Diagram(s)

sequenceDiagram
    participant Client
    participant RolesAPI as "Roles API"
    participant DB as "Database"

    Client->>RolesAPI: GET /roles/ (auth)
    RolesAPI->>DB: query roles
    DB-->>RolesAPI: roles list
    RolesAPI-->>Client: 200 OK + list

    Client->>RolesAPI: GET /roles/{id}/ (auth)
    RolesAPI->>DB: retrieve role (+permissions)
    DB-->>RolesAPI: role data
    RolesAPI-->>Client: 200 OK + role

    opt write operations (superuser)
        Client->>RolesAPI: POST /roles/ (payload)
        RolesAPI->>DB: validate + create role & RolePermission
        DB-->>RolesAPI: created
        RolesAPI-->>Client: 201 Created

        Client->>RolesAPI: PATCH /roles/{id}/ (payload)
        RolesAPI->>DB: validate + update role/permissions (reject system-role edits)
        DB-->>RolesAPI: updated
        RolesAPI-->>Client: 200 OK

        Client->>RolesAPI: DELETE /roles/{id}/
        RolesAPI->>DB: check not system role -> delete role & cascade RolePermission
        DB-->>RolesAPI: deleted
        RolesAPI-->>Client: 204 No Content
    end

    alt non-superuser attempts write
        Client->>RolesAPI: POST/PATCH/DELETE ...
        RolesAPI-->>Client: 403 Forbidden
    end

    alt invalid input (duplicates/missing perms/empty name)
        RolesAPI-->>Client: 400 Bad Request (validation errors)
    end

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• ohcnetwork/care#3196 — Enforces "at least one permission" and permission deduplication in role create/update logic; directly related to tests asserting permission presence and handling.

Suggested reviewers

• vigneshhari

Poem

Roles and tests align in line-by-line delight,
Superusers press keys, while regulars watch polite.
Assertions whisper firmly, "No system-role rewrite,"
DB checks nod off quietly — everything's airtight.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description only includes the merge checklist section and omits the mandatory "Proposed Changes" and "Associated Issue" headings from the repository’s template, leaving out any summary of what the PR actually does or the issue it addresses.	Please add a ## Proposed Changes section that briefly outlines the new test suite and an ### Associated Issue section linking the relevant issue, and include any architecture changes if applicable to fully comply with the template.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title succinctly and accurately describes the core change of the pull request, which is the addition of test cases for the roles API, making it immediately clear to reviewers what the primary intent of the PR is.
Docstring Coverage	✅ Passed	Docstring coverage is 86.36% which is sufficient. The required threshold is 80.00%.

✨ Finishing touches

• [ ] 📝 Generate Docstrings

🧪 Generate unit tests

• [ ] Create PR with unit tests
• [ ] Post copyable unit tests in a comment
• [ ] Commit unit tests in branch nandkishor/test/feat/roles

---

📜 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 f3381a2e6e4798b64758525c2ccedb02e92ebb18 and fd66d53bfa1d9e46c8955b550997171c4ab67bba.

📒 Files selected for processing (1)

• care/emr/tests/test_roles_api.py (1 hunks)

🧰 Additional context used

📓 Path-based instructions (2)

**/*.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/tests/test_roles_api.py

**/tests/**/*.py

📄 CodeRabbit inference engine (.cursorrules)

Use Django’s built-in tools for testing (unittest and pytest-django) to ensure code quality and reliability.

Files:

• care/emr/tests/test_roles_api.py

🧬 Code graph analysis (1)

care/emr/tests/test_roles_api.py (3)

care/security/models/permission.py (1)

• PermissionModel (6-18)

care/security/models/role.py (2)

• RoleModel (14-87)
• RolePermission (90-101)

care/utils/tests/base.py (2)

• CareAPITestBase (22-128)
• create_facility_organization (46-49)

⏰ 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/tests/test_roles_api.py (1)

24-29: Please restore the action/resource split in the permission fixture.

You're still using the resource segment for both the action and the resource, so the fixture creates names/descriptions like “Can Patient PATIENT”. That quietly diverges from how real permissions look and can mask regressions in any validation that leans on the action verb. Please keep the context uppercase, but pull the action from index 1 so the metadata mirrors production.

         for slug in valid_slugs:
-            context = slug.split("_")[2].upper()
+            action, resource = slug.split("_")[1:3]
+            context = resource.upper()
             perm = baker.make(
                 PermissionModel,
                 slug=slug,
-                name=f"Can {slug.split('_')[2].capitalize()} {context.capitalize()}",
-                description=f"Permission to {slug.split('_')[2]} {context.lower()}",
+                name=f"Can {action.capitalize()} {resource.capitalize()}",
+                description=f"Permission to {action} {resource.lower()}",
                 context=context,
             )

---

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

• X
• Mastodon
• Reddit
• LinkedIn

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

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 53.87%. Comparing base (fefde08) to head (c8ac12f). :warning: Report is 2 commits behind head on develop.

Additional details and impacted files

@@           Coverage Diff            @@
##           develop    #3067   +/-   ##
========================================
  Coverage    53.87%   53.87%           
========================================
  Files          434      434           
  Lines        19683    19683           
  Branches      2129     2129           
========================================
  Hits         10604    10604           
  Misses        9051     9051           
  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.

[praffq]

Go through code rabbit reviews, those are valid points

[praffq]

Add more testcase here based on specs - https://github.com/ohcnetwork/care/blob/develop/care/emr/resources/role/spec.py#L31C5-L57C20

Add test for each validation error