Survey database update v2 #489
Conversation
Work in progress. Triggers need to be added. Some remapped questions need to be remade into new questions due to constraints and new/changed/deleted options. Migraines category needs to be merged and removed. Basic testing w/UI needed.
15 new questions created w/shortnames ending in '_v2' to address questions with many changed response options. New questions linked and existing questions unlinked and retired. All triggers added. Migraines section needs merging. Testing against current unittests needed.
Traced entire execution and reviewed all SQL statements. This test appears like it can be resolved with changing some constants.
Fixed test to use new existing survey. relativedelta(month=2) appeared to have issues w/rollover for both month and year when the current month is 11 (November). Replaced w/standard datetime.timedelta() using 61 days, the median number of days in a given two month span (from beginning of the month to the end of the next month).
Added survey information for the new surveys. Did not delete references to the removed surveys for the moment.
Fixed localization using new Basic Information (10) survey. Possible todos in the future would be to replace the magic number 10 w/a new const name and change localization test to es_MX where the localizations are more different.
Also pushing some partial changes for test_bobo_takes_all_local_surveys() from last night. Not sure why they're not in there.
Removed entries from survey_groups that were retired. Added survey_template_ids2() to this commit for possible use fixing test_metadata_qiita_compatible_* tests.
Updated code and tests to resolve errors after previous adjustments to 0103.sql (now 0104.sql) broke two tests. Broken tests currently stand at four once more.
Four questions were present in the system but missing entries in group_questions, which prevented them from being shown. This has been fixed.
Minor bug in creating new surveys Localization is not implemented Removing old survey not implemented
Data Migration code complete. Undergoing additional testing. 4 qiita-related tests + 1 additional test is now breaking. Will review and fix along with review of the database changes.
All retired questions were migrated to group 10 (Basic Information). They previously made up the group number 9 (Retired Questions). They shoudn't appear in querys, because they are retired. However all questions need to be a member of a group, due to the mechanics of the system.
Co-authored-by: Daniel McDonald <[email protected]>
|
Thanks, @charles-cowart. |
Adding constraint to creation_time in ag_login_surveys.
Simplified complex conditionals in a loop. Removed a commented-out test.
Changed _get_timestamp() to use date and time w/out timezone values from ag.ag_kit_barcodes. To ensure compatibility with other timestamps, the 'America/Los Angeles' timestamp was assigned to the casted timestamp. The value of the casted timestamp is not changed.
survey_template_id isn't needed for the revised get_timestamp() query, as the timestamps for all results are going to be the same, due to the way the query is written. Moreover, regardless of whether there are say multiple template 10s, or no template 10s, we would want to center our response results around this timestamp.
| AND b.survey_template_id = %s""", | ||
| (barcode, survey_template_id)) | ||
|
|
||
| # Expect that one and only one result will always be found. |
There was a problem hiding this comment.
Given that we're changing the system to allow people to re-take surveys, that's not a safe assumption moving forward. I know my earlier comment reflected that it was a safe assumption for old data, but we either need this function to account for multiple records, or one of the upstream functions to do so.
There was a problem hiding this comment.
I think this is okay now, because the query has been changed per Daniel's request to use the sample_date and sample_time columns from ag.ag_kit_barcodes. In this case, there really will be only one timestamp, as this is the one associated with the barcode itself, instead of one of potentially many surveys.
|
I resolved the merge conflict with the idea that I didn't change anything substantial to the vioscreen-related method and hence the changes in master-overhaul superseded them. |
|
Reviewing and resolving test errors post merge. |
|
@cassidysymons @wasade Hi guys - I think responding to those two open questions makes this tentatively resolved again. All other comments have been resolved. Please let me know what you think. Best, C |
|
@wasade When you have a moment, can you please give this a look and see if there are any unresolved issues or further changes you think would be valuable? |
|
@cassidysymons I recommend squash and merge given the number of commits |
Alternative approach to survey/data migration that preserves retired survey templates/groups.
Working on final four broken unittests and adding additional tests as appropriate.