Skip to content
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

Individual Due Date Extension feature #2062

Merged
merged 1 commit into from
Jan 14, 2014
Merged

Conversation

chrisrossi
Copy link
Contributor

This pull request supercedes #1928. This is the same work, but with all commits squashed.

This changeset adds a feature to the edX platform which allows instructors to set individual due dates for students on particular coursework. This code is meant primarily for on-campus use--it is not intended that this feature would be used for MOOCs. It adds a new tab, "Extensions", to the instructor dashboard which allows changing due dates per student. This feature is enabled by setting FEATURES['INDIVIDUAL_DUE_DATES'] = True.

This work is performed on behalf of MIT and falls under the contributor agreement.

@chrisrossi chrisrossi mentioned this pull request Dec 31, 2013
@sarina
Copy link
Contributor

sarina commented Jan 3, 2014

@chrisrossi what further needs to be reviewed here? I'm just catching up from time off on the holidays so not sure I can take a look until Monday. Plus as I said I'd like to talk to our UX people briefly and with the snow day none of us went into work today.

Also, your final merged commit should have a better commit message. eg

Add individual due date extension feature.

This feature allows extensions on assignments for individual students.
(any other things that are important to mention should go here)

@chrisrossi
Copy link
Contributor Author

It address eveything I know about. We can write final commit message when doing final squash after UI tweaks.

@sarina
Copy link
Contributor

sarina commented Jan 6, 2014

The UX issues as I see it are a few:

  • No explanation of the feature
  • No explanation that an earlier date will have no effect
  • Need to explain how to enter in the UTC time, when I click in the box the dialogue goes away and I immediately forget the format
  • For 3/4 buttons you don't need to enter in the due date, for List Students you don't even need to enter in a username; for List All for 1 student you don't need to enter in anything except a username. However the UX presented makes it seem like you must fill in the three boxes for the change to take place. This means the page is super congested and hard to understand.

Here's my proposed visual change which separates this from one congested blip to three separate sections (similar to the Student Admin section, if you'd like to refer to another piece of templating that does the same thing). By separate sections I mean separate divs with lines. Again refer to Student Admin.

@sarina
Copy link
Contributor

sarina commented Jan 6, 2014

INDIVIDUAL DUE DATE EXTENSIONS
In this section, you have the ability to grant extensions on specific units to individual students. Please note that the latest date is always taken; you cannot use this tool to make an assignment due earlier for a particular student.

Specify the edX email address or username of a student here: [box]

Choose the graded unit: (drop-down)

Specify the extension due date and time (in UTC; please specify as MM/DD/YYYY HH:MM): [box prepopulated with MM/DD/YYYY HH:MM]

[Change Due Date For Student button]

-------- # hr tag

VIEWING GRANTED EXTENSIONS
Here you can see what extensions have been granted on particular units or for a particular student.

Choose a graded unit and click the button to obtain a list of all students who have extensions for the given unit.

Choose the graded unit: (drop-down) [List All Students With Extensions button] # all in one line

Specify a specific student to see all of that student's extensions.

Specify the edX email address or username of a student here: [box]

[List All Date Extensions For Specified Student button]

-------- # hr tag

RESETTING EXTENSIONS
Resetting a problem's due date rescinds a due date extension for a student on a particular unit. This will revert the due date for the student back to the problem's original due date.

Specify the edX email address or username of a student here: [box]

Choose the graded unit: (drop-down)

[Reset Due Date For Student button]

@sarina
Copy link
Contributor

sarina commented Jan 6, 2014

Also - your unit test coverage is looking swell! If there's any quality violations (https://jenkins.testeng.edx.org/job/edx-platform-report/1525/Diff_Quality_Report/?) you can address that'd be great, thanks.

due_date = instance_state.get('due', None)
due_date = instance_state.get('extended_due', None)
if due_date is None:
due_date = instance_state.get('due', None)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doesn't this need to also use get_extended_due_date() logic?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, instance state is a dictionary, and get_extended_due_date() operates on objects. This was written before we changed the due date semantics to always favor the later occurring date.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll figure out something for that spot.

