Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: store Numbas config in backend #16

Merged
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion app/api/api_root.rb
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,6 @@ class ApiRoot < Grape::API
mount Tii::TiiGroupAttachmentApi
mount Tii::TiiActionApi


mount NumbasApi
mount TestAttemptsApi
mount CampusesPublicApi
Expand Down
1 change: 0 additions & 1 deletion app/api/entities/numbas_entity.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

module Entities
class NumbasEntity < Grape::Entity
expose :file_content, documentation: { type: 'string', desc: 'File content' }
Expand Down
4 changes: 4 additions & 0 deletions app/api/entities/task_definition_entity.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,10 @@ def staff?(my_role)
expose :has_task_sheet?, as: :has_task_sheet
expose :has_task_resources?, as: :has_task_resources
expose :has_task_assessment_resources?, as: :has_task_assessment_resources, if: ->(unit, options) { staff?(options[:my_role]) }
expose :has_numbas_data?, as: :has_numbas_data
expose :has_enabled_numbas_test, if: ->(unit, options) { staff?(options[:my_role]) }
expose :numbas_time_delay, if: ->(unit, options) { staff?(options[:my_role]) }
expose :numbas_attempt_limit, if: ->(unit, options) { staff?(options[:my_role]) }
expose :is_graded
expose :max_quality_pts
expose :overseer_image_id, if: ->(unit, options) { staff?(options[:my_role]) }
Expand Down
1 change: 0 additions & 1 deletion app/api/entities/test_attempt_entity.rb
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@

module Entities
class TestAttemptEntity < Grape::Entity
expose :id, :name, :attempt_number, :pass_status, :exam_data, :completed, :cmi_entry
Expand Down
12 changes: 12 additions & 0 deletions app/api/task_definitions_api.rb
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ class TaskDefinitionsApi < Grape::API
optional :upload_requirements, type: String, desc: 'Task file upload requirements'
optional :plagiarism_checks, type: String, desc: 'The list of checks to perform'
requires :plagiarism_warn_pct, type: Integer, desc: 'The percent at which to record and warn about plagiarism'
requires :has_enabled_numbas_test, type: Boolean, desc: 'Whether or not Numbas test assessment is enabled for this task'
requires :numbas_time_delay, type: String, desc: 'The time delay between Numbas test attempts'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we change this time delay to an integer (in minutes) as well?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Daniel plans to have an incremental delay feature which the tutor can enable. So first 2 attempts would have no delay, the next would have a 30 min delay, then 2 hours, and so on until the student has to see the tutor. I've modified the code to accomodate this already.

requires :numbas_attempt_limit, type: Integer, desc: 'The number of times a Numbas test can be attempted'
requires :is_graded, type: Boolean, desc: 'Whether or not this task definition is a graded task'
requires :max_quality_pts, type: Integer, desc: 'A range for quality points when quality is assessed'
optional :assessment_enabled, type: Boolean, desc: 'Enable or disable assessment'
Expand Down Expand Up @@ -56,6 +59,9 @@ class TaskDefinitionsApi < Grape::API
:abbreviation,
:restrict_status_updates,
:plagiarism_warn_pct,
:has_enabled_numbas_test,
:numbas_time_delay,
:numbas_attempt_limit,
:is_graded,
:max_quality_pts,
:assessment_enabled,
Expand Down Expand Up @@ -106,6 +112,9 @@ class TaskDefinitionsApi < Grape::API
optional :upload_requirements, type: String, desc: 'Task file upload requirements'
optional :plagiarism_checks, type: String, desc: 'The list of checks to perform'
optional :plagiarism_warn_pct, type: Integer, desc: 'The percent at which to record and warn about plagiarism'
optional :has_enabled_numbas_test, type: Boolean, desc: 'Whether or not Numbas test assessment is enabled for this task'
optional :numbas_time_delay, type: String, desc: 'The time delay between Numbas test attempts'
optional :numbas_attempt_limit, type: Integer, desc: 'The number of times a Numbas test can be attempted'
optional :is_graded, type: Boolean, desc: 'Whether or not this task definition is a graded task'
optional :max_quality_pts, type: Integer, desc: 'A range for quality points when quality is assessed'
optional :assessment_enabled, type: Boolean, desc: 'Enable or disable assessment'
Expand Down Expand Up @@ -133,6 +142,9 @@ class TaskDefinitionsApi < Grape::API
:abbreviation,
:restrict_status_updates,
:plagiarism_warn_pct,
:has_enabled_numbas_test,
:numbas_time_delay,
:numbas_attempt_limit,
:is_graded,
:max_quality_pts,
:assessment_enabled,
Expand Down
50 changes: 47 additions & 3 deletions app/models/task_definition.rb
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ class TaskDefinition < ApplicationRecord

validates :weighting, presence: true

after_initialize :reset_numbas_configs_if_no_zip

include TaskDefinitionTiiModule
include TaskDefinitionSimilarityModule

Expand Down Expand Up @@ -102,6 +104,10 @@ def copy_to(other_unit)
new_td.add_task_resources(task_resources, copy: true)
end

if has_numbas_data?
new_td.add_numbas_data(task_numbas_data, copy: true)
end

new_td.save!

new_td
Expand Down Expand Up @@ -140,6 +146,10 @@ def move_files_on_abbreviation_change
if File.exist? task_assessment_resources_with_abbreviation(old_abbr)
FileUtils.mv(task_assessment_resources_with_abbreviation(old_abbr), task_assessment_resources())
end

if File.exist? task_numbas_data_with_abbreviation(old_abbr)
FileUtils.mv(task_numbas_data_with_abbreviation(old_abbr), task_numbas_data())
end
end

