feat: capture daily snapshots of aggregated task completion data#607
feat: capture daily snapshots of aggregated task completion data#607audreypho wants to merge 18 commits intodoubtfire-lms:10.0.xfrom
Conversation
audreypho
changed the title
b0ink
left a comment
b0ink
left a comment
There was a problem hiding this comment.
Looking good - we will need to discuss whether we store the aggregated data inside the database or if they should be saved as json files, mainly for easy archiving/compression at the end of the semester
Can you also fix the merge conflicts so i can run the tests? - I recommend to just rollback the migration via rails db:rollback, pull the latest schema changes, then start a new migration so that you have a newer migration timestamp, then re-run the migration with the requested changes (removing foreign key)
…s, rerun migration
b0ink
left a comment
b0ink
left a comment
There was a problem hiding this comment.
Overall workflow is looking really good
Just a few notes about bringing back the task status names, and querying the student's campus so we can bring back the campus filtering in the front end
Can you also please fix the RuboCop errors in the gh actions?
| def unit_task_status_snapshot_path(unit, snapshot_timestamp, create: true, archived: true) | ||
| snapshot_filename = "#{sanitized_filename(snapshot_timestamp.to_s)}.zip" | ||
| File.join(unit_task_status_snapshots_dir(unit, create: create, archived: archived), snapshot_filename) | ||
| end |
There was a problem hiding this comment.
Rather than storing each snapshot in its own zip file, it would be more efficient to store a single task-status-snapshots.zip in {unit}/analytics/task-status-snapshots.zip where we then place all the CSV files inside.
You'll need to then search for the timestamped snapshot within the ZIP
| row['username'], | ||
| "#{row['first_name']} #{row['last_name']}", | ||
| ] | ||
| student_details << row['campus_abbreviation'] if includes_campus |
There was a problem hiding this comment.
You can simplify this by just always including campus in the CSV, so you can remove the includes_campus argument.
task_status_uses_id should be fine to stay
2 participants
Description
Adds daily scheduled snapshots of task completion data for each unit aggregated at a tutorial level by task and status.
Fixes # (issue)
Type of change
How Has This Been Tested?
Checklist:
If you have any questions, please contact @macite or @jakerenzella.