Skip to content

Task/npa 6176/remove provision actor - #391

Open
hatwell wants to merge 16 commits into
masterfrom
task/NPA-6176/remove-provision-actor
Open

hatwell wants to merge 16 commits into
masterfrom
task/NPA-6176/remove-provision-actor

Conversation

@hatwell

@hatwell hatwell commented Sep 14, 2026

Copy link
Copy Markdown
Contributor

Pull Request

🧾 Ticket Link

https://nhsd-jira.digital.nhs.uk/browse/NPA-6176


📄 Description/Summary of Changes

  • Removes provision.actor from spec and postman collections as it it is no longer valid.
  • Makes policyRule a required field in Consent schema
  • Fixed linting/typing/SonarQube errors.
  • The changes for the ticket are in the spec and postman collections, all others are typing / linting fixes

🧪 Developer Testing Carried Out

  • Unit, schema and postman collection tests all pass.

📋 PR Principles

  • Keep PRs Small and Focused: Ensure the PR addresses a single task or feature to make it easier to review.
  • Multiple PRs for one Ticket: When splitting work into multiple PRs, clearly describe what this PR addresses and outline the remaining work to complete the ticket.
  • Ensure Tests Are Included: Add or update unit, integration, or end-to-end tests to cover the changes made.
  • Follow Coding Standards: Ensure the code adheres to the team's coding guidelines and best practices.
  • Resolve Comments Promptly: If you raise a comment, ensure you follow up and resolve it before approving the PR to maintain clarity and ensure comments are addressed.
  • Foster Learning: PR reviews are an opportunity to share knowledge, provide constructive feedback, and encourage a collaborative environment.

🏷️ Naming Conventions Reminder

Please ensure the following naming conventions are followed:

  • PR title follows the format: NPA-XXXX: <short-description>
  • Branch name follows the convention: <type>/NPA-XXXX/<short-description>
  • Commit messages follow the template: NPA-XXXX: <short-description>

Copilot AI lite review requested due to automatic review settings September 14, 2026 10:18

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The Postman payload violates the published Consent schema, and the shared mutable default remains unresolved.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Updates the Consent API specification and sandbox implementation for NPA-6176.

Changes:

  • Removes deprecated provision.actor.
  • Makes policyRule required.
  • Updates response typing, defaults, and Postman examples.
File summaries
File Summary
specification/validated-relationships-service-api.yaml Updates Consent schema and patch paths.
sandbox/api/utils.py Updates response annotations and helper defaults. nit (3 votes): stale return documentation. moderate (2 votes): shared mutable dictionary default.
sandbox/api/post_questionnaire_response.py Updates response typing.
sandbox/api/post_consent.py Updates response typing.
sandbox/api/patch_consent.py Updates response handling and annotations.
sandbox/api/get_related_person.py Updates response typing.
sandbox/api/get_questionnaire_response_by_path_id.py Updates response typing.
sandbox/api/get_consent.py Updates response typing.
sandbox/api/get_consent_by_id.py Updates response typing.
sandbox/api/app.py Updates route response annotations.
postman/validated_relationship_service.sandbox.postman_collection.json Removes deprecated actor data. critical (2 votes): policyRule is sent as an array instead of the schema-defined object.
Review details

Suppressed comments (1)

sandbox/api/utils.py:106

  • The annotation now says Optional[Response], but this Returns entry still documents an Optional[tuple] and a status-code tuple. Update the docstring so callers are not given the old return contract.
def check_for_get_consent_errors(request: Request) -> Optional[Response]:
  • Files reviewed: 11/12 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread postman/validated_relationship_service.sandbox.postman_collection.json Outdated
Comment thread sandbox/api/utils.py Outdated
Comment thread sandbox/api/utils.py
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants