medic / cht-core

The CHT Core Framework makes it faster to build responsive, offline-first digital health apps that equip health workers to provide better care in their communities. It is a central resource of the Community Health Toolkit.
https://communityhealthtoolkit.org
GNU Affero General Public License v3.0
439 stars 210 forks source link

Auditing not happening #1033

Closed garethbowen closed 9 years ago

garethbowen commented 9 years ago
  1. Submit a report and wait for sentinel to process it
  2. Check the audit record by going to /_utils/database.html?medic/_design/medic/_view/audit_records_by_doc . One of the result rows should be recording changes to your document. This looks like it's working but it's actually sentinel logging the changes and not api, which means certain information is missing. To prove this select your document and check the history - the first history element should be a create and have your user name.
  3. To test further, go back to the webapp and click Verify.
  4. Check the history again and you should see another history element recording the verify.
ghost commented 9 years ago

Hey Alex, would you mind taking a stab at acceptance testing this? Just looking for confirmation that it's fixed in the latest development build. Thanks!

alxndrsn commented 9 years ago

@browndav @garethbowen Not sure how to test this:

  1. as I'm not familiar with this functionality, it's not clear if the steps above are the observed (buggy) behaviour or expected (good) behaviour
  2. clicking Verify has no visible effect in the webapp or in the couch audit_record doc

My couch seems to be processing views, so I will give this another go later if I've got no response by then.

alxndrsn commented 9 years ago

Have tested this again:

  1. add a report
  2. observe that it's first history item has user: null
  3. clicking Verify has no visible effect in the webapp, but does add a new item to doc.history, e.g.:
   {
       "action": "update",
       "user": "admin",
       "timestamp": "2015-06-29T15:45:23.985Z",
       "doc": {
           "_id": "9f73e0868732c4bd48da6dfe2900c6bc",
           "_rev": "current",
           "type": "data_record",
           "from": "+13125551212",
           "form": "P",
           "errors": [
           ],
           "tasks": [
               {
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Thank you for registering MR_TESTER. Their pregnancy ID is 04145, and EDD is Sun, Mar 13th, 2016",
                           "uuid": "a076443e-a92f-49af-906a-e69feb521170"
                       }
                   ],
                   "state": "pending",
                   "state_history": [
                       {
                           "state": "pending",
                           "timestamp": "2015-06-29T15:41:37.938Z"
                       }
                   ]
               }
           ],
           "fields": {
               "last_menstrual_period": 3,
               "patient_name": "MR_TESTER"
           },
           "reported_date": 1435592497234,
           "sms_message": {
               "message_id": "5916",
               "sent_timestamp": "1435592497234",
               "message": "1!P!3#MR_TESTER",
               "from": "+13125551212",
               "type": "sms_message",
               "form": "P",
               "locale": "en"
           },
           "transitions": {
               "default_responses": {
                   "last_rev": 2,
                   "seq": 15081,
                   "ok": true
               },
               "registration": {
                   "last_rev": 2,
                   "seq": 15081,
                   "ok": true
               }
           },
           "patient_id": "04145",
           "lmp_date": "2015-06-06T22:00:00.000Z",
           "expected_date": "2016-03-12T23:00:00.000Z",
           "scheduled_tasks": [
               {
                   "due": "2015-08-31T07:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Please remind MR_TESTER (04145) to visit health facility for ANC visit this week. When she does let us know with 'V 04145'. Thanks!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.926Z"
                       }
                   ],
                   "group": 1,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2015-09-07T08:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Did MR_TESTER attend her ANC visit? When she does, respond with 'V 04145'. Thank you!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.928Z"
                       }
                   ],
                   "group": 1,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2015-11-23T08:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Please remind MR_TESTER (04145) to visit health facility for ANC visit this week. When she does let us know with 'V 04145'. Thanks!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.928Z"
                       }
                   ],
                   "group": 2,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2015-11-30T09:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Did MR_TESTER attend her ANC visit? When she does, respond with 'V 04145'. Thank you!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.929Z"
                       }
                   ],
                   "group": 2,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-01-18T08:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Please remind MR_TESTER (04145) to visit health facility for ANC visit this week. When she does let us know with 'V 04145'. Thanks!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.930Z"
                       }
                   ],
                   "group": 3,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-01-25T09:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Did MR_TESTER attend her ANC visit? When she does, respond with 'V 04145'. Thank you!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.931Z"
                       }
                   ],
                   "group": 3,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-02-15T08:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Please remind MR_TESTER (04145) to visit health facility for ANC visit this week. When she does let us know with 'V 04145'. Thanks!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.932Z"
                       }
                   ],
                   "group": 4,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-02-22T09:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Did MR_TESTER attend her ANC visit? When she does, respond with 'V 04145'. Thank you!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.932Z"
                       }
                   ],
                   "group": 4,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-03-14T08:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Please remind MR_TESTER (04145) to visit health facility for ANC visit this week. When she does let us know with 'V 04145'. Thanks!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.932Z"
                       }
                   ],
                   "group": 5,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-03-21T09:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Did MR_TESTER attend her ANC visit? When she does, respond with 'V 04145'. Thank you!"
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.933Z"
                       }
                   ],
                   "group": 5,
                   "type": "ANC Reminders LMP"
               },
               {
                   "due": "2016-04-18T09:00:00.000Z",
                   "messages": [
                       {
                           "to": "+13125551212",
                           "message": "Where did MR_TESTER deliver? Respond with 'D F 04145' for facility birth, 'D S 04145' for home birth with SBA, 'D NS 04145' for home birth."
                       }
                   ],
                   "state": "scheduled",
                   "state_history": [
                       {
                           "state": "scheduled",
                           "timestamp": "2015-06-29T15:41:37.934Z"
                       }
                   ],
                   "group": 6,
                   "type": "ANC Reminders LMP"
               }
           ],
           "read": [
               "admin"
           ],
           "verified": true
       }
   }