def docker_image_name_tag
Expand Down Expand Up @@ -348,7 +358,10 @@ def to_csv_row
end

def self.csv_columns
[:name, :abbreviation, :description, :weighting, :target_grade, :restrict_status_updates, :max_quality_pts, :is_graded, :plagiarism_warn_pct, :plagiarism_checks, :group_set, :upload_requirements, :start_week, :start_day, :target_week, :target_day, :due_week, :due_day, :tutorial_stream]
[:name, :abbreviation, :description, :weighting, :target_grade, :restrict_status_updates, :max_quality_pts,
:is_graded, :plagiarism_warn_pct, :plagiarism_checks, :group_set, :upload_requirements, :has_enabled_numbas_test,
:numbas_time_delay, :numbas_attempt_limit, :start_week, :start_day, :target_week, :target_day, :due_week, :due_day,
:tutorial_stream]
end

def self.task_def_for_csv_row(unit, row)
Expand Down Expand Up @@ -394,6 +407,10 @@ def self.task_def_for_csv_row(unit, row)
result.upload_requirements = JSON.parse(row[:upload_requirements]) unless row[:upload_requirements].nil?
result.due_date = due_date

result.has_enabled_numbas_test = %w(Yes y Y yes true TRUE 1).include? "#{row[:has_enabled_numbas_test]}".strip
result.numbas_time_delay = "#{row[:numbas_time_delay]}".strip
result.numbas_attempt_limit = row[:numbas_attempt_limit].to_i

result.plagiarism_warn_pct = row[:plagiarism_warn_pct].to_i
result.plagiarism_checks = JSON.parse(row[:plagiarism_checks]) unless row[:plagiarism_checks].nil?

Expand Down Expand Up @@ -445,6 +462,18 @@ def has_numbas_data?
File.exist? task_numbas_data
end

def has_enabled_numbas_test?
has_enabled_numbas_test
end

def numbas_time_delay?
numbas_time_delay
end

def numbas_attempt_limit?
numbas_attempt_limit
end

def is_graded?
is_graded
end
Expand Down Expand Up @@ -497,14 +526,20 @@ def remove_task_assessment_resources()
end
end

def add_numbas_data(file)
FileUtils.mv file, task_numbas_data
def add_numbas_data(file, copy: false)
if copy
FileUtils.cp file, task_numbas_data
else
FileUtils.mv file, task_numbas_data
end
end

def remove_numbas_data()
if has_numbas_data?
FileUtils.rm task_numbas_data
end

reset_numbas_configs_if_no_zip()
end

# Get the path to the task sheet - using the current abbreviation
Expand Down Expand Up @@ -566,6 +601,7 @@ def delete_associated_files()
remove_task_sheet()
remove_task_resources()
remove_task_assessment_resources()
remove_numbas_data()
end

# Calculate the path to the task sheet using the provided abbreviation
Expand Down Expand Up @@ -628,4 +664,12 @@ def task_numbas_data_with_abbreviation(abbr)
result_with_sanitised_file
end
end

def reset_numbas_configs_if_no_zip()
if !has_numbas_data?
self.has_enabled_numbas_test = false
self.numbas_time_delay = 'no delay'
self.numbas_attempt_limit = 0
end
end
end
2 changes: 1 addition & 1 deletion db/migrate/20231205011842_create_test_attempts.rb
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
class CreateTestAttempts < ActiveRecord::Migration[7.0]
def change
create_table :test_attempts do |t|
t.references :task, foreign_key: true
t.references :task
t.string :name
t.integer :attempt_number, default: 1, null: false
t.boolean :pass_status
Expand Down
19 changes: 0 additions & 19 deletions db/migrate/20231205011958_add_fields_to_task_def.rb

This file was deleted.

17 changes: 17 additions & 0 deletions db/migrate/20240322021829_modify_numbas_fields_in_task_def.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
class ModifyNumbasFieldsInTaskDef < ActiveRecord::Migration[7.0]
def change
change_table :task_definitions do |t|
t.boolean :has_enabled_numbas_test, default: false
t.string :numbas_time_delay
t.integer :numbas_attempt_limit
end
end

def down
change_table :task_definitions do |t|
t.remove :has_enabled_numbas_test
t.remove :numbas_time_delay
t.remove :numbas_attempt_limit
end
end
end
10 changes: 4 additions & 6 deletions db/schema.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
#
# It's strongly recommended that you check this file into your version control system.

ActiveRecord::Schema[7.0].define(version: 2023_12_05_011958) do
ActiveRecord::Schema[7.0].define(version: 2024_03_22_021829) 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
Expand Down Expand Up @@ -251,10 +251,9 @@
t.bigint "overseer_image_id"
t.string "tii_group_id"
t.string "moss_language"
t.boolean "has_test", default: false
t.boolean "restrict_attempts", default: false
t.integer "delay_restart_minutes"
t.boolean "retake_on_resubmit", default: false
t.boolean "has_enabled_numbas_test", default: false
t.string "numbas_time_delay"
t.integer "numbas_attempt_limit"
t.index ["group_set_id"], name: "index_task_definitions_on_group_set_id"
t.index ["overseer_image_id"], name: "index_task_definitions_on_overseer_image_id"
t.index ["tutorial_stream_id"], name: "index_task_definitions_on_tutorial_stream_id"
Expand Down Expand Up @@ -554,5 +553,4 @@
t.index ["user_id"], name: "index_webcals_on_user_id", unique: true
end

add_foreign_key "test_attempts", "tasks"
end
Loading