cds-hooks / docs

CDS Hooks website & specification
http://cds-hooks.org
Apache License 2.0
166 stars 61 forks source link

Fix delete; clarify extension #554

Closed mattvarghese closed 3 years ago

mattvarghese commented 3 years ago
dennispatterson commented 3 years ago

@ssmillard we had a bit of discussion about the extensions topic and I dug up https://github.com/cds-hooks/docs/issues/133 and https://github.com/cds-hooks/docs/issues/89, where there was a decision made to handle it this way (for both extensions and prefetch) vs the uri/value pairs mechanism.

isaacvetter commented 3 years ago

Related: http://hl7.org/fhir/us/davinci-crd/2019May/hooks.html#propose-alternate-request

The issue with:

  1. Deleting a resource should use a resource with just resourceType and id

Is that the same element name (resource) using different data types between delete vs create/update is inelegent depending upon the dev stack.

An alternative to the proposal would be to introduce a new element name (e.g. resourceLocation) to be used for deletes, which could also enable a transition path, and cds service could even provide both.

If we make any changes, we should ensure that CRD stays in sync as well.

isaacvetter commented 3 years ago

@dennispatterson , @brynrhodes and I talked through this issue.

Ultimately, it is awkward for developers to treat the same resource element as either a resource "location" (aka ProcedureRequest/123) or as a full resource. We believe that delete suggestions are not yet widely implemented. Specific to delete suggestions, from a developer perspective, constructing a shell resource withonly a resource type and id

`resourceId` | CONDITIONAL | *string* | A relative reference to the relevant resource in the suggestion. SHOULD be provided for delete suggestions. 
`resource` | OPTIONAL | *object* | A FHIR resource. When the `type` attribute is `create`, the `resource` attribute SHALL contain a new FHIR resource to be created.  For `update`, this holds the updated resource in its entirety and not just the changed fields. Use of this field to communicate a string of a FHIR id for delete suggestions is DEPRECATED and `resourceId` SHOULD be used instead.

For delete, this SHALL be the id of the resource to remove. In hooks where only one "content" resource is ever relevant, this attribute MAY be omitted for delete action types only.

This issue is also related to (and perhaps resolves?): https://github.com/cds-hooks/docs/issues/400

mattvarghese commented 3 years ago

Hey Folks - are we all agreed on this change? Can we move ahead with this pull request?