garethbowen commented 9 years ago

@alxndrsn What was happening before was no audit doc was being created at all. If there is a new history item then it's an improvement!

The way auditing works is...

  1. Every request goes through api
  2. Certain requests (in particular PUT, POST, DELETE) get proxied through an auditing service and the rest get sent straight to couchdb
  3. The auditing service looks up the audit doc related to the doc being changed. If it doesn't exist then it creates one.
  4. Then it adds a history element with the doc, author, date, etc which is what you're seeing in your previous comment.

It looks like it's working to me, but then, I wrote it so I'm not the right guy to test it. Spend a little time making sure we're capturing everything you think needs to be audited, and if it makes sense to you then you can call it done.

clicking Verify has no visible effect in the webapp

This is a separate issue, which I think I have now fixed.

alxndrsn commented 9 years ago

Creating a report

Steps taken

  1. add a report from the "Submit Report" non-modal dialog
  2. click on the new report to find out its ID

Behaviour observed

3 audit records are created:

  1. action:"create", user:null
  2. action:"update", user:"admin", transitions:...
  3. action:"update", user:"admin", transitions:..., read:["admin"]

Queries

  1. Is it expected that 3 records are created for a report submission?
  2. Is it expected that the first report should have null user?
alxndrsn commented 9 years ago

Viewing a report

Steps taken

After the previous test, I selected a different report, and then reselected the newly-created report. I did this repeatedly, and then hard-refreshed the page with the new report selected.

Behaviour Observed

No new read entries are added to the audit for this report.

Queries

Do we expect to see new audit records for every read? If not, what is the expected behaviour?

alxndrsn commented 9 years ago

Editing a report

Steps taken

  1. select report
  2. click Edit
  3. select Emmitt Engman
  4. click Submit

Behaviour observed

Two new audit records are created:

  1. action:"update", doc.errors:[], doc.tasks:[], doc.contact:emmitt
  2. action:"update", doc.errors:undefined, doc.tasks:undefined, doc.contact:emmitt

Queries

  1. Why are two audit records created for one update?
  2. what is the significance of empty tasks and errors arrays, versus these values being undefined?
garethbowen commented 9 years ago

Viewing a report: we don't current auditing viewing, so this is as expected.

The other two are behaving as expected I think. Extra records are created when sentinel updates the file so I suspect that's what happening here.

alxndrsn commented 9 years ago

If we don't audit viewing, then what's the action:"update", user:"admin", transitions:..., read:["admin"] update for when creating?

garethbowen commented 9 years ago

The read part is actually on the doc, not the audit record. That's how we show a bold item in the UI when the current user hasn't read the report yet. It's not protected so can't be relied on for auditing.

alxndrsn commented 9 years ago

Makes sense - thanks for clearing that up. Seems reasonable to close this.