Approaches for linking form instance changes to individuals

Development
#27

The second option seems like the best one to me as well but I'm not sure if I understand everything...
We have many comments here but no one approach (most of them come from the option 2)
.
We should ask for the user id every time a form is started//edited and write it to audit.csv file and that seems fine, but what about those comments... I saw the sample @LN attached https://docs.google.com/spreadsheets/d/1DqRHPKxrB1qZPp8rQtwky-Lm4WASTK33iVUQEKSJXtw/edit#gid=0

It assumes that a user should also add a comment just when the form is opened and it's a general comment. Does that make sense to ask for such a general comment before editing anything? I think a user might don't know what he is going to change at that point.
If we want to have a general comment for the entire form it should be displayed when the form is being exited.

Asking for a comment, for each edited question (if a question requires it) seems fine but in that case it should ask for a comment, only not user id becasue it doesn't make sense to duplicate it right?

I think we can implement it in two separate pr (two features):
First of all we should add an option to ask for the user id and a generall comment.
Then we can add an option to ask for comments for each separate questions.

#28

Just to add (although we have incentive enough as is) that we are getting fairly serious interest from major players in other serious epidemic diseases to use ODK if we can get the GCP compliance issues sorted - of which this audit trail thing is the key step.

1 Like
#29

FYI I think this would be inadequate; I can certainly see any change to a response - eg from "Fail" to "Pass" - as requiring both a timestamp and an optional (or mandatory?) comment. And in the general case potentially a signature too (@dr_michaelmarks?)

I think a fully general-purpose design may well need to fulfill:

  • allow for comment for every change (vs a single comment at beginning/end, or comment only for null-to-nonnull). And we probably want something to flag whether a comment is mandatory or not.
  • timestamp every change
  • allow for 'signature' (or otherwise initial) for every change (vs just a single signature at beginning). And likewise flag if mandatory or not.

Question is, we can we scope it back initially? eg only require signature at beginning (which might determine whether this is 1 PR or more...)

I think we can implement it in two separate pr (two features):
First of all we should add an option to ask for the user id and a generall comment.
Then we can add an option to ask for comments for each separate questions.

2 Likes
#30

@LN and I spent some time iterating on a proposal built around Option 2 and we've published it at https://docs.google.com/document/d/1NyBFhASCOAqOSmz70diQObKpymVE59_MuwjOUPCKlR8.

We've put it in a Google Doc so you can easily leave comments. We'd love your comments or objections over the next 5 days. @TAB please also review!

#31

Some thoughts/straw man (google doc comments getting rather long...)

  • when first start up or reopen a saved form, the user has to pseudo-'login' by providing their name/initial in a popup (if odk:track-user set)

  • if odk:track-change-reason (per form or per question?) is set then any user change to an existing non-null value - including initially non-null defaults in the instance XML, or going back and changing a response - brings up mandatory popup to enter reason (this would have to occur after constraint checking). By implication, this change is tagged against the current 'logged in' user, since otherwise Collect has no notion of internal sessions. Note, this (also) occurs when first filling in the form [why? 'cause I dont see a compelling reason to introduce and track a distinct different state to distinguish between before and after saved form state... but perhaps that's open to debate]

  • best practice is that you must 'save' forms before handing the device to another person, who will then have to 'login' using their id/initial should they reopen the form and edit it. Realistically I dont think there's a lot we can do to enforce a re-login when you hand the device to someone else, so it'll just have to be best-practice...

  • any and all changes timestamped, with the initial answering of a question (ie null to non-null) suitably tagged as such, since these wont have a change reason (as described above)

1 Like
#32

Thanks so much for all the thoughtful responses, everyone. @yanokwa caught me up on the TSC discussions.

The spec document has been edited to reflect the various feedback received. It's very similar to @Xiphware's strawman above.

You can reach the diff from the last version from File > Version History.

Some things to note:

  • asking for user ID and comments have been fully disaggregated as @Grzesiek2010, @Xiphware and others recommended. They are also completely independent from change tracking. This enables various scenarios such as auditing the identity of users navigating the form without including potentially sensitive form data in the log or using reasons for change as "notes to self" in a one-person data collection context.
  • the ODK XForms spec is deliberately agnostic to what the user identifier is and how it is obtained.
