Add more useful links from issues (#3568)

* Link assessment instance from question preview when showing variant linked to instance question

* Some simplifications in code, lint fix.

* Update reference to variants_select in variants_insert

* Fixed reference to assessment

* Restructuring of solution, to avoid confusion with the use of locals.assessment.

* Unified queries in variants_select.

* Bugfixes

* Bug fixes

* Update to properly show assessment links on assessment overview as instructor.

* Change issue list page to link to instance question when there is an instance

* Use plainUrlPrefix instead of /pl

* Build instructor URL prefix manually

* Add dropdown with other options for links

* Bugfix: wrong dropdown open

* Add link to manual grading page

* Styling issues and adding dropdown in issues not linked to instance

* Fix #3304 and #1350

* Restore page size

* Indentation

Co-authored-by: Matthew West <mwest@illinois.edu>

* Move links to text beside title

Co-authored-by: Matthew West <mwest@illinois.edu>

pages/instructorIssues/instructorIssues.ejs

@@ -170,9 +170,16 @@
               <% } else { %>
               <i class="fa fa-check-circle text-success issue-status-icon"></i>
               <% } %>
-              <a class="d-block" href="<%= urlPrefix %>/question/<%= row.question_id %>/?variant_id=<%= row.variant_id %>">
-                <%= row.question_qid %>
-              </a>
+              <div class="d-block">
+                <strong><%= row.question_qid %></strong>
+                (<a href="<%= urlPrefix %>/question/<%= row.question_id %>/?variant_id=<%= row.variant_id %>">instructor
+                  view</a><% if (row.instance_question_id) { %>,
+                <a href="<%= plainUrlPrefix %>/course_instance/<%= row.course_instance_id %>/instance_question/<%= row.instance_question_id %>/?variant_id=<%= row.variant_id %>">student view</a>,
+                <% if (course.manual_grading_visible) { %>
+                <a href="<%= plainUrlPrefix %>/course_instance/<%= row.course_instance_id %>/instructor/instance_question/<%= row.instance_question_id %>/manual_grading">manual grading</a>,
+                <% } %>
+                <a href="<%= plainUrlPrefix %>/course_instance/<%= row.course_instance_id %>/instructor/assessment_instance/<%= row.assessment_instance_id %>">assessment details</a><% } %>)
+              </div>
               <p class="mb-0">
                 <%= getFormattedMessage(row) %>
               </p>

pages/instructorIssues/instructorIssues.js

@@ -108,21 +108,24 @@ router.get('/', function(req, res, next) {
         if (result.rowCount != 2) return next(new Error('unable to obtain issue count, rowCount = ' + result.rowCount));
         res.locals.closedCount = result.rows[0].count;
         res.locals.openCount = result.rows[1].count;
-        res.locals.issueCount = res.locals.closedCount + res.locals.openCount;
-
-        _.assign(res.locals, paginate.pages(req.query.page, res.locals.issueCount, PAGE_SIZE));
-        res.locals.shouldPaginate = res.locals.issueCount > PAGE_SIZE;
 
         var params = {
             course_id: res.locals.course.id,
-            offset: (res.locals.currPage - 1) * PAGE_SIZE,
+            offset: 0,
             limit: PAGE_SIZE,
         };
+        if (_.isInteger(Number(req.query.page)))
+            params.offset = (Number(req.query.page) - 1) * PAGE_SIZE;
         _.assign(params, filters);
 
         sqldb.query(sql.select_issues, params, function(err, result) {
             if (ERR(err, next)) return;
 
+            res.locals.issueCount = result.rowCount ? result.rows[0].issue_count : 0;
+
+            _.assign(res.locals, paginate.pages(req.query.page, res.locals.issueCount, PAGE_SIZE));
+            res.locals.shouldPaginate = res.locals.issueCount > PAGE_SIZE;
+
             // Add human-readable relative dates to each row
             result.rows.forEach((row) => {
                 row.relative_date = moment(row.formatted_date).from(row.now_date);

pages/instructorIssues/instructorIssues.sql

@@ -26,13 +26,17 @@ SELECT
     ci.short_name AS course_instance_short_name,
     CASE WHEN i.assessment_id IS NOT NULL THEN assessments_format(i.assessment_id) ELSE NULL END AS assessment,
     i.question_id,
+    i.instance_question_id,
+    iq.assessment_instance_id,
+    i.course_instance_id,
     q.directory AS question_qid,
     u.uid AS user_uid,
     u.name AS user_name,
     i.student_message,
     i.variant_id,
     i.open,
-    i.manually_reported
+    i.manually_reported,
+    COUNT(*) OVER() AS issue_count
 FROM
     issues_select_with_filter (
         $filter_is_open,
@@ -51,6 +55,7 @@ FROM
     LEFT JOIN assessments AS a ON (a.id = i.assessment_id)
     LEFT JOIN questions AS q ON (q.id = i.question_id)
     LEFT JOIN users AS u ON (u.user_id = i.user_id)
+    LEFT JOIN instance_questions AS iq ON (iq.id = i.instance_question_id)
 WHERE
     i.course_id = $course_id
     AND i.course_caused

pages/partials/instructorInfoPanel.ejs

@@ -69,26 +69,39 @@
   <% } %>
   <% } %>
 
-  <% if (typeof assessment !== "undefined") { %>
+  <% 
+     let ip_assessment = null;
+     let ip_assessment_instance = null;
+     if (typeof assessment_instance !== "undefined") {
+        ip_assessment = assessment;
+        ip_assessment_instance = assessment_instance;
+     }
+     if (typeof variant !== "undefined" && variant.assessment_instance) {
+        ip_assessment = variant.assessment;
+        ip_assessment_instance = variant.assessment_instance;
+     }
+     if (ip_assessment && ip_assessment_instance) {
+        let instructorUrlPrefix = `${plainUrlPrefix}/course_instance/${ip_assessment.course_instance_id}/instructor`;
+     %>
       <hr>
       <h5 class="card-title">Assessment Instance:</h5>
       <div class="d-flex flex-wrap">
           <div class="pr-1">AID:</div>
-          <div><a href="<%= urlPrefix %>/instructor/assessment/<%= assessment.id %>"><%= assessment.tid %></a></div>
+          <div><a href="<%= instructorUrlPrefix %>/assessment/<%= ip_assessment.id %>"><%= ip_assessment.tid %></a></div>
       </div>
 
       <div class="d-flex flex-wrap">
           <div class="pr-1">Started at: </div>
-          <div><%= assessment_instance.formatted_date %></div>
+          <div><%= ip_assessment_instance.formatted_date %></div>
       </div>
 
       <div class="d-flex flex-wrap">
           <div class="pr-1">Duration:</div>
-          <div><%= assessment_instance.duration %></div>
+          <div><%= ip_assessment_instance.duration %></div>
       </div>
 
       <div class="pb-2">
-          <a href="<%= urlPrefix %>/instructor/assessment_instance/<%= assessment_instance.id %>">View log</a>
+          <a href="<%= instructorUrlPrefix %>/assessment_instance/<%= ip_assessment_instance.id %>">View log</a>
       </div>
   <% } %>
 

sprocs/variants_select.sql

@@ -10,9 +10,17 @@ DECLARE
     course_instance_display_timezone text;
     course_display_timezone text;
 BEGIN
-    SELECT v.*
+    SELECT
+        v.*,
+        to_jsonb(a.*) AS assessment,
+        to_jsonb(ai.*) AS assessment_instance,
+        ai.date AS assessment_instance_date
     INTO variant_with_id
-    FROM variants as v
+    FROM
+        variants as v
+        LEFT JOIN instance_questions AS iq ON (iq.id = v.instance_question_id)
+        LEFT JOIN assessment_instances AS ai ON (ai.id = iq.assessment_instance_id)
+        LEFT JOIN assessments AS a ON (a.id = ai.assessment_id)
     WHERE v.id = variants_select.variant_id;
 
     IF NOT FOUND THEN RAISE EXCEPTION 'no such variant_id: %', variant_id; END IF;
@@ -34,6 +42,11 @@ BEGIN
     WHERE
         q.id = variant_with_id.question_id;
 
+    IF variant_with_id.instance_question_id IS NOT NULL THEN
+        variant_with_id.assessment_instance := jsonb_set(variant_with_id.assessment_instance,
+            '{formatted_date}', to_jsonb(format_date_full_compact(variant_with_id.assessment_instance_date, COALESCE(course_instance_display_timezone, course_display_timezone))));
+    END IF;
+
     variant := jsonb_set(to_jsonb(variant_with_id.*), '{formatted_date}',
                          to_jsonb(format_date_full_compact(variant_with_id.date, COALESCE(course_instance_display_timezone, course_display_timezone))));
 END;