-
Notifications
You must be signed in to change notification settings - Fork 5
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
Section override fix #178
base: prefect-by-assignment
Are you sure you want to change the base?
Section override fix #178
Changes from all commits
d84e14b
495bc92
f258db2
215539e
96f01ba
bbbda7b
3150828
2816b18
80c118a
a6edda0
57a6590
1697e3e
465bc4d
fc9cc79
1f7ff05
cb0bc3e
820b8c0
b3a8150
db4e755
c33abe5
07a9dc1
92374ec
f9a0634
5f98e29
446ba89
f7ea51e
3c5f4e9
c0e43c9
2a75163
e852eaf
cecf555
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -138,14 +138,26 @@ def _get_snap_name(course_name, assignment, override): | |
def get_all_snapshots(config, course_id, assignments): | ||
snaps = [] | ||
for asgn in assignments: | ||
snaps.append( {'due_at' : asgn['due_at'], | ||
'name' : _get_snap_name(config.course_names[course_id], asgn, None), | ||
'student_id' : None}) | ||
if asgn['due_at'] is not None: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this change might be a bit tricky. Consider an assignment with no overrides but the instructor forgot to set the deadline -- we don't want to silently ignore that problem. But maybe that would have thrown an exception elsewhere anyway? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Might be worth raising here if there is an assignment that doesn't have any overrides and doesn't have due_at/etc. Then from here on out we can assume if due_at is missing it's because their are overrides There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I made it so it throws an error here if due_at is missing and there are no overrides in commit 465bc4d. |
||
snaps.append( {'due_at' : asgn['due_at'], | ||
'name' : _get_snap_name(config.course_names[course_id], asgn, None), | ||
'student_id' : None}) | ||
for override in asgn['overrides']: | ||
for student_id in override['student_ids']: | ||
snaps.append({'due_at': override['due_at'], | ||
'name' : _get_snap_name(config.course_names[course_id], asgn, override), | ||
'student_id' : student_id}) | ||
if 'course_section_id' in override: | ||
snaps.append( {'due_at' : override['due_at'], | ||
'name' : _get_snap_name(config.course_names[course_id], asgn, override), | ||
'student_id' : None}) | ||
else: | ||
for student_id in override['student_ids']: | ||
snaps.append({'due_at': override['due_at'], | ||
'name' : _get_snap_name(config.course_names[course_id], asgn, override), | ||
'student_id' : student_id}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this should all work OK. |
||
if asgn['due_at'] is None and len(asgn['overrides']) == 0: | ||
msg = f"Assignment {asgn['name']} has no due date and no overrides." | ||
sig = signals.FAIL(msg) | ||
sig.msg = msg | ||
raise sig | ||
|
||
return snaps | ||
|
||
@task(checkpoint=False) | ||
|
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 always exist? I imagine for instructors it might not? You should probably check existence and fill with
None
otherwiseThere 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.
also try probing the rest API directly to find out. Easy way to do that is to put the URL that I send via
requests
to get lists of people directly into the URL bar in your browser instead. You'll see raw JSON response in the browser, just look through that to check.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.
I checked the API docs and it appears to be a required field https://canvas.instructure.com/doc/api/enrollments.html. Also checked list of enrollments, looked like it was there for everyone.