Repository navigation
Feature idde #1928
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
Feature idde #1928
Changes from 28 commits
91075eb
ebd2759
c8794b8
8f57c59
eb76cf1
7f40071
b1716f4
083ab0d
0eaafba
12579d1
7e9cbac
087b539
04b70a5
aff6970
94ea0f9
8ea1441
37bc9ce
28a2bce
7834d82
b95a991
81b9c63
53325df
f5eb670
97c562b
8e9138c
4dc3a5c
9d19986
1f00bf5
757267c
a164e4d
b1c71db
4edbae7
685314e
93292e5
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 |
|---|---|---|
| @@ -0,0 +1,18 @@ | ||
| """ | ||
| Miscellaneous utility functions. | ||
| """ | ||
|
|
||
|
|
||
| def get_extended_due_date(node): | ||
| """ | ||
| Gets the actual due date for the logged in student for this node, returning | ||
| the extendeded due date if one has been granted and it is later than the | ||
| global due date, otherwise returning the global due date for the unit. | ||
| """ | ||
|
Contributor
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. Do we have to worry about the case where a student is given an extension, but then the entire assignment due date is pushed back (i.e. should we be grabbing the later of the two instead of always having
Contributor
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. It's a good question -- what to do when the due date is pushed later than the individual date. However, I think we should favor the date entered by staff over a business rule to always take the latest date. In the name of the feature the emphasis is on "individual due date" and less on "extension" Perhaps we should add something to the documentation (or the UI) to make this clearer. On Dec 13, 2013, at 11:17 AM, David Ormsbee wrote:
Contributor
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'm really concerned about Dave's point, and as an instructor I would expect that if I give a few students 2 days extension, then changed the global due dates to 7 days later, that the individual students would get the latest, most generous date. @pdpinch you say "the name of the feature the emphasis is on idd and not extension", but the code disagrees with that assertion. Everywhere throughout this code the feature is referred to as "extension", "extended_due", etc. Even the docstring for the field indicates extension: With all that in mind, I was actually really surprised that you don't just take whatever date is latest. I vote for erring on the side of benefitting the student, unless you go through this entire pull request and change the name of everything and all the docstrings to something that much more clearly indicates "individual due date" and removes all references to the word "extension". I'm open to further discussion but I am concerned about this.
Contributor
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. Ok, you've convinced me to get some more user input on this question of whether to favor the latest date, or the instructor's date. With finals this week, my timing isn't good to get an answer quickly but I'll try to get back to you all tomorrow.
Contributor
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'm fine with either as long as it's clearly documented. My main concern is that as is I think things are inconsistent. Firmly going one way or another would be best.
Contributor
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. My quick, informal poll of course staff has proven @sarina and @ormsbee to be correct. @chrisrossi let's change this to take whichever is the later date. On Dec 16, 2013, at 10:16 PM, Sarina Canelake wrote: In common/lib/xmodule/xmodule/utils.py:
I'm really concerned about Dave's point, and as an instructor I would expect that if I give a few students 2 days extension, then changed the global due dates to 7 days later, that the individual students would get the latest, most generous date. @pdpinchhttps://github.com/pdpinch you say "the name of the feature the emphasis is on idd and not extension", but the code disagrees with that assertion. Everywhere throughout this code the feature is referred to as "extension", "extended_due", etc. Even the docstring for the field indicates extension:
With all that in mind, I was actually really surprised that you don't just take whatever date is latest. I vote for erring on the side of benefitting the student, unless you go through this entire pull request and change the name of everything and all the docstrings to something that much more clearly indicates "individual due date" and removes all references to the word "extension". I'm open to further discussion but I am concerned about this. —
Contributor
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. Can you change the help text for Feel free to tweak my exact language there.
Contributor
Author
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. Done. |
||
| extended = getattr(node, 'extended_due', None) | ||
| due_date = getattr(node, 'due', None) | ||
| if not extended and not due_date: | ||
| return None | ||
| elif not extended or extended < due_date: | ||
| return due_date | ||
| return extended | ||
|
Contributor
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. Is it ever possible to get into a state where there is an
Contributor
Author
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. The UI only allows extensions on modules with due dates. So, it is possible, but it would require manually monkeying with the database. |
||
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.
While this test is good, capa is a notoriously flaky part of our system, and I would like to see some other tests of this functionality. Does it do what's expected when we're past the extended due date too? What does the module claim its due date is when it's being presented to the user? Is that correct?