search

LinuxForHealth / FHIR

The LinuxForHealth FHIR® Server and related projects
https://linuxforhealth.github.io/FHIR
Apache License 2.0
326 stars 157 forks source link

Notification Service operationType should match Audit action #2988

Closed d0roppe closed 2 years ago

d0roppe commented 2 years ago

Is your feature request related to a problem? Please describe. For the Notification Service: the operationType should match Audit action. So when a resource has been deleted, and then the same ID is used again for a create. the operationType should be create and not update.

Describe the solution you'd like So if the resource has been deleted and then you create a new resource with the same ID, it is a create. not an update.

Describe alternatives you've considered So there is another way of looking at this. And in the users guide it states that there are only 2 valid values for operationType, Create and Update. if you only use those two, then I would say if the resource ID has never been used before then it is a create and all subsequent uses are updates. But currently the delete of a resource has a operationType of delete.

So maybe need a separate issue for this but will leave it here if only create and update are valid for operationType:

If delete is valid for the operationType, the Users guide should be updated (section 4.2.1). And the delete entry in the Notification Service should include the /_history/ of the resource to match the create and update operationType.

lmsurpre commented 2 years ago

Relates to https://github.com/ibm/fhir/issues/2471, specifically the comment at https://github.com/IBM/FHIR/pull/2967#issuecomment-965809276

Its potentially broader than just the notification message, because the notifications are emitted from the PersistenceEvent via our FHIRPersistenceInterceptor framework. So, as indicated in that comment, the key thing is deciding whether we want to treat an "undelete" like a create from start to finish (i.e. call beforeCreate and afterCreate instead of beforeUpdate and afterUpdate). We do something similar for create-on-update interactions now.

lmsurpre commented 2 years ago

  1. verify whether we call beforeCreate and afterCreate on persistence interceptors and make sure thats well-documented in the FHIRPersistenceInterceptor interface. hint: check unit tests

  2. verify whether we call the event a 'create' or an 'update' in our Notification service and document it...I assume it matches the answer to number 1.

  3. verify what we get back from the history api (instance-level and whole-system level)...is it a 200 or a 201? we store a change_type column, but need to check that in the undelete case

lmsurpre commented 2 years ago

for number 3, our current history impl shows a response status of "200 OK" in the undelete case. this is inconsistent with the actual (correct) response code, but we think that is acceptable for now. we should open a separate low-priority issue for this.

lmsurpre commented 2 years ago

Now that we implemented https://github.com/IBM/FHIR/issues/3293 I think the event is being emitted as a 'create' instead of an 'update'.

I opened https://github.com/IBM/FHIR/issues/3507 for the fact that the _history API still indicates a status code of 200 (instead of 201).

d0roppe commented 2 years ago

Verified that now a resource that has been deleted and then the ID is used again, the fhir-server reports it as a create.