ODK TSC 1 Call - 2019-07-24
#33

We've gotten feedback on the spec from the research facilitation team at LSHTM and the takeaway is..

The implementation we've specified, paired with appropriate procedures, could be made compliant with good clinical practice (and thus pass internal review boards). LSHTM will put together some procedures about how ODK could be used in a way that achieves all of this, but that shouldn't be a blocker.

All this to say, I think this should settle @TSC-1's concerns. I would request that TSC members (especially @aurdipas, @Xiphware, @martijnr) please review the updated specification at https://docs.google.com/document/d/1NyBFhASCOAqOSmz70diQObKpymVE59_MuwjOUPCKlR8 and leave comments. It'd be great to get approval on this in the next two weeks!

1 Like
#34

Or slightly less... I'll put it on the agenda for next TSC call to discuss and (hopefully) approve! :slight_smile:

3 Likes
#35

Some additional thoughts after last week's TSC discussion...

One of the issues raised was around when to start tracking these changes; specifically you may not want to have to enter change reasons when initially filling in a form - which will mostly be the "blank to non-blank" scenario. Instead, you mostly want to track changes after initially filling in the form, and passing it off to someone else, or re-visiting it at a later time, or after a 'save' operation [although strictly speaking what constitutes 'saving' a form isnt particularly well defined in the spec...]

But then perhaps some questions are prefilled with defaults, in which case perhaps you do in fact want to track these change in the initial pass? @tomsmyth suggested perhaps we therefore need additional flags to indicate when this feature should kick in: "always", "after-save", or "after-finalize", etc...

My thought was, perhaps for an initial pass of this feature, to perhaps simply if odk:track-changes-reason = “true” then whenever said form is opened - either first time or re-opened - to popup prompt like "Do you wish to track all changes?" and if so have them enter their userid (to be used to tag these changes in the audit log). Although this wont enforce track changes - but instead defers the decision to the user - without a formal 'login'/authorization procedure there isnt a particularly reliable means to determine that a form has actually been passed on to someone else (hence need to start logging their changes) that doesn't effectively rely on the user voluntarily specifying it!

[aside, this would also handle the case when there is no save/validate/finalize step, as in my particular situation]

#36

I'm not strongly opposed to this but I do think it adds undesirable overhead to the user experience. I do feel pretty strongly that if we go this direction, the order should be flipped. That is, the user identifier should always be requested on open no matter what. Then the answer to this question can also be logged and associated with the user who made the decision.

I still think that always tracking changes from non-blank values (including defaults) and never tracking changes from blank values (including values that were set, cleared, then set again), is easy to explain and sufficient but I certainly could be convinced otherwise.

I commented on the doc but it may be easier to respond to different parts of my reasoning here.

Perhaps it would be slightly nicer to have a "pristine state" concept as @ggalmazor alluded to. That way a change from non-pristine blank to non-blank would get a reason for change. But I really do think this situation is rare. The added complexity of keeping a pristine flag for each field seems too high for the benefit it brings.

For non-pristine blank to non-blank to happen, the user would have to blank out a value, navigate away from the field (e.g. by tabbing in Enketo or by swiping in Collect) and then come back to the field to write a new value. This is different from selecting a field with a value in it and changing its value without navigating away (this would be tracked).

The paper equivalent would be crossing out a previously-entered value, doing other things, and then entering a value again. Generally I think that would be given a single explanation. For those who have used such protocols on paper, what do you think?

There is long-press to clear in Collect and the possibility of clearing certain question types by clicking a trash icon in Enketo. In those cases, I think it's reasonable to expect that the reason for change when the value is blanked would be an explanation for why the value is being changed to a new one. I think it would be strange for the user to be prompted again when setting the value.

For those of you who think this is common, could you say more about when it happens?

#37

