Merge pull request #7295 from donny-wong/v2.5.3

V2.5.3

Changelog.md

@@ -1,5 +1,13 @@
 # Changelog
 
+## [v2.5.3]
+
+### 🐛 Bug fixes
+
+- Fix Marks Spreadsheet csv bug of showing incorrect marks (#7257)
+- Fix incorrect inclusion of course parameter in LtiSyncJob (#7258)
+- Fix Google Colab Jupyter Notebooks rendering by excluding widgets (#7271)
+
 ## [v2.5.2]
 
 ### ✨ New features and improvements

Gemfile.lock

@@ -472,7 +472,7 @@ GEM
       addressable (>= 2.8.0)
       crack (>= 0.3.2)
       hashdiff (>= 0.4.0, < 2.0.0)
-    webrick (1.8.1)
+    webrick (1.8.2)
     websocket (1.2.11)
     websocket-driver (0.7.6)
       websocket-extensions (>= 0.1.0)

app/MARKUS_VERSION

@@ -1 +1 @@
-VERSION=v2.5.2,PATCH_LEVEL=DEV
+VERSION=v2.5.3,PATCH_LEVEL=DEV

app/controllers/assignments_controller.rb

@@ -628,7 +628,7 @@ def upload_config_files
   def create_lti_grades
     assessment = record
     lti_deployments = LtiDeployment.where(course: assessment.course, id: params[:lti_deployments])
-    @current_job = LtiSyncJob.perform_later(lti_deployments.to_a, assessment, current_course,
+    @current_job = LtiSyncJob.perform_later(lti_deployments.to_a, assessment,
                                             can_create_users: allowed_to?(:lti_manage?, with: UserPolicy),
                                             can_create_roles: allowed_to?(:manage?, with: RolePolicy))
     session[:job_id] = @current_job.job_id

app/controllers/submissions_controller.rb

@@ -1,3 +1,5 @@
+require 'json'
+
 class SubmissionsController < ApplicationController
   include SubmissionsHelper
   include RepositoryHelper
@@ -937,6 +939,11 @@ def notebook_to_html(file_contents, unique_path, type)
           '--template', 'markus-html-template'
         ]
       end
+      file_contents = JSON.parse(file_contents)
+      if file_contents['metadata'].key?('widgets')
+        file_contents['metadata'].delete('widgets')
+      end
+      file_contents = JSON.generate(file_contents)
       _stdout, stderr, status = Open3.capture3(*args, stdin_data: file_contents)
       return "#{I18n.t('submissions.cannot_display')}<br/><br/>#{stderr.lines.last}" unless status.exitstatus.zero?
 

app/jobs/lti_sync_job.rb

@@ -9,12 +9,12 @@ def self.completed_message(_status)
     I18n.t('lti.grade_sync_complete')
   end
 
-  def perform(lti_deployments, assessment, course, can_create_users: false, can_create_roles: false)
+  def perform(lti_deployments, assessment, can_create_users: false, can_create_roles: false)
     if lti_deployments.empty?
       raise I18n.t('lti.no_platform')
     end
     lti_deployments.each do |deployment|
-      roster_error = roster_sync(deployment, course,
+      roster_error = roster_sync(deployment,
                                  [LtiDeployment::LTI_ROLES[:learner], LtiDeployment::LTI_ROLES[:ta]],
                                  can_create_users: can_create_users, can_create_roles: can_create_roles)
       if roster_error

app/models/grade_entry_form.rb

@@ -122,11 +122,10 @@ def export_as_csv(role)
                        .group_by { |x| x[0] }
       num_items = self.grade_entry_items.count
     elsif role.ta?
-      grade_data = role.grade_entry_students
-                       .joins(:user)
-                       .joins(:grades)
-                       .joins(:grade_entry_items)
-                       .where(grade_entry_form: self)
+      grade_data = self.grades
+                       .joins(:grade_entry_item, grade_entry_student: :user)
+                       .joins(grade_entry_student: :grade_entry_student_tas)
+                       .where('grade_entry_student_tas.ta_id': role.id)
                        .pluck('users.user_name', 'grade_entry_items.position', 'grades.grade')
                        .group_by { |x| x[0] }
       num_items = self.grade_entry_items.count

spec/controllers/submissions_controller_spec.rb

@@ -1665,6 +1665,19 @@
         it_behaves_like 'notebook content'
       end
 
+      context 'a jupyter-notebook file with widgets',
+              skip: Rails.application.config.nbconvert_enabled ? false : 'nbconvert dependencies not installed' do
+        render_views
+        let(:filename) { 'example_widgets.ipynb' }
+
+        it 'renders without widgets' do
+          subject
+          expect(response.body).not_to include("KeyError: 'state'")
+        end
+
+        it_behaves_like 'notebook content'
+      end
+
       context 'an rmarkdown file' do
         let(:filename) { 'example.Rmd' }
 

spec/fixtures/files/example_widgets.ipynb

@@ -0,0 +1,52 @@
+{
+  "nbformat": 4,
+  "nbformat_minor": 0,
+  "metadata": {
+    "colab": {
+      "provenance": []
+    },
+    "kernelspec": {
+      "name": "python3",
+      "display_name": "Python 3"
+    },
+    "language_info": {
+      "name": "python"
+    },
+    "widgets": {
+      "application/vnd.jupyter.widget-state+json": {
+      }
+    }
+  },
+  "cells": [
+    {
+      "cell_type": "code",
+      "source": [
+      ],
+      "metadata": {
+        "id": "wekaZO6DYWz6",
+        "colab": {
+          "base_uri": "https://localhost:8080/",
+          "height": 1000
+        },
+        "outputId": "990f2494-5ca0-442f-ba8d-f7a31bd463e4"
+      },
+      "execution_count": 17,
+      "outputs": [
+        {
+          "output_type": "display_data",
+          "data": {
+            "text/plain": [
+              "Training:   0%|          | 0/1000 [00:00<?, ?it/s]"
+            ],
+            "application/vnd.jupyter.widget-view+json": {
+              "version_major": 2,
+              "version_minor": 0,
+              "model_id": "92b45d943b9e4c83b50bb2c83505e80d"
+            }
+          },
+          "metadata": {}
+        }
+      ]
+    }
+  ]
+}

spec/jobs/lti_sync_job_spec.rb

@@ -29,7 +29,7 @@
   end
 
   context 'when running as a background job' do
-    let(:job_args) { [[lti_deployment.id], assessment, course] }
+    let(:job_args) { [[lti_deployment.id], assessment] }
 
     include_examples 'background job'
   end
@@ -43,7 +43,7 @@
   end
 
   context 'with no lti deployments' do
-    let(:job_args) { [[], assessment, course] }
+    let(:job_args) { [[], assessment] }
 
     it 'should raise an error' do
       expect { LtiSyncJob.perform_now(*job_args) }.to raise_error(RuntimeError)

spec/models/grade_entry_form_spec.rb

@@ -385,4 +385,47 @@
       end
     end
   end
+
+  describe 'when TA downloads a csv file of the grades' do
+    let(:ta) { create(:ta) }
+    let!(:student1) { create(:student) }
+    let!(:student2) { create(:student) }
+    let!(:grade_entry_form_with_data) { create(:grade_entry_form_with_data) }
+
+    before do
+      grade_entry_student1 = GradeEntryStudent.find_by(role: student1)
+      grade_entry_student2 = GradeEntryStudent.find_by(role: student2)
+
+      GradeEntryStudentTa.create(grade_entry_student: grade_entry_student1, ta: ta)
+      GradeEntryStudentTa.create(grade_entry_student: grade_entry_student2, ta: ta)
+
+      item2 = create(:grade_entry_item, name: 'Test2', position: 2, grade_entry_form: grade_entry_form_with_data)
+
+      grade_entry_student1.grades.create(grade: Random.rand(item2.out_of), grade_entry_item: item2)
+      grade_entry_student1.save
+
+      grade_entry_student2.grades.create(grade: Random.rand(item2.out_of), grade_entry_item: item2)
+      grade_entry_student2.save
+
+      grade_spreadsheet = grade_entry_form_with_data.export_as_csv(ta)
+      results = CSV.parse(grade_spreadsheet, headers: false).drop(2)
+
+      @res_marks = {}
+      results.each do |x|
+        mark1 = x[6] == '' ? nil : x[6].to_f
+        mark2 = x[7] == '' ? nil : x[7].to_f
+        @res_marks[x[0]] = [mark1, mark2]
+      end
+
+      @student_marks = {}
+      @student_marks[student1.user.user_name] =
+        [grade_entry_student1.grades.first.grade, grade_entry_student1.grades.second.grade]
+      @student_marks[student2.user.user_name] =
+        [grade_entry_student2.grades.first.grade, grade_entry_student2.grades.second.grade]
+    end
+
+    it 'correctly displays the marks' do
+      expect(@res_marks).to eq(@student_marks)
+    end
+  end
 end