Merge pull request #81 from Harvard-University-iCommons/elliottyates/…

…tlt-4024/multiple_section_action_bug

TLT-4024: Only delete one section at a time

README.md

@@ -1,3 +1,51 @@
 # Canvas Manage Course
 
-This project provides a collection of tools used to administer Harvard TLT's instance of Canvas at the course level.
+This project provides a collection of utilies used to administer Harvard Academic Technology's instance of Canvas at the course level. The project is an LTI tool, i.e. installed via LTI.
+
+## Deploying
+
+Uses the tlt-ops `django_deploy` playbook.
+
+## Running locally
+
+### Install requirements
+
+```sh
+pip install -r canvas_manage_course/requirements/local.txt
+```
+
+Particular difficulties are sometimes found when installing the following directly on Mac OS X in a python environment:
+
+* [psycopg2](https://wiki.harvard.edu/confluence/display/k459/Installing+psycopg2%3E%3D2.8+on+macos) (at install time)
+* [cx_Oracle](https://wiki.harvard.edu/confluence/display/k459/Using+cx_Oracle+on+mac+OS+X) (at run time, if oracle instant client is not available or configured properly)
+
+### Run
+
+Run, and ensure the ENV environment var will be able to point the django-ssm-parameter-store library to appropriate SSM params (either locally, via a file, or on SSM, in which case ensure your local default AWS profile is authenticated to the correct environment and that you're on VPN so you can connect to non-local databases and caches).
+
+Note that the django-sslserver default server and port is 127.0.0.1:8000. If your LTI configuration was set up with a different port, e.g. 8443, you'll need to specify it as in the snippet below.
+
+```sh
+export ENV=dev
+export DJANGO_SETTINGS_MODULE=canvas_manage_course.settings.local
+# ensure you're connected to VPN
+python manage.py runsslserver
+# to use a different port:
+# python manage.py runsslserver 127.0.0.1:8443
+```
+
+Access at https://local.tlt.harvard.edu:(port)/.
+
+### Authorize
+
+This tool uses the django-canvas-lti-school-permissions library, which authorizes LTI launches against a database of permissions. The permissions are based on the subaccount that a course belongs to. So when launching from a test course in the AcTS (Academic Technology) subaccount in Canvas, an authorization row should be present for school `acts` or `*` and the main tool, `canvas_manage_course`, along with any specific utilities present in the dashboard, e.g. The tool uses the database name `canvas_course_admin_tools` and the appropriate permissions table is `lti_school_permissions_schoolpermission`.
+
+For example:
+
+| id | permission | canvas\_role | school\_id |
+| :--- | :--- | :--- | :--- |
+| 1 | canvas\_manage\_course | Account Admin | acts |
+| 22 | class\_roster | Account Admin | acts |
+| 64 | manage\_people | Account Admin | acts |
+| 1050 | manage\_school\_permissions | Account Admin | acts |
+| 85 | manage\_sections | Account Admin | acts |

canvas_manage_course/requirements/base.txt

@@ -1,5 +1,5 @@
 boto3==1.9.210
-Django==2.2.13
+Django==2.2.23
 cx-Oracle==7.2.2
 django-allow-cidr==0.3.1
 django-cached-authentication-middleware==0.2.2

canvas_manage_course/requirements/local.txt

@@ -1,11 +1,11 @@
 # local environment requirements
 
 -r aws.txt
- 
+
 # below are requirements specific to the local environment
 
 ddt==1.1.1
-django-debug-toolbar==1.9
+django-debug-toolbar==3.2.1
 django-sslserver==0.21
 
 

canvas_manage_course/settings/base.py

@@ -13,7 +13,7 @@
 # Build paths inside the project like this: os.path.join(BASE_DIR, ...)
 import logging
 import os
-from icommons_common.logging import JSON_LOG_FORMAT, ContextFilter
+from icommons_common.logging import JSON_LOG_FORMAT
 
 from django.urls import reverse_lazy
 
@@ -187,7 +187,6 @@
 STATIC_ROOT = os.path.normpath(os.path.join(BASE_DIR, 'http_static'))
 
 _DEFAULT_LOG_LEVEL = SECURE_SETTINGS.get('log_level', logging.DEBUG)
-_LOG_ROOT = SECURE_SETTINGS.get('log_root', '')  # Default to current directory
 
 
 # Turn off default Django logging
@@ -226,7 +225,6 @@
     },
 
     'handlers': {
-        # Log to a file by default that can be rotated by logrotate
         'default': {
             'class': 'splunk_handler.SplunkHandler',
             'formatter': 'json',
@@ -237,7 +235,6 @@
             'index': 'soc-isites',
             'token': SECURE_SETTINGS['splunk_token'],
             'level': _DEFAULT_LOG_LEVEL,
-            # 'filename': os.path.join(_LOG_ROOT, 'django-canvas_manage_course.log'),
             'filters': ['context'],
         },
         'gunicorn': {

canvas_manage_course/settings/local.py

@@ -11,7 +11,7 @@
 # For Django Debug Toolbar:
 INTERNAL_IPS = ('127.0.0.1', '10.0.2.2',)
 
-# Log to console instead of a file when running locally
+# Log to console instead of splunk when running locally
 LOGGING['handlers']['default'] = {
     'level': logging.DEBUG,
     'class': 'logging.StreamHandler',

manage_sections/templates/manage_sections/create_section_form.html

@@ -425,7 +425,7 @@ <h4 class="modal-title" id="confirmDelSectionLabel">Confirm Delete</h4>
     }
 
     function addEnrollmentCountText(enrollmentCount) {
-        modalBody = $(".modal-body")
+        var modalBody = $(".modal-body");
         if (enrollmentCount > 0) {
             modalBody.text(
                 `Permanently delete this section? There are currently ${enrollmentCount} enrollments in this section.`
@@ -435,41 +435,46 @@ <h4 class="modal-title" id="confirmDelSectionLabel">Confirm Delete</h4>
         }
     }
 
-    $("#sectionsMenu").on('click', '.remove_data', function(e) {
-        var url = this.href;
-        var userLine = $(this).closest("li");
-        var enrollmentCount = parseInt(userLine.find(".sectionCount").text());
-
-        addEnrollmentCountText(enrollmentCount)
-        $('#confirmDelSection').modal({ keyboard:false }) 
-        .one('click', '#btnConfirmDelSection', function() {
-            $.ajax({
-                url: url,
-                type : "POST",
-                success : function(json) {
-                    $("#message").hide();
-                    userLine.fadeOut('slow', function(){
-                        userLine.remove();
-                    });
-                    show_if_none();
-                },
-                error : function(xhr) {
-                    $("#message").addClass('alert alert-danger').show();
-                    if (xhr.status ==422){
-                         $("#message").text(xhr.responseText);
-                    }
-                    else{
-                        $("#message").text("Error: Section has not been deleted ");
-                    }
+    function onClickRemoveSection(e) {
+        e.preventDefault(); // do not allow browser to load the href URL
+        const currentSectionRow = $(this).closest('li');
+        $('#btnConfirmDelSection').data({url: this.href, sectionId: currentSectionRow.data('sectionId')});
+        const enrollmentCount = parseInt(currentSectionRow.find(".sectionCount").text());
+        addEnrollmentCountText(enrollmentCount);
+        $('#confirmDelSection').modal({ keyboard:false }).show();
+    }
+
+    function onClickConfirmRemoveSection(e) {
+        const removeUrl = $(this).data('url');
+        const sectionId = $(this).data('sectionId');
+        const sectionRow = $('[data-section-id="' + sectionId + '"]');
+        $.ajax({
+            url: removeUrl,
+            type : "POST",
+            success : function(json) {
+                $("#message").hide();
+                sectionRow.fadeOut('slow', function() {
+                    sectionRow.remove();
+                });
+                show_if_none();
+            },
+            error : function(xhr) {
+                $("#message").addClass('alert alert-danger').show();
+                if (xhr.status == 422) {
+                        $("#message").text(xhr.responseText);
+                }
+                else {
+                    $("#message").text("Error: Section has not been deleted ");
                 }
-            })
-            .always(function() {
-                $('#confirmDelSection').modal('hide');
-            });
+            }
+        })
+        .always(function() {
+            $('#confirmDelSection').modal('hide');
         });
-        e.preventDefault();
-    });
+    }
 
+    $("#sectionsMenu").on('click', '.remove_data', onClickRemoveSection);
+    $("#btnConfirmDelSection").on('click', onClickConfirmRemoveSection);
 });
 </script>
 {% endblock %}

manage_sections/templates/manage_sections/section_list.html

@@ -1,4 +1,4 @@
-<li id="{{section.id}}"  class="list-group-item courseSection">
+<li id="section-{{section.id}}" class="list-group-item courseSection" data-section-id="{{section.id}}">
     <div class="deleteMenu">
     {% if section.registrar_section_flag %}
         <a href="#" class="disable-delete lti-tooltip" rel="tooltip" data-toggle="tooltip" title="You can't delete sections that have been created outside of Canvas" data-original-title="You can't delete sections that have been created outside of Canvas"><i class="fa fa-trash-o"></i></a>

manage_sections/utils.py

@@ -1,12 +1,13 @@
+import logging
 import re
+
+from django.conf import settings
+
 from canvas_sdk.methods import (
     sections,
     enrollments as canvas_api_enrollments
 )
 from canvas_sdk.exceptions import CanvasAPIError
-from django.conf import settings
-from django.http import HttpResponse
-
 from icommons_common.canvas_utils import SessionInactivityExpirationRC
 from icommons_common.models import CourseInstance
 from icommons_common.canvas_api.helpers import (
@@ -15,8 +16,8 @@
     sections as canvas_api_helper_sections
 )
 
-from django.shortcuts import render
-from django.utils.safestring import mark_safe
+
+logger = logging.getLogger(__name__)
 
 # Set up the request context that will be used for canvas API calls
 SDK_CONTEXT = SessionInactivityExpirationRC(**settings.CANVAS_SDK_SETTINGS)
@@ -130,6 +131,11 @@ def delete_enrollments(enrollments, course_id):
     Delete a list of enrollments via Canvas API.
     Clears cache of section, enrollment, and
     course data.
+
+    Returns a tuple `(deleted_enrollments, is_empty)`:
+    - `deleted_enrollments` is a list of successful Canvas SDK responses
+    - `is_empty` is a boolean indicating to the caller whether the section
+    was successfully emptied (or was already empty)
     """
     is_empty = False
     deleted_enrollments = []
@@ -141,14 +147,31 @@ def delete_enrollments(enrollments, course_id):
     for enrollment in enrollments:
         user_section_id = enrollment.get('id', '')
         if not user_section_id:
+            logger.error(
+                f'Unexpected error while concluding enrollment for '
+                f'course_id:{course_id}; no user_section_id available '
+                f'for enrollment {enrollment}',
+                extra={
+                    'deleted_enrollments': deleted_enrollments,
+                    'enrollment_causing_error': enrollment,
+                    'enrollments_requested': enrollments,
+                }
+            )
             return (deleted_enrollments, is_empty)
+        response = None
         try:
             response = canvas_api_enrollments.conclude_enrollment(
                 SDK_CONTEXT, course_id, user_section_id, 'delete'
             )
-        except CanvasAPIError:
+        except CanvasAPIError as e:
+            logger.exception(
+                f'Unexpected error while concluding enrollment for '
+                f'user_section_id:{user_section_id}, '
+                f'course_id:{course_id}',
+                extra={'canvas_api_error': getattr(e, 'error_json', None)},
+            )
             canvas_api_helper_sections.delete_cache(course_id)
-            return (delete_enrollments, is_empty)
+            return (deleted_enrollments, is_empty)
 
         canvas_api_helper_sections.delete_section_cache(user_section_id)
         deleted_enrollments.append(response)

manage_sections/views.py

@@ -331,19 +331,16 @@ def remove_section(request, section_id):
         )
 
     enrollments = canvas_api_enrollments.list_enrollments_sections(SDK_CONTEXT, section_id).json()
-    if len(enrollments) == 0:
-        section = canvas_api_helper_sections.delete_section(canvas_course_id, section_id)
-        return JsonResponse(section)
-
-    # delete enrollments, then delete section
-    responses, is_empty = delete_enrollments(enrollments, canvas_course_id)
-    if not is_empty:
-        return JsonResponse({
-            'message': f'Issue clearing enrollments prior to deletion. Unable to delete section {section_id} from course {canvas_course_id}'},
-            status=500
-        )
-    section = canvas_api_helper_sections.delete_section(canvas_course_id, section_id)
+    if len(enrollments) > 0:
+        # delete enrollments before deleting section
+        responses, is_empty = delete_enrollments(enrollments, canvas_course_id)
+        if not is_empty:
+            return JsonResponse({
+                'message': f'Issue clearing enrollments prior to deletion. Unable to delete section {section_id} from course {canvas_course_id}'},
+                status=500
+            )
 
+    section = canvas_api_helper_sections.delete_section(canvas_course_id, section_id)
     return JsonResponse(section)