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

WIP: Answers in the audit file #3024

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public void prepareTestFile() {
@Test
public void updateHeaderTest() throws IOException, ExecutionException, InterruptedException {
// Use a form with enabled audit but without location
AuditEventSaveTask auditEventSaveTask = new AuditEventSaveTask(testFile, false);
AuditEventSaveTask auditEventSaveTask = new AuditEventSaveTask(testFile, false, false);
auditEventSaveTask.execute(getSampleAuditEventsWithoutLocations().toArray(new AuditEvent[0])).get();
String expectedAuditContent = FileUtils.readFileToString(testFile);
String expectedData = "event, node, start, end\n" +
Expand All @@ -81,7 +81,7 @@ public void updateHeaderTest() throws IOException, ExecutionException, Interrupt
assertEquals(expectedData, expectedAuditContent);

// Upgrade a form to use location and edit saved form
auditEventSaveTask = new AuditEventSaveTask(testFile, true);
auditEventSaveTask = new AuditEventSaveTask(testFile, true, false);
auditEventSaveTask.execute(getMoreSampleAuditEventsWithLocations().toArray(new AuditEvent[0])).get();
expectedAuditContent = FileUtils.readFileToString(testFile);
String expectedData2 = "event, node, start, end, latitude, longitude, accuracy\n" +
Expand Down Expand Up @@ -110,7 +110,7 @@ public void updateHeaderTest() throws IOException, ExecutionException, Interrupt

@Test
public void saveAuditWithLocation() throws ExecutionException, InterruptedException, IOException {
AuditEventSaveTask auditEventSaveTask = new AuditEventSaveTask(testFile, true);
AuditEventSaveTask auditEventSaveTask = new AuditEventSaveTask(testFile, true, false);
auditEventSaveTask.execute(getSampleAuditEventsWithLocations().toArray(new AuditEvent[0])).get();
String expectedAuditContent = FileUtils.readFileToString(testFile);
String expectedData = "event, node, start, end, latitude, longitude, accuracy\n" +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1033,7 +1033,7 @@ public boolean onOptionsItemSelected(MenuItem item) {
}

if (formController != null) {
formController.getAuditEventLogger().exitView();
formController.getAuditEventLogger().exitView(getAnswersForCurrentScreen());
formController.getAuditEventLogger().logEvent(AuditEvent.AuditEventType.HIERARCHY, null, true);
}

Expand Down Expand Up @@ -1106,6 +1106,12 @@ private void clearAnswer(QuestionWidget qw) {
}
}

private HashMap<FormIndex, IAnswerData> getAnswersForCurrentScreen() {
return getCurrentViewIfODKView() != null
? getCurrentViewIfODKView().getAnswers()
: null;
}

@Override
public void onCreateContextMenu(ContextMenu menu, View v,
ContextMenuInfo menuInfo) {
Expand Down Expand Up @@ -1189,8 +1195,11 @@ private View createView(int event, boolean advancingPage) {

setTitle(formController.getFormTitle());

formController.getAuditEventLogger().logEvent(AuditEvent.getAuditEventTypeFromFecType(event),
formController.getFormIndex().getReference(), true);
if (!formController.getAuditEventLogger().isCollectingAnswersEnabled()
|| (event != FormEntryController.EVENT_GROUP && event != FormEntryController.EVENT_QUESTION)) {
formController.getAuditEventLogger().logEvent(AuditEvent.getAuditEventTypeFromFecType(event),
formController.getFormIndex().getReference(), true);
}

switch (event) {
case FormEntryController.EVENT_BEGINNING_OF_FORM:
Expand Down Expand Up @@ -1341,6 +1350,10 @@ public void onClick(View v) {

// Makes a "clear answer" menu pop up on long-click
for (QuestionWidget qw : odkView.getWidgets()) {
if (formController.getAuditEventLogger().isCollectingAnswersEnabled()) {
formController.getAuditEventLogger().logEvent(AuditEvent.AuditEventType.QUESTION,
qw.getFormEntryPrompt().getIndex().getReference(), true);
}
if (!qw.getFormEntryPrompt().isReadOnly()) {
// If it's a StringWidget register all its elements apart from EditText as
// we want to enable paste option after long click on the EditText
Expand Down Expand Up @@ -1456,7 +1469,7 @@ private void showNextView() {
return;
}

formController.getAuditEventLogger().exitView(); // Close events waiting for an end time
formController.getAuditEventLogger().exitView(getAnswersForCurrentScreen()); // Close events waiting for an end time

switch (event) {
case FormEntryController.EVENT_QUESTION:
Expand Down Expand Up @@ -1523,7 +1536,7 @@ private void showPreviousView() {
// create savepoint
nonblockingCreateSavePointData();
}
formController.getAuditEventLogger().exitView(); // Close events
formController.getAuditEventLogger().exitView(getAnswersForCurrentScreen()); // Close events
View next = createView(event, false);
showView(next, AnimationType.LEFT);
} else {
Expand Down Expand Up @@ -2579,7 +2592,7 @@ public void savingComplete(SaveResult saveResult) {
break;
case FormEntryController.ANSWER_CONSTRAINT_VIOLATED:
case FormEntryController.ANSWER_REQUIRED_BUT_EMPTY:
formController.getAuditEventLogger().exitView();
formController.getAuditEventLogger().exitView(getAnswersForCurrentScreen());
formController.getAuditEventLogger().logEvent(AuditEvent.AuditEventType.CONSTRAINT_ERROR, null, true);
refreshCurrentView();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,4 +89,8 @@ public Long getLocationMaxAge() {
public boolean isLocationEnabled() {
return locationPriority != null && locationMinInterval != null && locationMaxAge != null;
}

public boolean isCollectingAnswersEnabled() {
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@ public String getValue() {
private String latitude;
private String longitude;
private String accuracy;
private String answer;
private long end;
private boolean endTimeSet;

Expand Down Expand Up @@ -113,20 +114,37 @@ public boolean hasLocation() {
&& accuracy != null && !accuracy.isEmpty();
}

private boolean hasAnswer() {
return answer != null;
}

public void setLocationCoordinates(String latitude, String longitude, String accuracy) {
this.latitude = latitude;
this.longitude = longitude;
this.accuracy = accuracy;
}

public void setAnswer(String answer) {
this.answer = answer;
}

/*
* convert the event into a record to write to the CSV file
*/
@NonNull
public String toString() {
return hasLocation()
? String.format("%s,%s,%s,%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "", latitude, longitude, accuracy)
: String.format("%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "");
String event;
if (hasLocation() && hasAnswer()) {
event = String.format("%s,%s,%s,%s,%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "", latitude, longitude, accuracy, answer);
} else if (hasLocation()) {
event = String.format("%s,%s,%s,%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "", latitude, longitude, accuracy);
} else if (hasAnswer()) {
event = String.format("%s,%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "", answer);
} else {
event = String.format("%s,%s,%s,%s", auditEventType.getValue(), node, start, end != 0 ? end : "");
}

return event;
}

// Get event type based on a Form Controller event
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,13 +20,16 @@
public class AuditEventSaveTask extends AsyncTask<AuditEvent, Void, Void> {
private final @NonNull File file;
private final boolean isLocationEnabled;
private final boolean isCollectingAnswersEnabled;

private static final String CSV_HEADER = "event, node, start, end";
private static final String CSV_HEADER_WITH_LOCATION_COORDINATES = CSV_HEADER + ", latitude, longitude, accuracy";
private static final String DEFAULT_COLUMNS = "event, node, start, end";
private static final String LOCATION_COORDINATES_COLUMNS = ", latitude, longitude, accuracy";
private static final String ANSWERS_COLUMN = ", answer";

public AuditEventSaveTask(@NonNull File file, boolean isLocationEnabled) {
public AuditEventSaveTask(@NonNull File file, boolean isLocationEnabled, boolean isCollectingAnswersEnabled) {
this.file = file;
this.isLocationEnabled = isLocationEnabled;
this.isCollectingAnswersEnabled = isCollectingAnswersEnabled;
}

@Override
Expand Down Expand Up @@ -72,10 +75,10 @@ private boolean updateHeaderIfNeeded() {
try {
br = new BufferedReader(new FileReader(file));
String header = br.readLine();
if (header != null && header.equals(CSV_HEADER)) { // update header
if (header != null && header.equals(DEFAULT_COLUMNS)) { // update header
File temporaryFile = new File(file.getParentFile().getAbsolutePath() + "/temporaryAudit.csv");
tfw = new FileWriter(temporaryFile, true);
tfw.write(CSV_HEADER_WITH_LOCATION_COORDINATES + "\n");
tfw.write(DEFAULT_COLUMNS + LOCATION_COORDINATES_COLUMNS + "\n");
String line;
while ((line = br.readLine()) != null) {
tfw.write(line + "\n");
Expand Down Expand Up @@ -105,8 +108,13 @@ private boolean updateHeaderIfNeeded() {
}

private String getHeader() {
return isLocationEnabled
? CSV_HEADER_WITH_LOCATION_COORDINATES + "\n"
: CSV_HEADER + "\n";
String header = DEFAULT_COLUMNS;
if (isLocationEnabled) {
header += LOCATION_COORDINATES_COLUMNS;
}
if (isCollectingAnswersEnabled) {
header += ANSWERS_COLUMN;
}
return header + "\n";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
import android.os.AsyncTask;
import android.os.SystemClock;

import org.javarosa.core.model.FormIndex;
import org.javarosa.core.model.data.IAnswerData;
import org.javarosa.core.model.instance.TreeReference;
import org.odk.collect.android.logic.AuditConfig;
import org.odk.collect.android.logic.AuditEvent;
Expand All @@ -13,6 +15,7 @@
import java.io.File;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;

import io.reactivex.annotations.Nullable;
import timber.log.Timber;
Expand Down Expand Up @@ -87,7 +90,7 @@ public void logEvent(AuditEvent.AuditEventType eventType, TreeReference ref, boo
* This can happen if the user is on a question page and the page gets refreshed
* The exception is hierarchy events since they interrupt an existing interval event
*/
if (newAuditEvent.isIntervalAuditEventType()) {
if (!auditConfig.isCollectingAnswersEnabled() && newAuditEvent.isIntervalAuditEventType()) {
for (AuditEvent aev : auditEvents) {
if (aev.isIntervalAuditEventType() && !aev.isEndTimeSet()) {
return;
Expand Down Expand Up @@ -139,13 +142,23 @@ boolean isDuplicateOfLastAuditEvent(AuditEvent.AuditEventType eventType) {
* Exit a question
*/
public void exitView() {
exitView(null);
}

public void exitView(Map<FormIndex, IAnswerData> answers) {
if (isAuditEnabled()) {
// Calculate the time and add the event to the auditEvents array
long end = getEventTime();
for (AuditEvent aev : auditEvents) {
if (!aev.isEndTimeSet() && aev.isIntervalAuditEventType()) {
addLocationCoordinatesToAuditEventIfNeeded(aev);
aev.setEnd(end);
if (answers != null && !answers.isEmpty() && aev.getAuditEventType() == AuditEvent.AuditEventType.QUESTION) {
Map.Entry entry = answers.entrySet().iterator().next();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This means the answers from questions in a field list will be provided in some arbitrary order because the map is a hashmap, right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the order should be the same as questions on the screen.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, right, this is actually worse than I had originally imagined, I think. Isn't it the case that the audit event has a reference and that the answer that gets associated with that audit event might not match the reference to the question? How about passing a map from reference to answer rather than FormIndex to answer? By reference I mean what is stored in the AuditEvent.node field (e.g. /data/name), so that you can just match the event to its answer?

IAnswerData answer = (IAnswerData) entry.getValue();
aev.setAnswer(answer != null ? answer.getValue().toString() : null);
answers.remove(entry.getKey());
}
}
}

Expand All @@ -157,7 +170,7 @@ private void writeEvents() {
if (saveTask == null || saveTask.getStatus() == AsyncTask.Status.FINISHED) {
AuditEvent[] auditEventArray = auditEvents.toArray(new AuditEvent[0]);
if (auditFile != null) {
saveTask = new AuditEventSaveTask(auditFile, auditConfig.isLocationEnabled()).execute(auditEventArray);
saveTask = new AuditEventSaveTask(auditFile, auditConfig.isLocationEnabled(), true).execute(auditEventArray);
} else {
Timber.e("auditFile null when attempting to write auditEvents.");
}
Expand Down Expand Up @@ -224,4 +237,8 @@ ArrayList<AuditEvent> getAuditEvents() {
List<Location> getLocations() {
return locations;
}

public boolean isCollectingAnswersEnabled() {
return auditConfig.isCollectingAnswersEnabled();
}
}