Merge pull request #24234 from edx/mikix/instructor-dashboard-pls

AA-184: Fix extension dashboard for self-paced courses

lms/djangoapps/instructor/tests/test_api.py

@@ -22,10 +22,9 @@ from django.core import mail
 from django.core.files.uploadedfile import SimpleUploadedFile
 from django.http import HttpRequest, HttpResponse
 from django.test import RequestFactory, TestCase
-from django.test.utils import override_settings
 from django.urls import reverse as django_reverse
 from django.utils.translation import ugettext as _
-from edx_when.api import get_overrides_for_user
+from edx_when.api import get_dates_for_course, get_overrides_for_user, set_date_for_block
 from mock import Mock, NonCallableMock, patch
 from opaque_keys.edx.keys import CourseKey
 from opaque_keys.edx.locator import UsageKey
@@ -3901,11 +3900,18 @@ def get_extended_due(course, unit, user):
     for override in dates:
         if text_type(override['location']) == location:
             return override['actual_date']
-    print(unit.location)
-    print(dates)
     return None
 
 
+def get_date_for_block(course, unit, user):
+    """
+    Gets the due date for the given user on the given unit (overridden or original).
+    Returns `None` if there is no date set.
+    (Differs from edx-when's get_date_for_block only in that we skip the cache.
+    """
+    return get_dates_for_course(course.id, user=user, use_cached=False).get((unit.location, 'due'), None)
+
+
 class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
     """
     Test data dumps for reporting.
@@ -4042,6 +4048,28 @@ class TestDueDateExtensions(SharedModuleStoreTestCase, LoginEnrollmentTestCase):
             get_extended_due(self.course, self.week1, self.user1)
         )
 
+    @RELATIVE_DATES_FLAG.override(True)
+    def test_reset_date_only_in_edx_when(self):
+        # Start with a unit that only has a date in edx-when
+        self.assertEqual(get_date_for_block(self.course, self.week3, self.user1), None)
+        original_due = datetime.datetime(2010, 4, 1, tzinfo=UTC)
+        set_date_for_block(self.course.id, self.week3.location, 'due', original_due)
+        self.assertEqual(get_date_for_block(self.course, self.week3, self.user1), original_due)
+
+        # set override, confirm it took
+        override = datetime.datetime(2010, 7, 1, tzinfo=UTC)
+        set_date_for_block(self.course.id, self.week3.location, 'due', override, user=self.user1)
+        self.assertEqual(get_date_for_block(self.course, self.week3, self.user1), override)
+
+        # Now test that we noticed the edx-when date
+        url = reverse('reset_due_date', kwargs={'course_id': text_type(self.course.id)})
+        response = self.client.post(url, {
+            'student': self.user1.username,
+            'url': text_type(self.week3.location),
+        })
+        self.assertContains(response, 'Successfully reset due date for student')
+        self.assertEqual(get_date_for_block(self.course, self.week3, self.user1), original_due)
+
     def test_show_unit_extensions(self):
         self.test_change_due_date()
         url = reverse('show_unit_extensions',

lms/djangoapps/instructor/tests/test_tools.py

@@ -15,6 +15,7 @@ from django.test import TestCase
 from opaque_keys.edx.keys import CourseKey
 from pytz import UTC
 
+from edx_when.api import set_dates_for_course
 from edx_when.field_data import DateLookupFieldData
 from openedx.core.djangoapps.course_date_signals import handlers
 from openedx.core.djangoapps.schedules.tests.factories import ScheduleFactory
@@ -137,17 +138,19 @@ class TestGetUnitsWithDueDate(ModuleStoreTestCase):
         """
         Fixtures.
         """
-        super(TestGetUnitsWithDueDate, self).setUp()
+        super().setUp()
+
+        course = CourseFactory.create()
+        week1 = ItemFactory.create(parent=course)
+        week2 = ItemFactory.create(parent=course)
+        child = ItemFactory.create(parent=week1)
 
         due = datetime.datetime(2010, 5, 12, 2, 42, tzinfo=UTC)
-        course = CourseFactory.create()
+        set_dates_for_course(course.id, [
-        week1 = ItemFactory.create(due=due, parent=course)
+            (week1.location, {'due': due}),
-        week2 = ItemFactory.create(due=due, parent=course)
+            (week2.location, {'due': due}),
-
+            (child.location, {'due': due}),
-        ItemFactory.create(
+        ])
-            parent=week1,
-            due=due
-        )
 
         self.course = course
         self.week1 = week1
@@ -242,13 +245,21 @@ class TestSetDueDateExtension(ModuleStoreTestCase):
             block.fields['due']._del_cached_value(block)  # pylint: disable=protected-access
 
     def test_set_due_date_extension(self):
-        extended = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC)
+        # First, extend the leaf assignment date
-        tools.set_due_date_extension(self.course, self.week1, self.user, extended)
+        extended_hw = datetime.datetime(2013, 10, 25, 0, 0, tzinfo=UTC)
-        tools.set_due_date_extension(self.course, self.assignment, self.user, extended)
+        tools.set_due_date_extension(self.course, self.assignment, self.user, extended_hw)
         self._clear_field_data_cache()
-        self.assertEqual(self.week1.due, extended)
+        self.assertEqual(self.week1.due, self.due)
-        self.assertEqual(self.homework.due, extended)
+        self.assertEqual(self.homework.due, self.due)
-        self.assertEqual(self.assignment.due, extended)
+        self.assertEqual(self.assignment.due, extended_hw)
+
+        # Now, extend the whole section that the assignment was in. Both it and all under it should change
+        extended_week = datetime.datetime(2013, 12, 25, 0, 0, tzinfo=UTC)
+        tools.set_due_date_extension(self.course, self.week1, self.user, extended_week)
+        self._clear_field_data_cache()
+        self.assertEqual(self.week1.due, extended_week)
+        self.assertEqual(self.homework.due, extended_week)
+        self.assertEqual(self.assignment.due, extended_week)
 
     def test_set_due_date_extension_invalid_date(self):
         extended = datetime.datetime(2009, 1, 1, 0, 0, tzinfo=UTC)

lms/djangoapps/instructor/views/api.py

@@ -8,20 +8,17 @@ Many of these GETs may become PUTs in the future.
 
 
 import csv
-import decimal
 import json
 import logging
 import random
 import re
 import string
-import time
 
 import six
 import unicodecsv
 from django.conf import settings
 from django.contrib.auth.models import User
 from django.core.exceptions import MultipleObjectsReturned, ObjectDoesNotExist, PermissionDenied, ValidationError
-from django.core.mail.message import EmailMessage
 from django.core.validators import validate_email
 from django.db import IntegrityError, transaction
 from django.http import HttpResponse, HttpResponseBadRequest, HttpResponseForbidden, HttpResponseNotFound
@@ -35,6 +32,7 @@ from django.views.decorators.csrf import ensure_csrf_cookie
 from django.views.decorators.http import require_http_methods, require_POST
 from edx_rest_framework_extensions.auth.jwt.authentication import JwtAuthentication
 from edx_rest_framework_extensions.auth.session.authentication import SessionAuthenticationAllowInactiveUser
+from edx_when.api import get_date_for_block
 from opaque_keys import InvalidKeyError
 from opaque_keys.edx.keys import CourseKey, UsageKey
 from rest_framework import status
@@ -51,7 +49,6 @@ import instructor_analytics.distributions
 from bulk_email.api import is_bulk_email_feature_enabled
 from bulk_email.models import CourseEmail
 from course_modes.models import CourseMode
-from edxmako.shortcuts import render_to_string
 from lms.djangoapps.certificates import api as certs_api
 from lms.djangoapps.certificates.models import (
     CertificateInvalidation,
@@ -93,7 +90,7 @@ from openedx.core.djangoapps.django_comment_common.models import (
     Role
 )
 from openedx.core.djangoapps.site_configuration import helpers as configuration_helpers
-from openedx.core.djangoapps.user_api.preferences.api import get_user_preference, set_user_preference
+from openedx.core.djangoapps.user_api.preferences.api import get_user_preference
 from openedx.core.djangolib.markup import HTML, Text
 from openedx.core.lib.api.authentication import BearerAuthenticationAllowInactiveUser
 from openedx.core.lib.api.view_utils import DeveloperErrorViewMixin
@@ -2408,14 +2405,16 @@ def reset_due_date(request, course_id):
     unit = find_unit(course, request.POST.get('url'))
     reason = strip_tags(request.POST.get('reason', ''))
 
+    original_due_date = get_date_for_block(course_id, unit.location)
+
     set_due_date_extension(course, unit, student, None, request.user, reason=reason)
-    if not getattr(unit, "due", None):
+    if not original_due_date:
         # It's possible the normal due date was deleted after an extension was granted:
         return JsonResponse(
             _("Successfully removed invalid due date extension (unit has no due date).")
         )
 
-    original_due_date_str = unit.due.strftime(u'%Y-%m-%d %H:%M')
+    original_due_date_str = original_due_date.strftime(u'%Y-%m-%d %H:%M')
     return JsonResponse(_(
         u'Successfully reset due date for student {0} for {1} '
         u'to {2}').format(student.profile.name, _display_unit(unit),

lms/djangoapps/instructor/views/tools.py

@@ -17,6 +17,7 @@ from pytz import UTC
 from six import string_types, text_type
 from six.moves import zip
 
+from openedx.core.djangoapps.schedules.models import Schedule
 from student.models import get_user_by_username_or_email, CourseEnrollment
 
 
@@ -127,13 +128,19 @@ def get_units_with_due_date(course):
     """
     units = []
 
+    # Pass in a schedule here so that we get back any relative dates in the course, but actual value
+    # doesn't matter, since we don't care about the dates themselves, just whether they exist.
+    # Thus we don't save or care about this temporary schedule object.
+    schedule = Schedule(start_date=course.start)
+    course_dates = api.get_dates_for_course(course.id, schedule=schedule)
+
     def visit(node):
         """
         Visit a node.  Checks to see if node has a due date and appends to
         `units` if it does.  Otherwise recurses into children to search for
         nodes with due dates.
         """
-        if getattr(node, 'due', None):
+        if (node.location, 'due') in course_dates:
             units.append(node)
         else:
             for child in node.get_children():
@@ -166,15 +173,35 @@ def set_due_date_extension(course, unit, student, due_date, actor=None, reason='
     if not mode:
         raise DashboardError(_("Could not find student enrollment in the course."))
 
-    if due_date:
+    # We normally set dates at the subsection level. But technically dates can be anywhere down the tree (and
-        try:
+    # usually are in self paced courses, where the subsection date gets propagated down).
-            api.set_date_for_block(course.id, unit.location, 'due', due_date, user=student, reason=reason, actor=actor)
+    # So find all children that we need to set the date on, then set those dates.
-        except api.MissingDateError:
+    course_dates = api.get_dates_for_course(course.id, user=student)
-            raise DashboardError(_(u"Unit {0} has no due date to extend.").format(unit.location))
+    blocks_to_set = {unit}  # always include the requested unit, even if it doesn't appear to have a due date now
-        except api.InvalidDateError:
+
-            raise DashboardError(_("An extended due date must be later than the original due date."))
+    def visit(node):
-    else:
+        """
-        api.set_date_for_block(course.id, unit.location, 'due', None, user=student, reason=reason, actor=actor)
+        Visit a node.  Checks to see if node has a due date and appends to
+        `blocks_to_set` if it does.  And recurses into children to search for
+        nodes with due dates.
+        """
+        if (node.location, 'due') in course_dates:
+            blocks_to_set.add(node)
+        for child in node.get_children():
+            visit(child)
+    visit(unit)
+
+    for block in blocks_to_set:
+        if due_date:
+            try:
+                api.set_date_for_block(course.id, block.location, 'due', due_date, user=student, reason=reason,
+                                       actor=actor)
+            except api.MissingDateError:
+                raise DashboardError(_(u"Unit {0} has no due date to extend.").format(unit.location))
+            except api.InvalidDateError:
+                raise DashboardError(_("An extended due date must be later than the original due date."))
+        else:
+            api.set_date_for_block(course.id, block.location, 'due', None, user=student, reason=reason, actor=actor)
 
 
 def dump_module_extensions(course, unit):