@ormsbee
Copy link
Contributor

ormsbee commented Jan 7, 2014

Aside from my one minor comment, 👍 after @sarina's happy with the UX changes.

@chrisrossi
Copy link
Contributor Author

Ok, I think I've addressed everything. If everything looks a-ok I'll squash and write a final commit message.

@sarina
Copy link
Contributor

sarina commented Jan 13, 2014

@chrisrossi I'm looking at this now. Two things -

  1. it looks like your branch has a conflict so it'll need a rebase
  2. the tests seem to have failed but not explicit failure, instead it looks like one of the CMS acceptance tests was aborted. So they likely just need to be re-run again (re-running here: https://jenkins.testeng.edx.org/job/edx-all-tests-manual-pr/522/)

@sarina
Copy link
Contributor

sarina commented Jan 13, 2014

@chrisrossi Visually this looks great, thanks so much for doing the changes.

My only nitpick - if this is really simple to fix, please do so, but if it's substantive work it's ok - you can still enter in dates that are earlier than the assigned due date, and the tool will list the student as having the earlier date. See in this screenshot, I set the date to 1/15/2012 when the due date is actually 1/20/2014:

screen shot 2014-01-13 at 1 05 32 pm

Now the feature still does work - we see in this screenshot that the user does have the correct, later due date of 1/20/14:

screen shot 2014-01-13 at 1 05 46 pm

If it's easy to add in something that will disallow you to enter in a date earlier than the due date for the unit, I would like that (either a pop-up box or text that displays in the normal confirmation error saying something like "You can't assign a date earlier than the due date for the unit"). But again if it's too much work, let's just go ahead and get this in.

@sarina
Copy link
Contributor

sarina commented Jan 13, 2014

Second build passed! Quality is 100% and coverage is 97%. Looks amazing 😄

@chrisrossi
Copy link
Contributor Author

@sarina Since we're out of budget for this, I'm going to leave the above alone for now. If you would like, you can enter it into an issue tracker somewhere so we can know about it and then I'm happy to loop back around and get it when/if MIT approves the additional work.

@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

@chrisrossi sounds good. I think the notice for now is enough. I'll put in a small bug for it once we're merged.

I can't merge it, though, until there's a rebase - and you said you wanted to do the squash.

@chrisrossi
Copy link
Contributor Author

Rebased and squashed.

@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

Will wait for build to pass before merging.

In the future please make your commit message's first 50 characters a summary of the feature, and then use additional lines to explain fully (so people can briefly understand what the commit is without hitting the "..." button to expand). eg

Add individual due date extension feature.

<More detail down here, after first sentence>

@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

Note that this is how I requested you structure your final squashed commit message in a comment on this PR from 11 days ago. This format allows us on release to scan the commit messages and see at a brief glance what was added in.

Adds a feature to the edX platform which allows instructors to set
individual due dates for students on particular coursework. This code is
meant primarily for on-campus use--it is not intended that this feature
would be used for MOOCs. It adds a new tab, "Extensions", to the beta
instructor dashboard which allows changing due dates per student. This
feature is enabled by setting FEATURES['INDIVIDUAL_DUE_DATES'] = True.
@chrisrossi
Copy link
Contributor Author

Ok, rewritten.

sarina added a commit that referenced this pull request Jan 14, 2014
Individual Due Date Extension feature
@sarina sarina merged commit 129c244 into openedx:master Jan 14, 2014
@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

@carsongee I made a bug on our JIRA board for the remaining small issue with this feature: https://edx-wiki.atlassian.net/browse/LMS-2035

@carsongee
Copy link
Contributor

@sarina thanks for the heads up. Totally missed this getting merged. Woot.

@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

Sure thing! The ticket will probably stay in our backlog awhile but it's a good UX improvement.

@sarina
Copy link
Contributor

sarina commented Jan 14, 2014

Feel free to get it done yourself if you wish :)

@carsongee
Copy link
Contributor

