diff --git a/app/api/api_root.rb b/app/api/api_root.rb index 2817baa83..e36e21226 100644 --- a/app/api/api_root.rb +++ b/app/api/api_root.rb @@ -94,6 +94,7 @@ class ApiRoot < Grape::API mount TutorialEnrolmentsApi mount UnitRolesApi mount UnitsApi + mount TutorNotesApi mount D2lIntegrationApi::D2lApi mount D2lIntegrationApi::OauthPublicApi @@ -154,6 +155,7 @@ class ApiRoot < Grape::API AuthenticationHelpers.add_auth_to MarkingSessionsApi AuthenticationHelpers.add_auth_to DiscussionPromptsApi AuthenticationHelpers.add_auth_to OverseerStepsApi + AuthenticationHelpers.add_auth_to TutorNotesApi add_swagger_documentation \ base_path: nil, diff --git a/app/api/entities/project_entity.rb b/app/api/entities/project_entity.rb index f390efd3c..054e5bd1c 100644 --- a/app/api/entities/project_entity.rb +++ b/app/api/entities/project_entity.rb @@ -29,5 +29,7 @@ class ProjectEntity < Grape::Entity expose :grade, if: :for_staff expose :grade_rationale, if: :for_staff + + expose :escalation_attempts_remaining end end diff --git a/app/api/entities/tutor_note_entity.rb b/app/api/entities/tutor_note_entity.rb new file mode 100644 index 000000000..1184b01d2 --- /dev/null +++ b/app/api/entities/tutor_note_entity.rb @@ -0,0 +1,21 @@ +module Entities + class TutorNoteEntity < Grape::Entity + expose :id + + expose :note + expose :unit_role_id + expose :user_id + + expose :created_at + expose :updated_at + + expose :reply_to_id + + expose :task_id + expose :task_definition_id + expose :project_id + + expose :read_by_unit_role + + end +end diff --git a/app/api/entities/unit_role_entity.rb b/app/api/entities/unit_role_entity.rb index ae257f218..c989d99b5 100644 --- a/app/api/entities/unit_role_entity.rb +++ b/app/api/entities/unit_role_entity.rb @@ -1,9 +1,19 @@ module Entities class UnitRoleEntity < Grape::Entity + + def staff?(my_role) + [Role.tutor_id, Role.convenor_id, Role.admin_id, Role.auditor_id].include?(my_role.id) unless my_role.nil? + end + expose :id expose :role do |unit_role, options| unit_role.role.name end expose :user, using: Entities::Minimal::MinimalUserEntity expose :unit, using: Entities::Minimal::MinimalUnitEntity, unless: :in_unit expose :observer_only + + expose :mentor_id, if: ->(unit_role, options) { staff?(options[:my_role]) } + expose :tutor_note_count, if: ->(unit_role, options) { unit_role.unit.unit_role_for(options[:user])&.id == unit_role.id } do |unit_role, options| + unit_role.tutor_notes.where(read_by_unit_role: false).count + end end end diff --git a/app/api/tasks_api.rb b/app/api/tasks_api.rb index 47a41e64b..6153c13a0 100644 --- a/app/api/tasks_api.rb +++ b/app/api/tasks_api.rb @@ -424,4 +424,42 @@ class TasksApi < Grape::API end end + desc 'Request a feedback review (creates an escalation ModeratedTask)' + params do + requires :id, type: Integer, desc: 'The project id' + requires :task_definition_id, type: Integer, desc: 'The id of the task definition for the task to review' + end + post '/projects/:id/task_def_id/:task_definition_id/feedback_review' do + project = Project.find(params[:id]) + + unless authorise?(current_user, project, :make_submission) + error!({ error: 'You do not have permission to request a feedback review for this project.' }, 403) + end + + if project.escalation_attempts_remaining <= 0 + error!({ error: 'You can not escalate any more tasks.' }, 403) + end + # TODO: ensure that feedback has actually been left by a tutor (task comments) + + task_definition = project.unit.task_definitions.find(params[:task_definition_id]) + task = project.task_for_task_definition(task_definition) + + existing = ModeratedTask.find_by(task: task) + + if existing&.moderation_type == "escalation" + error!({ error: "A feedback review has already been requested for this task." }, 409) + end + + existing&.destroy! + + moderated_task = ModeratedTask.create!( + task: task, + task_definition: task_definition, + moderation_type: :escalation, + state: "open" + ) + + moderated_task.valid? + end + end diff --git a/app/api/tutor_notes_api.rb b/app/api/tutor_notes_api.rb new file mode 100644 index 000000000..ddb4cbc4a --- /dev/null +++ b/app/api/tutor_notes_api.rb @@ -0,0 +1,178 @@ +require 'grape' + +class TutorNotesApi < Grape::API + helpers AuthenticationHelpers + helpers AuthorisationHelpers + + before do + authenticated? + end + + helpers do + def can_access_tutor_notes?(unit, current_user, unit_role) + current_user_role = unit.unit_role_for(current_user) + + current_user_role.role == Role.convenor || + unit_role.mentor_id == current_user_role.id || + unit_role == current_user_role + end + end + + desc "Get all the tutor notes for a unit role" + params do + requires :unit_role_id, type: Integer, desc: 'Unit role to fetch the notes for' + end + get '/unit_roles/:unit_role_id/tutor_notes' do + unit_role = UnitRole.find(params[:unit_role_id]) + unit = unit_role.unit + unless authorise? current_user, unit, :get_unit + error!({ error: 'You do not have permission to access this unit' }, 403) + end + + unless authorise? current_user, unit_role, :create_tutor_note + error!({ error: 'You do not have permission to access this.' }, 403) + end + + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to access this.' }, 403) + end + + result = unit_role.tutor_notes + + present result, with: Entities::TutorNoteEntity, user: current_user + end + + desc "Mark a tutor note as read" + params do + requires :unit_role_id, type: Integer, desc: 'Unit role to fetch the notes for' + end + put '/unit_roles/:unit_role_id/tutor_notes/:id/mark_as_read' do + unit_role = UnitRole.find(params[:unit_role_id]) + + unit = unit_role.unit + unless authorise? current_user, unit, :get_unit + error!({ error: 'You do not have permission to access this unit' }, 403) + end + + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to access this.' }, 403) + end + + tutor_note = unit_role.tutor_notes.find(params[:id]) + + current_unit_role = unit.unit_role_for(current_user) + + unless current_unit_role == unit_role && unit_role == tutor_note.unit_role + error!({ error: 'You do not have permission to update this note.' }, 403) + end + + tutor_note.update!(read_by_unit_role: true) + + true + end + + desc "Create a new note for a tutor" + params do + requires :note, type: String, desc: 'The text to add to the tutor note' + optional :reply_to_id, type: Integer, desc: 'ID of the tutor note this is being replied to' + optional :task_id, type: Integer, desc: 'ID of the task this note is related to' + end + post '/unit_roles/:unit_role_id/tutor_notes' do + unit_role = UnitRole.find(params[:unit_role_id]) + unit = unit_role.unit + unless authorise? current_user, unit, :get_unit + error!({ error: 'You do not have permission to access this unit.' }, 403) + end + + unless authorise? current_user, unit_role, :create_tutor_note + error!({ error: 'You do not have permission to create note.' }, 403) + end + + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to create note.' }, 403) + end + + text_note = params[:note] + + reply_to_id = params[:reply_to_id] + if reply_to_id.present? + original_staff_note = TutorNote.find(reply_to_id) + error!(error: 'You do not have permission to read the replied tutor note') unless authorise?(current_user, original_staff_note.unit_role, :get) + error!(error: 'Original tutor note is not in this project.') if unit_role.tutor_notes.find(reply_to_id).blank? + end + + task_id = params[:task_id] + if task_id.present? + task = Task.find(task_id) + error!(error: 'You do not have permission to add a note related to this task') unless authorise?(unit_role.user, task.project, :assess) + end + + result = unit_role.add_tutor_note(current_user, text_note, task_id, reply_to_id) + + if result.nil? + error!({ error: 'Duplicate note.' }, 403) + else + present result, with: Entities::TutorNoteEntity, user: current_user + end + end + + desc "Delete a tutor note for a unit role" + delete '/unit_roles/:unit_role_id/tutor_notes/:id' do + unit_role = UnitRole.find(params[:unit_role_id]) + unit = unit_role.unit + unless authorise? current_user, unit, :get_unit + error!({ error: 'You do not have permission to access this unit' }, 403) + end + + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to access create note.' }, 403) + end + + tutor_note = unit_role.tutor_notes.find(params[:id]) + + error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.unit_role != unit_role + + unless authorise?(current_user, unit_role, :delete_tutor_note) || tutor_note.user.id == current_user.id + error!({ error: 'You do not have permission to delete this note.' }, 403) + end + + tutor_note.destroy + error!({ error: tutor_note.errors.full_messages.last }, 403) unless tutor_note.destroyed? + + present tutor_note.destroyed?, with: Grape::Presenters::Presenter + end + + desc "Update a tutor note for a project" + params do + requires :unit_role_id, type: Integer, desc: 'The ID of the unit role' + requires :id, type: Integer, desc: 'The tutor note id to update' + requires :note, type: String, desc: 'The text to update the tutor note with' + end + put '/unit_roles/:unit_role_id/tutor_notes/:id' do + unit_role = UnitRole.find(params[:unit_role_id]) + unit = unit_role.unit + unless authorise? current_user, unit, :get_unit + error!({ error: 'You do not have permission to access this unit' }, 403) + end + + unless authorise? current_user, unit_role, :create_tutor_note + error!({ error: 'You do not have permission to access this.' }, 403) + end + + tutor_note = unit_role.tutor_notes.find(params[:id]) + + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to access create note.' }, 403) + end + + error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.unit_role != unit_role + + unless tutor_note.user.id == current_user.id + error!({ error: 'You do not have permission to delete this note.' }, 403) + end + + tutor_note.update!(note: params[:note]) + present tutor_note, with: Entities::TutorNoteEntity, user: current_user + end + +end diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index 57f8ba3f2..8e397359d 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -71,6 +71,7 @@ class UnitRolesApi < Grape::API requires :unit_role, type: Hash do requires :role_id, type: Integer, desc: 'The role to create with' optional :observer_only, type: Boolean, desc: 'If the staff has read-only permissions' + optional :mentor_id, type: Integer, desc: 'Assign a mentor to this unit role' end end put '/unit_roles/:id' do @@ -99,7 +100,8 @@ class UnitRolesApi < Grape::API .require(:unit_role) .permit( :role_id, - :observer_only + :observer_only, + :mentor_id ) if unit_role_parameters[:role_id] == Role.tutor.id && unit_role.role == Role.convenor && unit_role.unit.convenors.count == 1 @@ -109,4 +111,73 @@ class UnitRolesApi < Grape::API unit_role.update!(unit_role_parameters) present unit_role, with: Entities::UnitRoleEntity, in_unit: true end + + desc 'Moderate tutor feedback' + params do + requires :id, type: Integer, desc: 'The id of the unit role to moderate' + requires :task_id, type: Integer, desc: 'The id of the task' + requires :action, type: String, desc: 'Action to apply to this moderated task' + # requires :score, type: Integer, desc: 'Moderation for the task' + # TODO: accept an "outcome" enum/string? + end + post '/unit_roles/:id/moderation/:task_id' do + unit_role = UnitRole.find(params[:id]) + unit = unit_role.unit + + task = Task.find(params[:task_id]) + tutor_user = task.project.tutor_for(task.task_definition) + tutor = unit.unit_role_for(tutor_user) + unless tutor.id == unit_role.id + error!("Invalid unit role", 400) + end + + current_unit_role = unit.unit_role_for(current_user) + unless tutor.mentor == current_unit_role + error!({ error: 'You do not have permission to moderate this feedback' }, 400) + end + + action = params[:action].lower + # unless [-1, 0, 1].include?(score) + unless %w[show_more show_less dismiss_ok upheld overturn].include?(action) + error!({ error: 'Invalid moderation action' }, 400) + end + + moderated_task = ModeratedTask.find_by(task: task) + + recent_threshold = 0.minutes.ago + if moderated_task.last_moderated_date && moderated_task.last_moderated_date > recent_threshold + error!({ error: 'Feedback is too new to moderate' }, 400) + end + + factor = Doubtfire::Application.config.moderation_score_factor + + delta = + case action + when 'show_more', 'overturn' + -1 + when 'show_less' + 1 + when 'dismiss_ok', 'upheld' + 0 + end + + score = score.to_i + (delta * factor) + + td_score = TutorFeedbackScore.find_by(unit_role: unit_role, task_definition: task.task_definition) + + td_score.update!( + score: (td_score.score + delta).clamp(0, 99) + ) + + moderated_task.update!(last_moderated_date: Time.zone.now) + + unless score == -1 + moderated_task.update!({ + state: :resolved, + user: current_user + }) + end + + true + end end diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 0198568e8..0ee14b8df 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -62,7 +62,7 @@ class UnitsApi < Grape::API # Unit uses user from thread to limit exposure # my_role = unit.role_for(current_user) - present unit, with: Entities::UnitEntity, my_role: my_role, in_unit: true + present unit, with: Entities::UnitEntity, user: current_user, my_role: my_role, in_unit: true end desc 'Update unit' @@ -303,6 +303,42 @@ class UnitsApi < Grape::API present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter end + desc 'Get tasks ready for moderation' + get '/units/:id/tasks/moderation' do + unit = Unit.find(params[:id]) + + unless authorise? current_user, unit, :get_students + error!({ error: 'Not authorised to provide feedback for this unit' }, 403) + end + + unless authorise? current_user, unit, :provide_feedback + error!({ error: 'Not authorised to provide feedback for this unit' }, 403) + end + + tasks = unit.tasks_for_moderation(current_user) + data = tasks.map do |t| + { + id: t.task_id, + project_id: t.project_id, + task_definition_id: t.task_definition_id, + tutorial_id: t.tutorial_id, + status: TaskStatus.id_to_key(t.status_id), + completion_date: t.completion_date, + submission_date: t.submission_date, + times_assessed: t.times_assessed, + grade: t.grade, + quality_pts: t.quality_pts, + num_new_comments: t.number_unread, + similarity_flag: t.similar_to_count > 0, + pinned: t.pinned, + has_extensions: t.has_extensions, + moderation_type: t.moderated_task&.moderation_type + } + end + # present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter + present data, with: Grape::Presenters::Presenter + end + desc 'Download the grades for a unit' get '/units/:id/grades' do unit = Unit.find(params[:id]) diff --git a/app/models/moderated_task.rb b/app/models/moderated_task.rb new file mode 100644 index 000000000..adcc4be8b --- /dev/null +++ b/app/models/moderated_task.rb @@ -0,0 +1,17 @@ +class ModeratedTask < ApplicationRecord + belongs_to :task + belongs_to :task_definition + belongs_to :user, optional: true + + enum :state, { + open: 'open', + waiting_for_new_feedback: 'waiting_for_new_feedback', + resolved: 'resolved' + } + + enum :moderation_type, { + first_feedback: 'first_feedback', + random_sample: 'random_sample', + escalation: 'escalation' + } +end diff --git a/app/models/project.rb b/app/models/project.rb index 1e5782484..1444730f9 100644 --- a/app/models/project.rb +++ b/app/models/project.rb @@ -715,6 +715,12 @@ def add_staff_note(user, text, reply_to_id = nil) note end + # TODO: env var for escalation attempts -- per unit setting? max_feedback_escalation_attempts + + def escalation_attempts_remaining + 3 - ModeratedTask.where(task: tasks, moderation_type: :escalation, outcome: [nil, 'upheld']).count + end + private def can_destroy? diff --git a/app/models/task.rb b/app/models/task.rb index d89792002..bda82ea56 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -125,6 +125,7 @@ def specific_permission_hash(role, perm_hash, _other) belongs_to :group_submission, optional: true has_one :unit, through: :project + has_one :moderated_task, dependent: :destroy has_many :comments, class_name: 'TaskComment', dependent: :destroy, inverse_of: :task has_many :task_similarities, class_name: 'TaskSimilarity', dependent: :destroy, inverse_of: :task @@ -659,6 +660,25 @@ def assess(task_status, assessor, assess_date = Time.zone.now) # Save the task if save! + if assessor == tutor + moderated_task = ModeratedTask.find_by(task: self) + if moderated_task + if moderated_task.assessor_id != tutor.id + moderated_task.update!(assessor_id: tutor.id) + end + else + sample_count = ModeratedTask.where( + moderation_type: :first_feedback, + assessor_id: tutor.id, + task_definition: task_definition + ).count + + if sample_count < 3 + mark_as_moderated(moderation_type: :first_feedback) + end + end + end + if task_status == TaskStatus.fix_and_resubmit # Look for other submitted tasks from this student that has this task as a prerequisite # If they are ready for feedback, automatically assess them to fix and resubmit @@ -1595,6 +1615,20 @@ def overseer_enabled? (has_new_files? || has_done_file?) end + def mark_as_moderated(moderation_type: :random_sample) + moderated_task = ModeratedTask.find_by(task_id: id) + if moderated_task.nil? + ModeratedTask.create!({ + task: self, + task_definition: task_definition, + assessor_id: tutor.id, + state: :open, + moderation_type: moderation_type, + last_moderated_date: Time.zone.now + }) + end + end + private def delete_associated_files diff --git a/app/models/tutor_feedback_score.rb b/app/models/tutor_feedback_score.rb new file mode 100644 index 000000000..e18ed5674 --- /dev/null +++ b/app/models/tutor_feedback_score.rb @@ -0,0 +1,4 @@ +class TutorFeedbackScore < ApplicationRecord + belongs_to :unit_role + belongs_to :task_definition +end diff --git a/app/models/tutor_note.rb b/app/models/tutor_note.rb new file mode 100644 index 000000000..c97461409 --- /dev/null +++ b/app/models/tutor_note.rb @@ -0,0 +1,14 @@ +class TutorNote < ApplicationRecord + belongs_to :unit_role + belongs_to :user + belongs_to :task + belongs_to :reply_to, class_name: "TutorNote", optional: true + + def task_definition_id + task&.task_definition&.id + end + + def project_id + task&.project&.id + end +end diff --git a/app/models/unit.rb b/app/models/unit.rb index 1dba4ed8e..457a66f4c 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -2171,6 +2171,77 @@ def tasks_for_task_inbox(user, my_students_only = false) .order('pinned DESC, submission_date ASC, MAX(task_comments.created_at) ASC, task_definition_id ASC') end + # + # Return the tasks that have been selected to be moderated by the tutor's mentor + # + # Tasks that are included are: + # - If a ModeratedTask exists for a task, and has not been dismissed + # - A TaskComment exists from the task's tutor, that is atleast 15 minutes old + # + # Tasks with the oldest feedback are shown first + # + def tasks_for_moderation(user) + my_unit_role = unit_role_for(user) + mentees = staff.where(mentor_id: my_unit_role.id) + + tasks = student_tasks + .includes(:comments) + .left_joins(:moderated_task) + .where(moderated_tasks: { state: %w[open waiting_for_new_feedback] }) + .where(projects: { unit_id: id }) + .joins(:task_definition) + .joins(project: { tutorial_enrolments: :tutorial }) + .where(tutorials: { unit_role_id: mentees.select(:id) }) + .where('tutorials.tutorial_stream_id = task_definitions.tutorial_stream_id') + .select( + 'tasks.id AS task_id', + 'tasks.project_id', + 'tasks.task_definition_id', + 'tutorials.id AS tutorial_id', + 'tasks.task_status_id AS status_id', + 'tasks.completion_date', + 'tasks.submission_date', + 'tasks.times_assessed', + 'tasks.grade', + 'tasks.quality_pts', + '0 AS number_unread', + '0 AS similar_to_count', + 'false AS pinned', + 'false AS has_extensions', + 'tasks.*', + ) + .distinct + + + # Only include tasks where the tutor's latest comment is at least 15 minutes old + # to ensure that the feedback is likely complete before adding it for moderation + comment_threshold = 15.minutes.ago + + tasks = tasks.map do |task| + mt = task.moderated_task + next if mt.nil? + + if mt.escalation? + [task, Time.zone.at(0)] + else + tutor_comments = task.comments.select { |c| c.user == task.tutor } + next nil if tutor_comments.empty? + + most_recent = tutor_comments.max_by(&:created_at) + next nil if most_recent.created_at > comment_threshold || + most_recent.created_at <= (task.moderated_task&.last_moderated_date || Time.zone.at(0)) + + [task, most_recent.created_at] + end + end.compact + + # Sort tasks: show tasks with the oldest feedback first + # New feedback will send it to the bottom of the moderation queue + tasks.sort_by! { |_task, last_comment_time| last_comment_time } + tasks.map!(&:first) + end + + # # Return stats on the number of students in each status for each task / tutorial # diff --git a/app/models/unit_role.rb b/app/models/unit_role.rb index 7a0050786..d63271c92 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -5,12 +5,16 @@ class UnitRole < ApplicationRecord belongs_to :role, optional: false # Foreign key + belongs_to :mentor, class_name: 'UnitRole', optional: true + has_many :tutorials, class_name: 'Tutorial', dependent: :nullify has_many :projects, through: :tutorials has_many :tasks, through: :projects has_many :task_engagements, through: :tasks has_many :comments, through: :tasks + has_many :tutor_notes, dependent: :destroy + validates :unit_id, presence: true validates :user_id, presence: true validates :role_id, presence: true @@ -47,12 +51,15 @@ def self.permissions ] # What can tutors do with unit roles? tutor_role_permissions = [ - :get + :get, + :create_tutor_note ] # What can convenors do with unit roles? convenor_role_permissions = [ :get, - :delete + :delete, + :delete_tutor_note, + :create_tutor_note ] # What can nil users do with unit roles? nil_role_permissions = [] @@ -286,4 +293,43 @@ def get_marking_sessions_csv(start_date: nil, end_date: nil, timezone: nil) end end + + def add_tutor_note(user, text, task_id = nil, reply_to_id = nil) + text = text.strip + return nil if user.nil? || text.nil? || text.empty? + + ln = tutor_notes.last + + # don't add if duplicate note + return if ln && ln.user == user && ln.note == text + + note = TutorNote.create + note.note = text + note.user = user + note.unit_role = self + note.reply_to_id = reply_to_id + note.task_id = task_id + note.read_by_unit_role = false + note.save! + note + end + + def should_moderate_task?(task) + td = task.task_definition + td_rep = TutorFeedbackScore.find_by(unit_role: self, task_definition: td) + if td_rep.nil? + td_rep = TutorFeedbackScore.create!({ + unit_role: self, + task_definition: td, + score: 50 + }) + end + + # We sample randomly during a task submission + if rand(0..100) > td_rep.score + return true + end + # TODO: integrate escalation + false + end end diff --git a/app/sidekiq/accept_submission_job.rb b/app/sidekiq/accept_submission_job.rb index a8a44bf7f..00c590f61 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -45,6 +45,16 @@ def perform(task_id, user_id, accepted_tii_eula, test_submission) return end + # Mark this task for moderation + tutor_user = task.project.tutor_for(task.task_definition) + if tutor_user + tutor = task.unit.unit_role_for(tutor_user) + if tutor&.should_moderate_task?(task) + logger.info "Marking task #{task.id} for moderation (project #{task.project.id})" + task.mark_as_moderated + end + end + # When converted, we can now send documents to turn it in for checking if TurnItIn.enabled? task.send_documents_to_tii(user, accepted_tii_eula: accepted_tii_eula) diff --git a/config/application.rb b/config/application.rb index 19ea42155..770faf7fc 100644 --- a/config/application.rb +++ b/config/application.rb @@ -97,6 +97,9 @@ def self.fetch_boolean_env(name) # LTI.js will send signed JWT tokens using this secret config.lti_api_secret = ENV.fetch('LTI_SHARED_API_SECRET', nil) + # ==> Moderation settings + config.moderation_score_factor = Float(ENV.fetch('MODERATION_SCORE_FACTOR', 1.0)) + # ==> Institution settings # Institution YAML and ENV (override) config load config.institution = YAML.load_file(Rails.root.join('config/institution.yml').to_s).with_indifferent_access diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb new file mode 100644 index 000000000..e110e3eb4 --- /dev/null +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -0,0 +1,62 @@ +class AddModerationFeat < ActiveRecord::Migration[8.0] + def change + create_table :tutor_notes do |t| + t.text :note + t.references :task, index: true, null: true + t.references :unit_role, index: true, null: false + t.references :user, index: true, null: false + t.references :reply_to, index: true, null: true + + t.boolean :read_by_unit_role, null: false + + t.timestamps + end + + create_table :tutor_feedback_scores do |t| + t.references :unit_role, null: false + t.references :task_definition, null: false + t.integer :score, default: 50, null: false + + t.timestamps + end + + create_table :moderated_tasks do |t| + t.references :task, null: false + t.references :task_definition, null: false + + t.datetime :last_moderated_date, null: true + + # open | waiting_for_new_feedback | resolved + t.string :state, null: false + + # random_sample | initial_feedback | escalation + t.string :moderation_type, null: false + + t.bigint :assessor_id, null: true + + # User that dismissed the moderated task + t.bigint :resolved_by_user_id, null: true + t.datetime :resolved_at + + # => Moderation + # - Show me more: state=waiting_for_new_feedback, -score for TaskDefinition + # - Show me less: state=resolved, outcome=dismissed_good, +score for TaskDefinition + # - Dismiss: state=resolved, outcome=dismissed_ok, no score change + # => Escalation + # Upheld: state=resolved, outcome=upheld, -score for TaskDefinition + # Overturned state=resolved, outcome=overturned, +score for TaskDefinition + t.string :outcome, null: true + + t.timestamps + end + + add_index :moderated_tasks, [:assessor_id, :task_definition_id, :moderation_type], + name: "idx_mod_tasks_assessor_td_type" + + add_index :moderated_tasks, [:task_id, :moderation_type], + unique: true, + name: "uniq_mod_tasks_task_type" + + add_reference :unit_roles, :mentor, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 3b78acfba..fead37d30 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[8.0].define(version: 2025_12_18_031455) do +ActiveRecord::Schema[8.0].define(version: 2026_01_13_033152) do create_table "activity_types", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| t.string "name", null: false t.string "abbreviation", null: false @@ -208,6 +208,24 @@ t.index ["user_id"], name: "index_marking_sessions_on_user_id" end + create_table "moderated_tasks", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| + t.bigint "task_id", null: false + t.bigint "task_definition_id", null: false + t.datetime "last_moderated_date" + t.string "state", null: false + t.string "moderation_type", null: false + t.bigint "assessor_id" + t.bigint "resolved_by_user_id" + t.datetime "resolved_at" + t.string "outcome" + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["assessor_id", "task_definition_id", "moderation_type"], name: "idx_mod_tasks_assessor_td_type" + t.index ["task_definition_id"], name: "index_moderated_tasks_on_task_definition_id" + t.index ["task_id", "moderation_type"], name: "uniq_mod_tasks_task_type", unique: true + t.index ["task_id"], name: "index_moderated_tasks_on_task_id" + end + create_table "overseer_assessments", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| t.bigint "task_id", null: false t.string "submission_timestamp", null: false @@ -587,6 +605,31 @@ t.index ["tii_task_similarity_id"], name: "index_tii_submissions_on_tii_task_similarity_id" end + create_table "tutor_feedback_scores", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| + t.bigint "unit_role_id", null: false + t.bigint "task_definition_id", null: false + t.integer "score", default: 50, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["task_definition_id"], name: "index_tutor_feedback_scores_on_task_definition_id" + t.index ["unit_role_id"], name: "index_tutor_feedback_scores_on_unit_role_id" + end + + create_table "tutor_notes", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| + t.text "note" + t.bigint "task_id" + t.bigint "unit_role_id", null: false + t.bigint "user_id", null: false + t.bigint "reply_to_id" + t.boolean "read_by_unit_role", null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["reply_to_id"], name: "index_tutor_notes_on_reply_to_id" + t.index ["task_id"], name: "index_tutor_notes_on_task_id" + t.index ["unit_role_id"], name: "index_tutor_notes_on_unit_role_id" + t.index ["user_id"], name: "index_tutor_notes_on_user_id" + end + create_table "tutorial_enrolments", charset: "utf8mb4", collation: "utf8mb4_general_ci", force: :cascade do |t| t.datetime "created_at", null: false t.datetime "updated_at", null: false @@ -639,6 +682,8 @@ t.bigint "role_id" t.bigint "unit_id" t.boolean "observer_only", default: false + t.bigint "mentor_id" + t.index ["mentor_id"], name: "index_unit_roles_on_mentor_id" t.index ["role_id"], name: "index_unit_roles_on_role_id" t.index ["tutorial_id"], name: "index_unit_roles_on_tutorial_id" t.index ["unit_id"], name: "index_unit_roles_on_unit_id" diff --git a/test/api/projects_api_test.rb b/test/api/projects_api_test.rb index 4e49eced6..30407838e 100644 --- a/test/api/projects_api_test.rb +++ b/test/api/projects_api_test.rb @@ -51,8 +51,8 @@ def test_projects_returns_correct_data # Add username and auth_token to Header add_auth_header_for(user: user) - keys = %w(id unit campus_id user_id target_grade portfolio_available spec_con_days) - key_test = %w(campus_id target_grade spec_con_days) + keys = %w[id unit campus_id user_id target_grade portfolio_available spec_con_days escalation_attempts_remaining] + key_test = %w[campus_id target_grade spec_con_days] get '/api/projects' assert_equal 2, last_response_body.count, last_response_body @@ -76,7 +76,7 @@ def test_get_project_response_is_correct # Add username and auth_token to Header add_auth_header_for(user: user) - keys = %w[id unit unit_id user_id campus_id target_grade submitted_grade portfolio_files compile_portfolio portfolio_available uses_draft_learning_summary tasks tutorial_enrolments groups spec_con_days] + keys = %w[id unit unit_id user_id campus_id target_grade submitted_grade portfolio_files compile_portfolio portfolio_available uses_draft_learning_summary tasks tutorial_enrolments groups spec_con_days escalation_attempts_remaining] key_test = keys - %w[unit user_id portfolio_available tasks tutorial_enrolments groups] get "/api/projects/#{project.id}" diff --git a/test/api/units_api_test.rb b/test/api/units_api_test.rb index 8915b884e..d73cdaa00 100644 --- a/test/api/units_api_test.rb +++ b/test/api/units_api_test.rb @@ -345,9 +345,13 @@ def test_unit_output assert actual_unit.key?("staff"), actual_unit.inspect assert_equal expected_unit.staff.count, actual_unit["staff"].count, actual_unit["staff"].inspect actual_unit["staff"].each do |staff| - keys = %w[id role user observer_only] - assert_json_limit_keys_to_exactly keys, staff ur = UnitRole.find(staff['id']) + + keys = %w[id role user observer_only mentor_id] + # Check to ensure tutor_note_count is only exposed to the current user's unit role + keys << 'tutor_note_count' if ur.unit.unit_role_for(expected_unit.main_convenor_user).id == ur.id + + assert_json_limit_keys_to_exactly keys, staff assert_equal ur.id, staff['id'] assert_equal ur.role.name, staff['role'] assert_equal ur.user.id, staff['user']['id'] diff --git a/test/factories/moderated_tasks.rb b/test/factories/moderated_tasks.rb new file mode 100644 index 000000000..df1075db3 --- /dev/null +++ b/test/factories/moderated_tasks.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :moderated_task do + + end +end diff --git a/test/factories/tutor_feedback_scores.rb b/test/factories/tutor_feedback_scores.rb new file mode 100644 index 000000000..9ac2a7acd --- /dev/null +++ b/test/factories/tutor_feedback_scores.rb @@ -0,0 +1,5 @@ +FactoryBot.define do + factory :tutor_feedback_score do + + end +end diff --git a/test/factories/tutor_notes.rb b/test/factories/tutor_notes.rb new file mode 100644 index 000000000..de9b15338 --- /dev/null +++ b/test/factories/tutor_notes.rb @@ -0,0 +1,4 @@ +FactoryBot.define do + factory :tutor_note do + end +end diff --git a/test/models/moderated_task_test.rb b/test/models/moderated_task_test.rb new file mode 100644 index 000000000..0a7b20a10 --- /dev/null +++ b/test/models/moderated_task_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +describe ModeratedTask do + # it "does a thing" do + # value(1+1).must_equal 2 + # end +end diff --git a/test/models/tutor_feedback_score_test.rb b/test/models/tutor_feedback_score_test.rb new file mode 100644 index 000000000..d7ea5680e --- /dev/null +++ b/test/models/tutor_feedback_score_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +describe TutorFeedbackScore do + # it "does a thing" do + # value(1+1).must_equal 2 + # end +end diff --git a/test/models/tutor_note_test.rb b/test/models/tutor_note_test.rb new file mode 100644 index 000000000..9cfd9a593 --- /dev/null +++ b/test/models/tutor_note_test.rb @@ -0,0 +1,7 @@ +require "test_helper" + +describe TutorNote do + # it "does a thing" do + # value(1+1).must_equal 2 + # end +end