diff --git a/app/api/test_attempts_api.rb b/app/api/test_attempts_api.rb index 7196e6b62..97988d857 100644 --- a/app/api/test_attempts_api.rb +++ b/app/api/test_attempts_api.rb @@ -77,7 +77,7 @@ class TestAttemptsApi < Grape::API attempt.review # TODO: add review permission flag to taskdef end - present test, with: Entities::TestAttemptEntity + present attempt, with: Entities::TestAttemptEntity end desc 'Initiate a new test attempt' @@ -100,7 +100,7 @@ class TestAttemptsApi < Grape::API return end - metadata = params.merge(attempt_number: test_count + 1) + metadata = { task_id: task.id, attempt_number: test_count + 1 } test = TestAttempt.create!(metadata) present test, with: Entities::TestAttemptEntity end @@ -110,19 +110,25 @@ class TestAttemptsApi < Grape::API requires :id, type: String, desc: 'ID of the test attempt' optional :cmi_datamodel, type: String, desc: 'JSON CMI datamodel to update' optional :terminated, type: Boolean, desc: 'Terminate the current attempt' + optional :success_status, type: Boolean, desc: 'Override the success status of the current attempt' end patch 'test_attempts/:id' do - attempt_data = ActionController::Parameters.new(params).permit(:cmi_datamodel, :terminated) test = TestAttempt.find(params[:id]) - unless test.terminated - test.update!(attempt_data) - test.save! - if params[:terminated] - task = Task.find(test.task_id) - task.add_scorm_comment(test) + if params[:success_status].present? + test.override_success_status(params[:success_status]) + else + attempt_data = ActionController::Parameters.new(params).permit(:cmi_datamodel, :terminated) + + unless test.terminated + test.update!(attempt_data) + test.save! + if params[:terminated] + test.add_scorm_comment + end end end + present test, with: Entities::TestAttemptEntity end @@ -131,10 +137,9 @@ class TestAttemptsApi < Grape::API requires :id, type: String, desc: 'ID of the test attempt' end delete 'test_attempts/:id' do - raise NotImplementedError # TODO: fix permissions before enabling this - # test = TestAttempt.find(params[:id]) - # test.destroy! + test = TestAttempt.find(params[:id]) + test.destroy! end end diff --git a/app/models/comments/scorm_comment.rb b/app/models/comments/scorm_comment.rb new file mode 100644 index 000000000..16502f50a --- /dev/null +++ b/app/models/comments/scorm_comment.rb @@ -0,0 +1,16 @@ +class ScormComment < TaskComment + belongs_to :test_attempt, optional: false + + before_create do + self.content_type = :scorm + end + + def serialize(user) + json = super(user) + json[:test_attempt] = { + id: self.test_attempt_id, + success_status: self.test_attempt.success_status + } + json + end +end diff --git a/app/models/task.rb b/app/models/task.rb index 291d89366..c0d0f620c 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -107,6 +107,7 @@ def specific_permission_hash(role, perm_hash, _other) has_many :task_submissions, dependent: :destroy has_many :overseer_assessments, dependent: :destroy has_many :tii_submissions, dependent: :destroy + has_many :test_attempts, dependent: :destroy delegate :unit, to: :project delegate :student, to: :project @@ -652,18 +653,6 @@ def add_text_comment(user, text, reply_to_id = nil) comment end - def add_scorm_comment(test) - comment = TaskComment.create - comment.task = self - comment.user = self.tutor - comment.comment = "Test Attempt #{test.attempt_number} completed with score: #{format("%.2f", test.score_scaled * 100)}%" - comment.content_type = 'scorm' - comment.recipient = project.student - comment.save! - - comment - end - def individual_task_or_submitter_of_group_task? return true if !group_task? # its individual return true unless group.present? # no group yet... so individual diff --git a/app/models/test_attempt.rb b/app/models/test_attempt.rb index 7807de244..b55571214 100644 --- a/app/models/test_attempt.rb +++ b/app/models/test_attempt.rb @@ -2,6 +2,11 @@ require 'time' class TestAttempt < ApplicationRecord + belongs_to :task, optional: false + + has_one :scorm_comment, dependent: :destroy + + validates :task_id, presence: true def self.permissions # TODO: this is all wrong, students should not be able to delete test attempts @@ -99,8 +104,36 @@ def review write_attribute(:cmi_datamodel, dm.to_json) end - def pass_override - # TODO: implement tutor override pass + def override_success_status(new_success_status) + dm = JSON.parse(self.cmi_datamodel) + dm['cmi.success_status'] = (new_success_status ? 'passed' : 'failed') + write_attribute(:cmi_datamodel, dm.to_json) + self.success_status = dm['cmi.success_status'] == 'passed' + self.save! + self.update_scorm_comment end + def add_scorm_comment + comment = ScormComment.create + comment.task = task + comment.user = task.tutor + comment.comment = "Test attempt #{self.attempt_number} #{self.success_status ? 'passed' : 'failed'} with score: #{(self.score_scaled * 100).to_i}%" + comment.recipient = task.student + comment.test_attempt = self + comment.save! + + comment + end + + def update_scorm_comment + if self.scorm_comment.present? + self.scorm_comment.comment = "Test attempt #{self.attempt_number} #{self.success_status ? 'passed' : 'failed'} with score: #{(self.score_scaled * 100).to_i}%" + self.scorm_comment.save! + + return self.scorm_comment + end + + puts "WARN: Unexpected need to create scorm comment for test attempt: #{self.id}" + add_scorm_comment + end end diff --git a/db/migrate/20240601103707_add_test_attempt_link_to_comment.rb b/db/migrate/20240601103707_add_test_attempt_link_to_comment.rb new file mode 100644 index 000000000..51db18b9e --- /dev/null +++ b/db/migrate/20240601103707_add_test_attempt_link_to_comment.rb @@ -0,0 +1,7 @@ +class AddTestAttemptLinkToComment < ActiveRecord::Migration[7.1] + def change + # Link to corresponding SCORM test attempt for scorm comments + add_column :task_comments, :test_attempt_id, :integer + add_index :task_comments, :test_attempt_id + end +end diff --git a/db/schema.rb b/db/schema.rb index 79932c092..2808350aa 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_03_22_021829) do +ActiveRecord::Schema[7.1].define(version: 2024_06_01_103707) do create_table "activity_types", charset: "utf8", collation: "utf8_unicode_ci", force: :cascade do |t| t.string "name", null: false t.string "abbreviation", null: false @@ -215,6 +215,7 @@ t.string "extension_response" t.bigint "reply_to_id" t.bigint "overseer_assessment_id" + t.integer "test_attempt_id" t.index ["assessor_id"], name: "index_task_comments_on_assessor_id" t.index ["discussion_comment_id"], name: "index_task_comments_on_discussion_comment_id" t.index ["overseer_assessment_id"], name: "index_task_comments_on_overseer_assessment_id" @@ -222,6 +223,7 @@ t.index ["reply_to_id"], name: "index_task_comments_on_reply_to_id" t.index ["task_id"], name: "index_task_comments_on_task_id" t.index ["task_status_id"], name: "index_task_comments_on_task_status_id" + t.index ["test_attempt_id"], name: "index_task_comments_on_test_attempt_id" t.index ["user_id"], name: "index_task_comments_on_user_id" end