Some of this may depend on when the particular widget in question 'loses focus' and initiates a refresh. For example, with a slider widget, pretty much every time you move the slider it can (should? and does in my case...) update the instance XML and generate a re-calc cycle, even though the user is still techncially 'focued' on that widget/question (you dont really know when the user has stopped moving the slider...they may pause and wait to see what happens). Whereas, primarily for efficiency, you dont really want to fire a re-calc every time you are typing in text, so you probably dont want to update the instance XML/re-calc till they've explicitly "navigated away" from your text question. More specifically, with a text entry widget, by virtue of how its implemented, yes you can delete the existing value entirely (non-blank to blank) and enter something else (blank to non-blank), and from the outside its all just a single operation. But potentially for other widgets - eg those that update continuously - it may be more problematic to determine when the user is 'done' (by which to determine a value change vs blank to non-blank vs non-blank to blank...). It feels like this behavior needs to be intimately (and explicitly?) tied to when precisely the actual instance XML gets updated; ie xforms-value-changed event.

One of the (seemingly?) unresolved questions is WHEN exactly is this audit change logged. It seems like this has to be scoped within WHEN exactly (and under what exact circumstances) does a xforms-value-changed event take place? I'm not trying to assert anything one way or the other, but it does feels like that is the case... I'd appreciate other's thoughts!

1 Like
#38

That sounds reasonable. If (and that's a big IF which still needs to be decided upon...) we ultimately - and effectively - defer turning audit change tracking On/Off to the user, then requiring them to enter their id whenever odk:track-changes-reason is enabled seems appropriate, simply to be able to accurately log them disabling it.

1 Like
#39

Thanks so much to all who have contributed to improving this spec. @yanokwa has invited me to be on the TSC call tomorrow. I can be there about 15 minutes in.

I took a moment to listen to the last call. I should have done that before responding. Luckily what I wrote in my last comment is still relevant but I realize it may need more context. First, thanks to @aurdipas for bringing up the question about the non-blank to blank to non-blank case and to @adam.butler for the explanation of the intended behavior in the current spec draft.

Why not ask for change reasons only on form re-open

An earlier draft of the spec proposed asking for change reasons on form re-open but as mentioned by @tomsmyth in the call, it’s common to partially fill a form and to complete it on re-open. It doesn’t make sense to ask for change reasons then.

Also, asking for change reasons any time a non-blank value is changed better matches paper. For example, if I write down a child’s age as 12 and then I need to cross it out to write 13, I would need to initial and explain this because it was written in ink. This is the case even if I immediately catch the mistake and make the fix.

Possible strategies to overcome issues with asking for change reasons only on form re-open

The big downside of letting the form designer choose when to ask for change reasons (always, after save, after finalize…) as @tomsmyth suggested or leaving it up to the enumerator as @Xiphware described is that both require more training and leave more room for enumerator error. For example, if the form designer can choose when change reasons are requested, enumerators have to be trained on when to save vs. when to finalize and have to be given instructions on what to do if they do the wrong one.

In the case of the enumerator deciding, another downside is that it leaves the door open for both accidental and deliberate opt-out.

Why tracking pristine status doesn’t seem worth it

On the call, @martijnr described requesting change reasons when non-blank values are changed as a crude stand-in for requesting change reasons when a previously-set value is changed. That’s correct, though it’s not clear to me that requesting reasons for changes to previously-set value is much better.

Tracking whether each field has ever been set (“pristine status”) is possible. Then explanations for changes could be requested when changes are made to non-pristine values regardless of whether the new value is blank or not.

I believe this would lead to the following two differences when compared to what is currently written in the spec:

  • Changes to default values would not require explanations because they are pristine
  • In a case where the user blanks a value and then sets it again, explanations would be requested both when blanking the value and when it is set again (even if immediately after).

I’m not entirely sure that those are improvements. In the default case, I can see it either way. In the blanked to non-blank case, requesting two reasons (one for clearing the value and one for re-entering a value) seems like it would generally be redundant.

Tracking pristine status would add complexity and possibly have performance implications. I think it would need to provide clear user benefit for it to be worthwhile.

Am I underestimating the importance of the blanked to non-blank case?

See my previous post for more on why I think the blanked to non-blank case is uncommon and above for why I think suggested ways to address it are not much better or have other downsides. But am I underestimating its importance and/or overblowing the challenges with the alternatives? @aurdipas, is it possible you reacted really strongly to this because the wording in the limitation section made it sound like no change reason was asked for at all in that case?

When to log events when there are multiple questions on a screen

As @Xiphware has pointed out in his recent post and @martijnr highlighted in the doc, it’s not as clear when question events should be logged when there are several questions on a page and especially when some question types can be updated continuously.

This affects all audit features and not this one specifically. Currently, Collect defines the start and end times of question events from within field lists as the times when the field list is entered and exited, respectively. This definitely needs to be improved, but I don’t see a reason for it to block this spec.

I propose we make separate decisions on this, possibly for each question type separately, as other clients get ready to implement the audit log features.

Study design standards

On the call, @martijnr also asked about study design standards. See bullet 2 in the original feature description for an example. One thing I’ve understood from @dr_michaelmarks (correct me if I’m getting this wrong) is that protocols and training are more important than tech.

That is, the technology has to make it possible to collect things like user identifiers and reasons for change but beyond that, a robust protocol with things like independent oversight go a long way in determining whether an approach is standards compliant. It’s possible to design a bad protocol with great tech or to design a great protocol with limited tech (e.g. paper).

3 Likes
#40

100% correct @LN

In reality many minor changes don't have an explanation given either; i.e if I cross out age 8 and put 9 (or even say 20 and put 40) I might well just initial & date-time.

That is to say that ideally there needs to be an option to record changes but its not compulsory to do so (but it is compulsory to date & identify who made the change)

1 Like
ODK TSC 1 Call - 2019-09-04
#41

Correct, although its not quite so bad to continuously logs value changes when moving a slider. But you certainly dont want to pop up and block on a 'Reason for change?' window every time... [I'm trying to think how on earth I'd implement this tracking for my current sliders :anguished: ]

I violently agree :grin:

1 Like
#42

I agree with the fact that many minor changes don't need a reason. As I said in the call, ideally the reason for change should be requested at question level. It would be great if in the xls form we could specifify for a question if a reason for change is required or not.
Depending on the CT (let's say more or less strict) it is not needed a reason for all changes.
But if we can have compulsory to date & identify who made the change that is OK.

I didn't react "really strongly" :slight_smile: I just just said that if it is a choice this should not be listed as a "technical limitation" .

@dr_michaelmarks Are you going to the ECTMIH in Liverpool? I'll be there and present the ODK implementation of the WHO VA. It would be cool if you are around and we could meet in person.

1 Like
#43

Yes I'll be there on the Thursday and Friday

2 Likes
#44

Thanks for inviting me to the TSC call and to all TSC members for being so thoughtful in your feedback.

On the call, we decided the next step would be for @aurdipas to get one last round of feedback from his team. I also mentioned I'd show a sample interaction possibly from a form @aurdipas provided.

I went ahead and built a quick example with a simple toy form. Please note that this shows the interaction in a one-question-per-screen context. There will need to be an addition made to the spec for clients that show multiple questions per screen by default. See conversation at Define audit log event behavior when there are multiple questions on one screen.

This is an extremely rough prototype so the dialogs are unstyled, the text is not polished, the question shouldn't change until the reason for change dialog is dismissed, etc. What it does show is the flow intended by the current spec. You can also see the resulting audit log with a description of how events relate to the video in this Google Sheet which also contains the blank form.

#45

Its probably worth making explicit (since I'm not sure it has yet?) that the sticking point seems to be around specifically when to prompt the user to enter change reasons.

Requiring the enumerator to enter their id/name at the start (and assume an implied opt-out if they dont?), and then logging this info and a timestamp whenever anything changes (including blank-to-non-blank!), is probably a no-brainer; its a minimal amount of additional audit data, and it happens entirely behind-the-scenes on the client. However, what is important is when we will interrupt the user while they are in the midst of filling in a form, and require them to enter a RFC (Reason For Change), eg via a popup.

I'm not saying this gets us any closer to answering the question (alas) but maybe it'll help focus on the core issue? IMO its not quite so much "When to link form instance changes to individuals", as it is "When MUST we interrupt users to enter RFCs".

#46

there's no link to a google sheet :frowning: