Merge pull request #6938 from mitocw/bdero/gitlog-pagination

Add pagination to sysadmin dashboard Git import logs

lms/djangoapps/dashboard/sysadmin.py

@@ -15,6 +15,7 @@ from django.contrib.auth import authenticate
 from django.contrib.auth.decorators import login_required
 from django.contrib.auth.models import User
 from django.core.exceptions import PermissionDenied
+from django.core.paginator import Paginator, PageNotAnInteger, EmptyPage
 from django.db import IntegrityError
 from django.http import HttpResponse, Http404
 from django.utils.decorators import method_decorator
@@ -684,6 +685,8 @@ class GitLogs(TemplateView):
         if course_id:
             course_id = SlashSeparatedCourseKey.from_deprecated_string(course_id)
 
+        page_size = 10
+
         # Set mongodb defaults even if it isn't defined in settings
         mongo_db = {
             'host': 'localhost',
@@ -715,7 +718,7 @@ class GitLogs(TemplateView):
             # Require staff if not going to specific course
             if not request.user.is_staff:
                 raise Http404
-            cilset = CourseImportLog.objects.all().order_by('-created')
+            cilset = CourseImportLog.objects.order_by('-created')
         else:
             try:
                 course = get_course_by_id(course_id)
@@ -729,11 +732,29 @@ class GitLogs(TemplateView):
                     CourseStaffRole(course.id).has_user(request.user)):
                 raise Http404
             log.debug('course_id={0}'.format(course_id))
-            cilset = CourseImportLog.objects.filter(course_id=course_id).order_by('-created')
+            cilset = CourseImportLog.objects.filter(
+                course_id=course_id
+            ).order_by('-created')
             log.debug('cilset length={0}'.format(len(cilset)))
+
+        # Paginate the query set
+        paginator = Paginator(cilset, page_size)
+        try:
+            logs = paginator.page(request.GET.get('page'))
+        except PageNotAnInteger:
+            logs = paginator.page(1)
+        except EmptyPage:
+            # If the page is too high or low
+            given_page = int(request.GET.get('page'))
+            page = min(max(1, given_page), paginator.num_pages)
+            logs = paginator.page(page)
+
         mdb.disconnect()
-        context = {'cilset': cilset,
+        context = {
+            'logs': logs,
             'course_id': course_id.to_deprecated_string() if course_id else None,
-                   'error_msg': error_msg}
+            'error_msg': error_msg,
+            'page_size': page_size
+        }
 
         return render_to_response(self.template_name, context)

lms/djangoapps/dashboard/tests/test_sysadmin.py

@@ -25,6 +25,7 @@ from xmodule.modulestore.tests.django_utils import (
 from dashboard.models import CourseImportLog
 from dashboard.sysadmin import Users
 from dashboard.git_import import GitImportError
+from datetime import datetime
 from external_auth.models import ExternalAuthMap
 from student.roles import CourseStaffRole, GlobalStaff
 from student.tests.factories import UserFactory
@@ -581,6 +582,40 @@ class TestSysAdminMongoCourseImport(SysadminBaseTestCase):
 
         self._rm_edx4edx()
 
+    def test_gitlog_pagination_out_of_range_invalid(self):
+        """
+        Make sure the pagination behaves properly when the requested page is out
+        of range.
+        """
+
+        self._setstaff_login()
+
+        mongoengine.connect(TEST_MONGODB_LOG['db'])
+
+        for _ in xrange(15):
+            CourseImportLog(
+                course_id=SlashSeparatedCourseKey("test", "test", "test"),
+                location="location",
+                import_log="import_log",
+                git_log="git_log",
+                repo_dir="repo_dir",
+                created=datetime.now()
+            ).save()
+
+        for page, expected in [(-1, 1), (1, 1), (2, 2), (30, 2), ('abc', 1)]:
+            response = self.client.get(
+                '{}?page={}'.format(
+                    reverse('gitlogs'),
+                    page
+                )
+            )
+            self.assertIn(
+                'Page {} of 2'.format(expected),
+                response.content
+            )
+
+        CourseImportLog.objects.delete()
+
     def test_gitlog_courseteam_access(self):
         """
         Ensure course team users are allowed to access only their own course.

lms/templates/sysadmin_dashboard_gitlogs.html

@@ -30,6 +30,28 @@
       });
   </script>
 </%block>
+<%def name="pagination()">
+    <div class="pagination">
+        %if logs.has_previous():
+            <span class="previous-page">
+              <a href="?page=${logs.previous_page_number()}">
+                ${_("previous")}
+              </a>
+            </span>
+        %endif
+        ${_("Page {current_page} of {total_pages}".format(
+            current_page=logs.number,
+            total_pages=logs.paginator.num_pages
+        ))}
+        %if logs.has_next():
+            <span class="next-page">
+              <a href="?page=${logs.next_page_number()}">
+                ${_("next")}
+              </a>
+            </span>
+        %endif
+      </div>
+</%def>
 <style type="text/css">
 a.active-section {
 	color: #551A8B;
@@ -63,6 +85,19 @@ table.stat_table td {
     display: none;
 }
 
+.pagination, .page-status {
+    text-align: center;
+    padding: 12px 0 12px 0;
+}
+
+.pagination .previous-page {
+    padding-right: 10px;
+}
+
+.pagination .next-page {
+    padding-left: 10px;
+}
+
 a.selectedmode { background-color: yellow; }
 
 textarea {
@@ -102,6 +137,9 @@ textarea {
           %endif
       %endif
 
+      %if len(logs):
+          ${pagination()}
+
           <table class="stat_table" width="100%">
             <thead>
               <tr>
@@ -112,15 +150,7 @@ textarea {
               </tr>
             </thead>
             <tbody>
-          <%
-          if course_id == None:
-              logs = cilset[:10]
-          else:
-              logs = cilset[:5]
-
-          cil = None
-          %>
-          % for index, cil in enumerate(logs):
+              %for index, cil in enumerate(logs):
                   <%
                       # Appropriate datetime string for current locale and timezone
                       date = get_time_display(cil.created.replace(tzinfo=UTC),
@@ -155,11 +185,21 @@ textarea {
             </tbody>
           </table>
 
-      ## If viewing a single course and there are no logs available, let the user know
-      %if course_id is not None and cil is None:
+          ${pagination()}
+      %else:
+          <div class="page-status">
+            %if not course_id:
+                # If viewing all logs there are no logs available, let the user know
+                ## Translators: git is a version-control system; see http://git-scm.com/about
+                ${_('No git import logs have been recorded.')}
+            %else:
+                # If viewing a single course and there are no logs available, let the user know
                 ## Translators: git is a version-control system; see http://git-scm.com/about
                 ${_('No git import logs have been recorded for this course.')}
             %endif
+          </div>
+      %endif
+
     </section>
   </div>
 </section>