I bookmarked it and added it to my backlog so that if I ever run out of other changes I'll get there ;)

@sarina
Copy link
Contributor

sarina commented Mar 23, 2015

@chrisrossi @carsongee : I know this is old 😲 but is there any documentation available for this feature? Thanks

@pdpinch
Copy link
Contributor

pdpinch commented Mar 23, 2015

We have some docs that we used internally at MIT (requires MIT certificate), but they're so out-of-date, they still refer to the "New Beta Dashboard"

https://odl.mit.edu/wiki/EXTENSIONS

I'd be happy to update this text, and put it some place more convenient. Where would you like the docs to go?

On Mar 23, 2015, at 1:53 PM, Sarina Canelake notifications@github.com wrote:

@chrisrossi @carsongee : I know this is old but is there any documentation available for this feature? Thanks


Reply to this email directly or view it on GitHub.

@sarina
Copy link
Contributor

sarina commented Mar 23, 2015

If you want to create a page on either the Github wiki for edx-platform, or
in the "Open Source" space on
https://openedx.atlassian.net/wiki/display/OPEN/Open+Source+Home, I think
those would be the best and most open options.

On Mon, Mar 23, 2015 at 2:01 PM, Peter Pinch notifications@github.com
wrote:

We have some docs that we used internally at MIT (requires MIT
certificate), but they're so out-of-date, they still refer to the "New Beta
Dashboard"

https://odl.mit.edu/wiki/EXTENSIONS

I'd be happy to update this text, and put it some place more convenient.
Where would you like the docs to go?

On Mar 23, 2015, at 1:53 PM, Sarina Canelake notifications@github.com
wrote:

@chrisrossi @carsongee : I know this is old but is there any
documentation available for this feature? Thanks


Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
https://github.com/edx/edx-platform/pull/2062#issuecomment-85124585.

jenkins-ks pushed a commit to nttks/edx-platform that referenced this pull request Sep 22, 2017
* Add menu to ga_operation for ga_analyzer openedx#2039 (openedx#2088)

* add role for old course viewer openedx#2062 (openedx#2087)

* add role for old course viewer openedx#2062

* Change action for biz course by BetaTester role openedx#2062

* Construction of image server openedx#2025 (openedx#2106)

* cherry-pick 8c8953f

* Fix file upload in IE

* Construction of image server openedx#2025

* add all keywords search in Student management openedx#2029 (openedx#2034)

* Fix bug for before enrollment start in ga old course viewer openedx#2062 (openedx#2125)

* fix. Construction of image server openedx#2025 (openedx#2117)

* Modify message and css of enrollment for Course About openedx#2130

* Add a certificate list to user's profile page. openedx#2042 (openedx#2108)

* Mod UT openedx#2130

* add PDF File Construction of image server openedx#2025 (openedx#2140)

* add library option, and library links to the course. openedx#2001 (openedx#2124)

* Invalid StudioPermissionsService object in API to show/save xblock settings in CMS.
Randomized Content Block editor did not check Studio user's permissions

* add library option, and library links to the course. openedx#2001

* fix. add all keywords search in Student management openedx#2029 (openedx#2034) (openedx#2157)

* second fix. Construction of image server openedx#2025 (openedx#2158)

* add library option, and library links to the course. openedx#2001 (openedx#2160)

* third fix. Construction of image server openedx#2163 (openedx#2164)

* Add filter by category for certificates on profile page openedx#2042 (openedx#2165)

* Fix bug for  add library option, and library links to the course. openedx#2162 openedx#2133 (openedx#2167)

* Develop/dogwood/gacco201708 (openedx#2170)

* Fixed bugs openedx#2039 (openedx#2112)

* Fixed csv format openedx#2039 (openedx#2127)

* Change to split download if there are many display items openedx#916 (openedx#2121)

* Change to split download if there are many display items openedx#916

* Fix UT

* Fix Review

* Fix review2
shimulch pushed a commit to open-craft/edx-platform that referenced this pull request Jan 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants