-
Notifications
You must be signed in to change notification settings - Fork 31
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
Issue/2772 add form version column (Connect #2772) #2966
Conversation
…sition. Extract constant strings.
GAE/src/org/waterforpeople/mapping/app/gwt/client/surveyinstance/SurveyInstanceDto.java
Outdated
Show resolved
Hide resolved
inst.setDeviceIdentifier("IMPORTER"); | ||
inst.setUuid(UUID.randomUUID().toString()); | ||
inst.setSurveyedLocaleId(importReq.getSurveyedLocaleId()); | ||
inst.setUuid(UUID.randomUUID().toString()); | ||
inst.setSubmitterName(importReq.getSubmitter()); | ||
inst.setSurveyalTime(importReq.getSurveyDuration()); | ||
inst.setFormVersion(importReq.getFormVersion()); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean its possible to (re)set the form version when (re)importing data?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only when importing new data. It will be ignored when reimporting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Jana said to handle it just like SurveyalTime, so I did.
GAE/src/org/waterforpeople/mapping/dataexport/GraphicalSurveySummaryExporter.java
Outdated
Show resolved
Hide resolved
GAE/src/org/waterforpeople/mapping/dataexport/RawDataSpreadsheetImporter.java
Outdated
Show resolved
Hide resolved
Row row = sheet.getRow(headerRow); | ||
for (int i = 0; i < firstQuestionColumnIndex; i++) { | ||
String header = row.getCell(i).getStringCellValue(); | ||
if (header.equalsIgnoreCase(IDENTIFIER_LABEL)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The tricky part of using the label to determine which column we are dealing with is the possibility that the user inadvertently changes the text or its in another language. This change would have to go hand in hand with making our documentation clearly state what the column headers should be.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. There is also the possibility of locking or colouring the headers in the data_cleaning output to indicate they are important.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue now lists what needs to change in the documentation. Jana said she would handle that.
// options.put(TYPE_OPT, DATA_CLEANING_TYPE); | ||
options.put(TYPE_OPT, DATA_ANALYSIS_TYPE); | ||
options.put(TYPE_OPT, DATA_CLEANING_TYPE); | ||
// options.put(TYPE_OPT, DATA_ANALYSIS_TYPE); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is the commented out code necessary?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes. That code is only used when debugging locally, and then you uncomment one of the three report types.
Before the PR (what is the issue or what needed to be done)
Form version of a form instance was hidden
The solution
Show it in exports and allow instances created by data cleaning import to set it.
Screenshots (if appropriate)
Checklist