Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Notification Service operationType should match Audit action #2988

Closed
d0roppe opened this issue Nov 15, 2021 · 5 comments
Closed

Notification Service operationType should match Audit action #2988

d0roppe opened this issue Nov 15, 2021 · 5 comments
Labels
audit P2 Priority 2 - Should Have

Comments

@d0roppe
Copy link
Collaborator

d0roppe commented Nov 15, 2021

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
Copy link
Member

lmsurpre commented Nov 15, 2021

Relates to #2471, specifically the comment at #2967 (comment)

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.

@prb112 prb112 added the audit label Nov 15, 2021
@lmsurpre
Copy link
Member

lmsurpre commented Jan 24, 2022

  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
Copy link
Member

lmsurpre commented Jan 24, 2022

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
Copy link
Member

Now that we implemented #3293 I think the event is being emitted as a 'create' instead of an 'update'.

I opened #3507 for the fact that the _history API still indicates a status code of 200 (instead of 201).

@d0roppe
Copy link
Collaborator Author

d0roppe commented Mar 28, 2022

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

@d0roppe d0roppe closed this as completed Mar 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
audit P2 Priority 2 - Should Have
Projects
None yet
Development

No branches or pull requests

3 participants