From b2ab7a48c2c8ada346ec2c3b5a50e641d3c35e5b Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 7 Jan 2026 13:42:49 +1100 Subject: [PATCH 01/26] feat: init tutor notes and mentorship --- app/api/api_root.rb | 2 + app/api/entities/tutor_note_entity.rb | 17 +++ app/api/entities/unit_role_entity.rb | 2 + app/api/tutor_notes_api.rb | 118 ++++++++++++++++++ app/api/unit_roles_api.rb | 4 +- app/models/tutor_note.rb | 5 + app/models/unit_role.rb | 21 ++++ .../20260105234326_create_tutor_notes.rb | 17 +++ .../20260106005505_add_mentor_to_unit_role.rb | 5 + db/schema.rb | 21 +++- test/factories/tutor_notes.rb | 4 + test/models/tutor_note_test.rb | 7 ++ 12 files changed, 221 insertions(+), 2 deletions(-) create mode 100644 app/api/entities/tutor_note_entity.rb create mode 100644 app/api/tutor_notes_api.rb create mode 100644 app/models/tutor_note.rb create mode 100644 db/migrate/20260105234326_create_tutor_notes.rb create mode 100644 db/migrate/20260106005505_add_mentor_to_unit_role.rb create mode 100644 test/factories/tutor_notes.rb create mode 100644 test/models/tutor_note_test.rb diff --git a/app/api/api_root.rb b/app/api/api_root.rb index 2817baa835..e36e21226e 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/tutor_note_entity.rb b/app/api/entities/tutor_note_entity.rb new file mode 100644 index 0000000000..5f4997eb80 --- /dev/null +++ b/app/api/entities/tutor_note_entity.rb @@ -0,0 +1,17 @@ +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 + + # TODO: what is tutor_notes_id? + + end +end diff --git a/app/api/entities/unit_role_entity.rb b/app/api/entities/unit_role_entity.rb index ae257f2189..98bf1f21d6 100644 --- a/app/api/entities/unit_role_entity.rb +++ b/app/api/entities/unit_role_entity.rb @@ -5,5 +5,7 @@ class UnitRoleEntity < Grape::Entity expose :user, using: Entities::Minimal::MinimalUserEntity expose :unit, using: Entities::Minimal::MinimalUnitEntity, unless: :in_unit expose :observer_only + # TODO: expose to staff only + expose :mentor_id end end diff --git a/app/api/tutor_notes_api.rb b/app/api/tutor_notes_api.rb new file mode 100644 index 0000000000..92304b467c --- /dev/null +++ b/app/api/tutor_notes_api.rb @@ -0,0 +1,118 @@ +require 'grape' + +class TutorNotesApi < Grape::API + helpers AuthenticationHelpers + helpers AuthorisationHelpers + + before do + authenticated? + 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 + + current_user_role = unit.unit_role_for(current_user) + + can_access = + current_user_role == Role.convenor || + unit_role.mentor_id == current_user_role.id || + unit_role == current_user_role + + unless can_access + 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 "Create a new tutor note for a project" + 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 staff note this is being replied 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 + + 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 + + result = unit_role.add_tutor_note(current_user, text_note, 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 + + tutor_note = unit_role.tutor_notes.find(params[:id]) + + # TODO: delete permissions + # unless authorise?(current_user, project, :delete_staff_note) || staff_note.user.id == current_user.id + # error!({ error: 'You do not have permission to delete this note.' }, 403) + # end + + # error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.user_role != user_role.id + + 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 + + tutor_note = unit_role.tutor_notes.find(params[:id]) + + # TODO: permissions + + # unless authorise?(current_user, project, :create_staff_note) && staff_note.user.id == current_user.id + # error!({ error: 'You do not have permission to edit this note.' }, 403) + # end + + # error!({ error: 'Note does not belong to this project' }, 404) if staff_note.project_id != project.id + + 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 85ead4295c..ec5319cd38 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 @@ -95,7 +96,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 diff --git a/app/models/tutor_note.rb b/app/models/tutor_note.rb new file mode 100644 index 0000000000..98d85f6f6f --- /dev/null +++ b/app/models/tutor_note.rb @@ -0,0 +1,5 @@ +class TutorNote < ApplicationRecord + belongs_to :unit_role + belongs_to :user + belongs_to :reply_to, class_name: "TutorNote", optional: true +end diff --git a/app/models/unit_role.rb b/app/models/unit_role.rb index 7a00507863..bd5dc5935a 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -11,6 +11,8 @@ class UnitRole < ApplicationRecord 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 @@ -286,4 +288,23 @@ def get_marking_sessions_csv(start_date: nil, end_date: nil, timezone: nil) end end + + def add_tutor_note(user, text, 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.convenor_only = false + note.save! + note + end end diff --git a/db/migrate/20260105234326_create_tutor_notes.rb b/db/migrate/20260105234326_create_tutor_notes.rb new file mode 100644 index 0000000000..d000d550dc --- /dev/null +++ b/db/migrate/20260105234326_create_tutor_notes.rb @@ -0,0 +1,17 @@ +class CreateTutorNotes < 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 :unit_roles, :mentor, index: true, null: false + t.references :user, index: true, null: false + t.references :tutor_notes, :reply_to, index: true, null: true + + # TODO: Only accessible by users with the convenor role? + t.boolean :convenor_only, null: false + + t.timestamps + end + end +end diff --git a/db/migrate/20260106005505_add_mentor_to_unit_role.rb b/db/migrate/20260106005505_add_mentor_to_unit_role.rb new file mode 100644 index 0000000000..298e5620e5 --- /dev/null +++ b/db/migrate/20260106005505_add_mentor_to_unit_role.rb @@ -0,0 +1,5 @@ +class AddMentorToUnitRole < ActiveRecord::Migration[8.0] + def change + add_reference :unit_roles, :mentor, index: true + end +end diff --git a/db/schema.rb b/db/schema.rb index 3b78acfba1..03bbf4073a 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_06_005505) 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 @@ -587,6 +587,23 @@ t.index ["tii_task_similarity_id"], name: "index_tii_submissions_on_tii_task_similarity_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 "tutor_notes_id" + t.bigint "reply_to_id" + t.boolean "convenor_only", 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 ["tutor_notes_id"], name: "index_tutor_notes_on_tutor_notes_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 +656,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/factories/tutor_notes.rb b/test/factories/tutor_notes.rb new file mode 100644 index 0000000000..de9b15338f --- /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/tutor_note_test.rb b/test/models/tutor_note_test.rb new file mode 100644 index 0000000000..9cfd9a5937 --- /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 From 036e34229442188bc4e55b46650034762e9d0494 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Thu, 8 Jan 2026 13:09:21 +1100 Subject: [PATCH 02/26] feat: init moderation inbox endpoint --- app/api/units_api.rb | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 2bd22e45ec..4b800ee1eb 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -303,6 +303,43 @@ 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 + + my_unit_role = unit.unit_role_for(current_user) + mentees = unit.staff.where(mentor_id: my_unit_role.id) + + # TODO: search for ModeratedTask's where feedback has been left by the tutor since last_moderation_date + + tasks = unit.student_tasks + .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' + ) + present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter + end + desc 'Download the grades for a unit' get '/units/:id/grades' do unit = Unit.find(params[:id]) From 4df0d2cd090064515dc1b1b1a417eaa36cbf6d04 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Fri, 9 Jan 2026 15:41:15 +1100 Subject: [PATCH 03/26] feat: moderated tasks --- app/api/units_api.rb | 24 ++++++++++++++++--- app/models/moderated_task.rb | 4 ++++ app/models/task.rb | 11 +++++++++ app/sidekiq/accept_submission_job.rb | 4 ++++ .../20260109033055_create_moderated_tasks.rb | 14 +++++++++++ db/schema.rb | 14 ++++++++++- test/factories/moderated_tasks.rb | 5 ++++ test/models/moderated_task_test.rb | 7 ++++++ 8 files changed, 79 insertions(+), 4 deletions(-) create mode 100644 app/models/moderated_task.rb create mode 100644 db/migrate/20260109033055_create_moderated_tasks.rb create mode 100644 test/factories/moderated_tasks.rb create mode 100644 test/models/moderated_task_test.rb diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 4b800ee1eb..d52622e710 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -314,9 +314,11 @@ class UnitsApi < Grape::API my_unit_role = unit.unit_role_for(current_user) mentees = unit.staff.where(mentor_id: my_unit_role.id) - # TODO: search for ModeratedTask's where feedback has been left by the tutor since last_moderation_date - tasks = unit.student_tasks + .includes(:comments) + .left_joins(:moderated_task) + .where(moderated_tasks: { dismissed: false }) + .where(projects: { unit_id: unit.id }) .joins(:task_definition) .joins(project: { tutorial_enrolments: :tutorial }) .where(tutorials: { unit_role_id: mentees.select(:id) }) @@ -335,8 +337,24 @@ class UnitsApi < Grape::API '0 AS number_unread', '0 AS similar_to_count', 'false AS pinned', - 'false AS has_extensions' + '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.select do |task| + tutor_comments = task.comments.select { |c| c.user == task.tutor } + next false if tutor_comments.empty? + + most_recent = tutor_comments.max_by(&:created_at) + most_recent.created_at <= comment_threshold && + most_recent.created_at > (task.moderated_task&.last_moderated_date || Time.at(0)) + end + present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter end diff --git a/app/models/moderated_task.rb b/app/models/moderated_task.rb new file mode 100644 index 0000000000..734cd2b6f8 --- /dev/null +++ b/app/models/moderated_task.rb @@ -0,0 +1,4 @@ +class ModeratedTask < ApplicationRecord + belongs_to :task + belongs_to :user, optional: true +end diff --git a/app/models/task.rb b/app/models/task.rb index e6a166fc6f..49621b2778 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 @@ -1577,6 +1578,16 @@ def overseer_enabled? (has_new_files? || has_done_file?) end + def mark_as_moderated + moderated_task = ModeratedTask.find_by(task_id: id) + if moderated_task.nil? + ModeratedTask.create!({ + task: self, + last_moderated_date: Time.zone.now + }) + end + end + private def delete_associated_files diff --git a/app/sidekiq/accept_submission_job.rb b/app/sidekiq/accept_submission_job.rb index a8a44bf7f9..9c2d80cfeb 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -45,6 +45,10 @@ def perform(task_id, user_id, accepted_tii_eula, test_submission) return end + # Mark this task for moderation + # TODO: base it on the task's tutor's trust factor + task.mark_as_moderated if rand < 0.5 + # 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/db/migrate/20260109033055_create_moderated_tasks.rb b/db/migrate/20260109033055_create_moderated_tasks.rb new file mode 100644 index 0000000000..d57c16d4ed --- /dev/null +++ b/db/migrate/20260109033055_create_moderated_tasks.rb @@ -0,0 +1,14 @@ +class CreateModeratedTasks < ActiveRecord::Migration[8.0] + def change + create_table :moderated_tasks do |t| + t.references :task, null: false, foreign_key: true, index: { unique: true } + t.datetime :last_moderated_date, null: true + + # User that dismissed the moderated task + t.references :user, null: true + t.boolean :dismissed, null: false, default: false + + t.timestamps + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 03bbf4073a..a38a815fea 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: 2026_01_06_005505) do +ActiveRecord::Schema[8.0].define(version: 2026_01_09_033055) 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,17 @@ 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.datetime "last_moderated_date" + t.bigint "user_id" + t.boolean "dismissed", default: false, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["task_id"], name: "index_moderated_tasks_on_task_id", unique: true + t.index ["user_id"], name: "index_moderated_tasks_on_user_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 @@ -781,6 +792,7 @@ add_foreign_key "feedback_chips", "learning_outcomes" add_foreign_key "learning_outcome_links", "learning_outcomes", column: "source_id" add_foreign_key "learning_outcome_links", "learning_outcomes", column: "target_id" + add_foreign_key "moderated_tasks", "tasks" add_foreign_key "user_oauth_states", "users" add_foreign_key "user_oauth_tokens", "users" end diff --git a/test/factories/moderated_tasks.rb b/test/factories/moderated_tasks.rb new file mode 100644 index 0000000000..df1075db35 --- /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/models/moderated_task_test.rb b/test/models/moderated_task_test.rb new file mode 100644 index 0000000000..0a7b20a106 --- /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 From a4b2cc86737ec2cf09ae0418f4383893a2a6839e Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 12 Jan 2026 13:53:00 +1100 Subject: [PATCH 04/26] feat: add unit role trust factor --- app/api/unit_roles_api.rb | 51 +++++++++++++++++++ app/sidekiq/accept_submission_job.rb | 9 +++- ...260112021148_add_unit_role_trust_factor.rb | 5 ++ db/schema.rb | 3 +- 4 files changed, 65 insertions(+), 3 deletions(-) create mode 100644 db/migrate/20260112021148_add_unit_role_trust_factor.rb diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index ec5319cd38..111d11dece 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -107,4 +107,55 @@ 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 :score, type: Integer, desc: 'Moderation for the task' + 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 + + score = params[:score] + return error!({ error: 'Invalid moderation score' }, 400) unless [-1, 0, 1].include?(score) + + moderated_task = ModeratedTask.find_by(task: task) + + recent_threshold = 15.minutes.ago + if moderated_task.last_moderated_date && moderated_task.last_moderated_date > recent_threshold + error!({ error: 'Invalid moderation score' }, 400) + end + + # TODO: adjust scale via ENV var? + delta = score.to_i * 5 + + unit_role.update!( + trust_factor: (unit_role.trust_factor + delta).clamp(0, 99) + ) + + moderated_task.update!(last_moderated_date: Time.zone.now) + + unless score == -1 + moderated_task.update!({ + dismissed: true, + user: current_user + }) + end + + true + end end diff --git a/app/sidekiq/accept_submission_job.rb b/app/sidekiq/accept_submission_job.rb index 9c2d80cfeb..bf9d8c6564 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -46,8 +46,13 @@ def perform(task_id, user_id, accepted_tii_eula, test_submission) end # Mark this task for moderation - # TODO: base it on the task's tutor's trust factor - task.mark_as_moderated if rand < 0.5 + tutor_user = task.project.tutor_for(task.task_definition) + if tutor_user + tutor = task.unit.unit_role_for(tutor_user) + if tutor && rand(0..100) > tutor.trust_factor + task.mark_as_moderated + end + end # When converted, we can now send documents to turn it in for checking if TurnItIn.enabled? diff --git a/db/migrate/20260112021148_add_unit_role_trust_factor.rb b/db/migrate/20260112021148_add_unit_role_trust_factor.rb new file mode 100644 index 0000000000..7f01bd4bc2 --- /dev/null +++ b/db/migrate/20260112021148_add_unit_role_trust_factor.rb @@ -0,0 +1,5 @@ +class AddUnitRoleTrustFactor < ActiveRecord::Migration[8.0] + def change + add_column :unit_roles, :trust_factor, :integer, default: 50, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index a38a815fea..241d13af6d 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: 2026_01_09_033055) do +ActiveRecord::Schema[8.0].define(version: 2026_01_12_021148) 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 @@ -668,6 +668,7 @@ t.bigint "unit_id" t.boolean "observer_only", default: false t.bigint "mentor_id" + t.integer "trust_factor", default: 50, null: false 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" From 7ea9df25b3250c6dbb27542bc94955dd797b44d0 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 12 Jan 2026 13:55:47 +1100 Subject: [PATCH 05/26] fix: use time.zone --- app/api/units_api.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/api/units_api.rb b/app/api/units_api.rb index d52622e710..520b10cd9c 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -352,7 +352,7 @@ class UnitsApi < Grape::API most_recent = tutor_comments.max_by(&:created_at) most_recent.created_at <= comment_threshold && - most_recent.created_at > (task.moderated_task&.last_moderated_date || Time.at(0)) + most_recent.created_at > (task.moderated_task&.last_moderated_date || Time.zone.at(0)) end present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter From 48142a9e8ef169fa2b7bb2f2be78adc7e592e303 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 12 Jan 2026 15:11:25 +1100 Subject: [PATCH 06/26] refactor: expose tutor notes count --- app/api/entities/unit_role_entity.rb | 8 ++++++-- app/api/unit_roles_api.rb | 8 +++++--- app/models/unit_role.rb | 2 ++ 3 files changed, 13 insertions(+), 5 deletions(-) diff --git a/app/api/entities/unit_role_entity.rb b/app/api/entities/unit_role_entity.rb index 98bf1f21d6..3cf8cef3a9 100644 --- a/app/api/entities/unit_role_entity.rb +++ b/app/api/entities/unit_role_entity.rb @@ -5,7 +5,11 @@ class UnitRoleEntity < Grape::Entity expose :user, using: Entities::Minimal::MinimalUserEntity expose :unit, using: Entities::Minimal::MinimalUnitEntity, unless: :in_unit expose :observer_only - # TODO: expose to staff only - expose :mentor_id + + expose :mentor_id, if: ->(unit_role, options) { unit_role.unit.unit_role_for(options[:user]) } + expose :tutor_note_count, if: ->(unit_role, options) { unit_role.unit.unit_role_for(options[:user])&.id == unit_role.id } do |unit_role, options| + # TODO: get unread tutor notes only + unit_role.unit.unit_role_for(options[:user]).tutor_notes.count + end end end diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index 111d11dece..78a37dde2b 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -130,14 +130,16 @@ class UnitRolesApi < Grape::API error!({ error: 'You do not have permission to moderate this feedback' }, 400) end - score = params[:score] - return error!({ error: 'Invalid moderation score' }, 400) unless [-1, 0, 1].include?(score) + score = params[:score].to_i + unless [-1, 0, 1].include?(score) + error!({ error: 'Invalid moderation score' }, 400) + end moderated_task = ModeratedTask.find_by(task: task) recent_threshold = 15.minutes.ago if moderated_task.last_moderated_date && moderated_task.last_moderated_date > recent_threshold - error!({ error: 'Invalid moderation score' }, 400) + error!({ error: 'Feedback is too new to moderate' }, 400) end # TODO: adjust scale via ENV var? diff --git a/app/models/unit_role.rb b/app/models/unit_role.rb index bd5dc5935a..81197c9de3 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -5,6 +5,8 @@ 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 From e7839b4dbbc26b7b26fac3fd3a7b6c16bc82b945 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 13:33:35 +1100 Subject: [PATCH 07/26] refactor: add tutor notes permissions --- app/api/entities/tutor_note_entity.rb | 4 ++ app/api/tutor_notes_api.rb | 73 +++++++++++++++++++-------- app/models/tutor_note.rb | 9 ++++ app/models/unit_role.rb | 10 ++-- 4 files changed, 72 insertions(+), 24 deletions(-) diff --git a/app/api/entities/tutor_note_entity.rb b/app/api/entities/tutor_note_entity.rb index 5f4997eb80..923d752d78 100644 --- a/app/api/entities/tutor_note_entity.rb +++ b/app/api/entities/tutor_note_entity.rb @@ -11,6 +11,10 @@ class TutorNoteEntity < Grape::Entity expose :reply_to_id + expose :task_id + expose :task_definition_id + expose :project_id + # TODO: what is tutor_notes_id? end diff --git a/app/api/tutor_notes_api.rb b/app/api/tutor_notes_api.rb index 92304b467c..dc251f0605 100644 --- a/app/api/tutor_notes_api.rb +++ b/app/api/tutor_notes_api.rb @@ -8,6 +8,16 @@ class TutorNotesApi < Grape::API 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' @@ -19,14 +29,11 @@ class TutorNotesApi < Grape::API error!({ error: 'You do not have permission to access this unit' }, 403) end - current_user_role = unit.unit_role_for(current_user) - - can_access = - current_user_role == Role.convenor || - unit_role.mentor_id == current_user_role.id || - unit_role == current_user_role + unless authorise? current_user, unit_role, :create_tutor_note + error!({ error: 'You do not have permission to access this.' }, 403) + end - unless can_access + unless can_access_tutor_notes?(unit, current_user, unit_role) error!({ error: 'You do not have permission to access this.' }, 403) end @@ -35,16 +42,25 @@ class TutorNotesApi < Grape::API present result, with: Entities::TutorNoteEntity, user: current_user end - desc "Create a new tutor note for a project" + 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 staff note this is being replied to' + 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) + 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] @@ -56,7 +72,13 @@ class TutorNotesApi < Grape::API error!(error: 'Original tutor note is not in this project.') if unit_role.tutor_notes.find(reply_to_id).blank? end - result = unit_role.add_tutor_note(current_user, text_note, reply_to_id) + 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) @@ -73,14 +95,17 @@ class TutorNotesApi < Grape::API 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]) - # TODO: delete permissions - # unless authorise?(current_user, project, :delete_staff_note) || staff_note.user.id == current_user.id - # error!({ error: 'You do not have permission to delete this note.' }, 403) - # end + error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.unit_role != unit_role - # error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.user_role != user_role.id + 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? @@ -101,15 +126,21 @@ class TutorNotesApi < Grape::API 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]) - # TODO: permissions + unless can_access_tutor_notes?(unit, current_user, unit_role) + error!({ error: 'You do not have permission to access create note.' }, 403) + end - # unless authorise?(current_user, project, :create_staff_note) && staff_note.user.id == current_user.id - # error!({ error: 'You do not have permission to edit this note.' }, 403) - # end + error!({ error: 'Note does not belong to this tutor' }, 404) if tutor_note.unit_role != unit_role - # error!({ error: 'Note does not belong to this project' }, 404) if staff_note.project_id != project.id + 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 diff --git a/app/models/tutor_note.rb b/app/models/tutor_note.rb index 98d85f6f6f..c97461409d 100644 --- a/app/models/tutor_note.rb +++ b/app/models/tutor_note.rb @@ -1,5 +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_role.rb b/app/models/unit_role.rb index 81197c9de3..8381b00846 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -51,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 = [] @@ -291,7 +294,7 @@ def get_marking_sessions_csv(start_date: nil, end_date: nil, timezone: nil) end - def add_tutor_note(user, text, reply_to_id = nil) + 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? @@ -305,6 +308,7 @@ def add_tutor_note(user, text, reply_to_id = nil) note.user = user note.unit_role = self note.reply_to_id = reply_to_id + note.task_id = task_id note.convenor_only = false note.save! note From 8c0d5810e6b13fb522b395f66adf0c3a8a5e30cc Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 15:03:29 +1100 Subject: [PATCH 08/26] refactor: rebase schema and mark tutor notes as read --- app/api/entities/tutor_note_entity.rb | 2 +- app/api/tutor_notes_api.rb | 29 +++++++++++++++++++ app/api/unit_roles_api.rb | 2 +- app/models/unit_role.rb | 2 +- .../20260105234326_create_tutor_notes.rb | 17 ----------- .../20260106005505_add_mentor_to_unit_role.rb | 5 ---- .../20260109033055_create_moderated_tasks.rb | 14 --------- ...260112021148_add_unit_role_trust_factor.rb | 5 ---- .../20260113033152_add_moderation_feat.rb | 29 +++++++++++++++++++ db/schema.rb | 6 ++-- 10 files changed, 63 insertions(+), 48 deletions(-) delete mode 100644 db/migrate/20260105234326_create_tutor_notes.rb delete mode 100644 db/migrate/20260106005505_add_mentor_to_unit_role.rb delete mode 100644 db/migrate/20260109033055_create_moderated_tasks.rb delete mode 100644 db/migrate/20260112021148_add_unit_role_trust_factor.rb create mode 100644 db/migrate/20260113033152_add_moderation_feat.rb diff --git a/app/api/entities/tutor_note_entity.rb b/app/api/entities/tutor_note_entity.rb index 923d752d78..1184b01d2a 100644 --- a/app/api/entities/tutor_note_entity.rb +++ b/app/api/entities/tutor_note_entity.rb @@ -15,7 +15,7 @@ class TutorNoteEntity < Grape::Entity expose :task_definition_id expose :project_id - # TODO: what is tutor_notes_id? + expose :read_by_unit_role end end diff --git a/app/api/tutor_notes_api.rb b/app/api/tutor_notes_api.rb index dc251f0605..ddb4cbc4a3 100644 --- a/app/api/tutor_notes_api.rb +++ b/app/api/tutor_notes_api.rb @@ -42,6 +42,35 @@ def can_access_tutor_notes?(unit, current_user, unit_role) 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' diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index 265803f27a..d6a137a065 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -147,7 +147,7 @@ class UnitRolesApi < Grape::API end # TODO: adjust scale via ENV var? - delta = score.to_i * 5 + delta = score.to_i * 2 unit_role.update!( trust_factor: (unit_role.trust_factor + delta).clamp(0, 99) diff --git a/app/models/unit_role.rb b/app/models/unit_role.rb index 8381b00846..affe78e2dc 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -309,7 +309,7 @@ def add_tutor_note(user, text, task_id = nil, reply_to_id = nil) note.unit_role = self note.reply_to_id = reply_to_id note.task_id = task_id - note.convenor_only = false + note.read_by_unit_role = false note.save! note end diff --git a/db/migrate/20260105234326_create_tutor_notes.rb b/db/migrate/20260105234326_create_tutor_notes.rb deleted file mode 100644 index d000d550dc..0000000000 --- a/db/migrate/20260105234326_create_tutor_notes.rb +++ /dev/null @@ -1,17 +0,0 @@ -class CreateTutorNotes < 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 :unit_roles, :mentor, index: true, null: false - t.references :user, index: true, null: false - t.references :tutor_notes, :reply_to, index: true, null: true - - # TODO: Only accessible by users with the convenor role? - t.boolean :convenor_only, null: false - - t.timestamps - end - end -end diff --git a/db/migrate/20260106005505_add_mentor_to_unit_role.rb b/db/migrate/20260106005505_add_mentor_to_unit_role.rb deleted file mode 100644 index 298e5620e5..0000000000 --- a/db/migrate/20260106005505_add_mentor_to_unit_role.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddMentorToUnitRole < ActiveRecord::Migration[8.0] - def change - add_reference :unit_roles, :mentor, index: true - end -end diff --git a/db/migrate/20260109033055_create_moderated_tasks.rb b/db/migrate/20260109033055_create_moderated_tasks.rb deleted file mode 100644 index d57c16d4ed..0000000000 --- a/db/migrate/20260109033055_create_moderated_tasks.rb +++ /dev/null @@ -1,14 +0,0 @@ -class CreateModeratedTasks < ActiveRecord::Migration[8.0] - def change - create_table :moderated_tasks do |t| - t.references :task, null: false, foreign_key: true, index: { unique: true } - t.datetime :last_moderated_date, null: true - - # User that dismissed the moderated task - t.references :user, null: true - t.boolean :dismissed, null: false, default: false - - t.timestamps - end - end -end diff --git a/db/migrate/20260112021148_add_unit_role_trust_factor.rb b/db/migrate/20260112021148_add_unit_role_trust_factor.rb deleted file mode 100644 index 7f01bd4bc2..0000000000 --- a/db/migrate/20260112021148_add_unit_role_trust_factor.rb +++ /dev/null @@ -1,5 +0,0 @@ -class AddUnitRoleTrustFactor < ActiveRecord::Migration[8.0] - def change - add_column :unit_roles, :trust_factor, :integer, default: 50, null: false - end -end diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb new file mode 100644 index 0000000000..e33ade5d11 --- /dev/null +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -0,0 +1,29 @@ +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 :moderated_tasks do |t| + t.references :task, null: false, foreign_key: true, index: { unique: true } + t.datetime :last_moderated_date, null: true + + # User that dismissed the moderated task + t.references :user, null: true + t.boolean :dismissed, null: false, default: false + + t.timestamps + end + + add_reference :unit_roles, :mentor, index: true + add_column :unit_roles, :trust_factor, :integer, default: 50, null: false + end +end diff --git a/db/schema.rb b/db/schema.rb index 241d13af6d..3678ddfd8d 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: 2026_01_12_021148) 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 @@ -603,14 +603,12 @@ t.bigint "task_id" t.bigint "unit_role_id", null: false t.bigint "user_id", null: false - t.bigint "tutor_notes_id" t.bigint "reply_to_id" - t.boolean "convenor_only", null: false + 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 ["tutor_notes_id"], name: "index_tutor_notes_on_tutor_notes_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 From d4fef973ada33997ded918ef6fabc4c0e36d7ea3 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 15:26:48 +1100 Subject: [PATCH 09/26] refactor: set correct permissions for unit role entity --- app/api/entities/unit_role_entity.rb | 10 +++++++--- app/api/units_api.rb | 2 +- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/app/api/entities/unit_role_entity.rb b/app/api/entities/unit_role_entity.rb index 3cf8cef3a9..c989d99b55 100644 --- a/app/api/entities/unit_role_entity.rb +++ b/app/api/entities/unit_role_entity.rb @@ -1,15 +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) { unit_role.unit.unit_role_for(options[:user]) } + 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| - # TODO: get unread tutor notes only - unit_role.unit.unit_role_for(options[:user]).tutor_notes.count + unit_role.tutor_notes.where(read_by_unit_role: false).count end end end diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 520b10cd9c..4384cb894f 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' From 48dfdbb3918d310d11d491a5f4925bd7ba8061c6 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 15:27:19 +1100 Subject: [PATCH 10/26] refactor: sort tasks by showing oldest feedback first --- app/api/units_api.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 4384cb894f..64b8e13f8d 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -346,14 +346,21 @@ class UnitsApi < Grape::API # to ensure that the feedback is likely complete before adding it for moderation comment_threshold = 15.minutes.ago - tasks = tasks.select do |task| + tasks = tasks.map do |task| tutor_comments = task.comments.select { |c| c.user == task.tutor } - next false if tutor_comments.empty? + next nil if tutor_comments.empty? most_recent = tutor_comments.max_by(&:created_at) - most_recent.created_at <= comment_threshold && - most_recent.created_at > (task.moderated_task&.last_moderated_date || Time.zone.at(0)) - end + 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.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) present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter end From 1f99bc53f0ecc2ccfb581e3e7a4a5caa24c872a2 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 15:56:18 +1100 Subject: [PATCH 11/26] chore: check for mentor_id key --- test/api/units_api_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api/units_api_test.rb b/test/api/units_api_test.rb index 8915b884ec..6833236143 100644 --- a/test/api/units_api_test.rb +++ b/test/api/units_api_test.rb @@ -345,7 +345,7 @@ 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] + keys = %w[id role user observer_only mentor_id] assert_json_limit_keys_to_exactly keys, staff ur = UnitRole.find(staff['id']) assert_equal ur.id, staff['id'] From 227f31a39cae0881fdcad16e2dd8b228f376df76 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 13 Jan 2026 16:14:52 +1100 Subject: [PATCH 12/26] test: check for tutor_note_count key --- test/api/units_api_test.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/api/units_api_test.rb b/test/api/units_api_test.rb index 6833236143..1d579a5284 100644 --- a/test/api/units_api_test.rb +++ b/test/api/units_api_test.rb @@ -345,7 +345,7 @@ 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 mentor_id] + keys = %w[id role user observer_only mentor_id tutor_note_count] assert_json_limit_keys_to_exactly keys, staff ur = UnitRole.find(staff['id']) assert_equal ur.id, staff['id'] From 462f5540bb15736b1960e79172551eb535b29874 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 2 Feb 2026 14:22:51 +1100 Subject: [PATCH 13/26] test: ensure tutor note count is only exposed to current user --- test/api/units_api_test.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/test/api/units_api_test.rb b/test/api/units_api_test.rb index 1d579a5284..d73cdaa001 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 mentor_id tutor_note_count] - 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'] From 93bb8b19afe85d756d3fb7d769bfd520f99414a6 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 2 Feb 2026 17:28:38 +1100 Subject: [PATCH 14/26] refactor: move moderation task logic to unit model --- app/api/units_api.rb | 51 +---------------------------------- app/models/unit.rb | 64 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+), 50 deletions(-) diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 64b8e13f8d..7ad1b4e1bc 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -311,56 +311,7 @@ class UnitsApi < Grape::API error!({ error: 'Not authorised to provide feedback for this unit' }, 403) end - my_unit_role = unit.unit_role_for(current_user) - mentees = unit.staff.where(mentor_id: my_unit_role.id) - - tasks = unit.student_tasks - .includes(:comments) - .left_joins(:moderated_task) - .where(moderated_tasks: { dismissed: false }) - .where(projects: { unit_id: unit.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| - 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.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) + tasks = unit.tasks_for_moderation(current_user) present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter end diff --git a/app/models/unit.rb b/app/models/unit.rb index 077fc5baef..bc43deb252 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -2138,6 +2138,70 @@ 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 for moderation by the tutor's mentor + # + # Tasks that are included are: + # - If a ModeratedTask exists for a task + # - 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: { dismissed: false }) + .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| + 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.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 # From fa86d00d16bb20bb23f22b8256b7c57061bc68ec Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 2 Feb 2026 17:34:32 +1100 Subject: [PATCH 15/26] refactor: add type to moderated task schema --- app/models/task.rb | 1 + db/migrate/20260113033152_add_moderation_feat.rb | 2 ++ db/schema.rb | 1 + 3 files changed, 4 insertions(+) diff --git a/app/models/task.rb b/app/models/task.rb index 49621b2778..93c4c029ca 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1583,6 +1583,7 @@ def mark_as_moderated if moderated_task.nil? ModeratedTask.create!({ task: self, + type: :moderation, last_moderated_date: Time.zone.now }) end diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb index e33ade5d11..89133ba04e 100644 --- a/db/migrate/20260113033152_add_moderation_feat.rb +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -20,6 +20,8 @@ def change t.references :user, null: true t.boolean :dismissed, null: false, default: false + t.string :type, null: false + t.timestamps end diff --git a/db/schema.rb b/db/schema.rb index 3678ddfd8d..a4f6b29d39 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -213,6 +213,7 @@ t.datetime "last_moderated_date" t.bigint "user_id" t.boolean "dismissed", default: false, null: false + t.string "type", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["task_id"], name: "index_moderated_tasks_on_task_id", unique: true From 1c179b0602fd09312a5c0d72cb406b619185e7cb Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 3 Feb 2026 10:57:41 +1100 Subject: [PATCH 16/26] refactor: rename field to moderation_type --- app/models/task.rb | 2 +- db/migrate/20260113033152_add_moderation_feat.rb | 2 +- db/schema.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/task.rb b/app/models/task.rb index 93c4c029ca..58dd975656 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1583,7 +1583,7 @@ def mark_as_moderated if moderated_task.nil? ModeratedTask.create!({ task: self, - type: :moderation, + moderation_type: :moderation, last_moderated_date: Time.zone.now }) end diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb index 89133ba04e..ec1237a769 100644 --- a/db/migrate/20260113033152_add_moderation_feat.rb +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -20,7 +20,7 @@ def change t.references :user, null: true t.boolean :dismissed, null: false, default: false - t.string :type, null: false + t.string :moderation_type, null: false t.timestamps end diff --git a/db/schema.rb b/db/schema.rb index a4f6b29d39..ce1b16cdee 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -213,7 +213,7 @@ t.datetime "last_moderated_date" t.bigint "user_id" t.boolean "dismissed", default: false, null: false - t.string "type", null: false + t.string "moderation_type", null: false t.datetime "created_at", null: false t.datetime "updated_at", null: false t.index ["task_id"], name: "index_moderated_tasks_on_task_id", unique: true From c3e16c2ec104d393d600a51bcf649c43eba543d0 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 3 Feb 2026 11:06:23 +1100 Subject: [PATCH 17/26] feat: add env var for score factor --- app/api/unit_roles_api.rb | 4 ++-- config/application.rb | 3 +++ 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index d6a137a065..f064917bb5 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -146,8 +146,8 @@ class UnitRolesApi < Grape::API error!({ error: 'Feedback is too new to moderate' }, 400) end - # TODO: adjust scale via ENV var? - delta = score.to_i * 2 + factor = Doubtfire::Application.config.moderation_score_factor + delta = score.to_i * factor unit_role.update!( trust_factor: (unit_role.trust_factor + delta).clamp(0, 99) diff --git a/config/application.rb b/config/application.rb index 19ea421559..770faf7fc4 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 From c003b3797fcf5d9d03c2a97ab3228c04939bac4d Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 4 Feb 2026 10:26:21 +1100 Subject: [PATCH 18/26] chore: add moderation log --- app/sidekiq/accept_submission_job.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/sidekiq/accept_submission_job.rb b/app/sidekiq/accept_submission_job.rb index bf9d8c6564..530db7fded 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -50,6 +50,7 @@ def perform(task_id, user_id, accepted_tii_eula, test_submission) if tutor_user tutor = task.unit.unit_role_for(tutor_user) if tutor && rand(0..100) > tutor.trust_factor + logger.info "Marking task #{task.id} for moderation (project #{task.project.id})" task.mark_as_moderated end end From 81c2437b1619f44832334afe790c2decee07aa7e Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 4 Feb 2026 10:33:37 +1100 Subject: [PATCH 19/26] chore: fix wording --- app/models/unit.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/models/unit.rb b/app/models/unit.rb index bc43deb252..ecdeb85db5 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -2139,10 +2139,10 @@ def tasks_for_task_inbox(user, my_students_only = false) end # - # Return the tasks that have been selected for moderation by the tutor's mentor + # 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 + # - 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 From 9f9ea0eb34a8d76fe5453446a1e69583e552c6bb Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 9 Feb 2026 11:58:18 +1100 Subject: [PATCH 20/26] refactor: track feedback score per task definition --- app/api/unit_roles_api.rb | 8 ++-- app/api/units_api.rb | 27 +++++++++++- app/models/moderated_task.rb | 7 ++++ app/models/task.rb | 7 +++- app/models/tutor_feedback_score.rb | 4 ++ app/models/unit_role.rb | 19 +++++++++ app/sidekiq/accept_submission_job.rb | 2 +- .../20260113033152_add_moderation_feat.rb | 41 ++++++++++++++++--- db/schema.rb | 26 +++++++++--- test/factories/tutor_feedback_scores.rb | 5 +++ test/models/tutor_feedback_score_test.rb | 7 ++++ 11 files changed, 134 insertions(+), 19 deletions(-) create mode 100644 app/models/tutor_feedback_score.rb create mode 100644 test/factories/tutor_feedback_scores.rb create mode 100644 test/models/tutor_feedback_score_test.rb diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index f064917bb5..09fa9f9518 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -149,15 +149,17 @@ class UnitRolesApi < Grape::API factor = Doubtfire::Application.config.moderation_score_factor delta = score.to_i * factor - unit_role.update!( - trust_factor: (unit_role.trust_factor + delta).clamp(0, 99) + 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!({ - dismissed: true, + state: :resolved, user: current_user }) end diff --git a/app/api/units_api.rb b/app/api/units_api.rb index 7ad1b4e1bc..395adf598a 100644 --- a/app/api/units_api.rb +++ b/app/api/units_api.rb @@ -311,9 +311,32 @@ class UnitsApi < Grape::API error!({ error: 'Not authorised to provide feedback for this unit' }, 403) end - tasks = unit.tasks_for_moderation(current_user) + unless authorise? current_user, unit, :provide_feedback + error!({ error: 'Not authorised to provide feedback for this unit' }, 403) + end - present unit.tasks_as_hash(tasks), with: Grape::Presenters::Presenter + 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' diff --git a/app/models/moderated_task.rb b/app/models/moderated_task.rb index 734cd2b6f8..4085093bad 100644 --- a/app/models/moderated_task.rb +++ b/app/models/moderated_task.rb @@ -1,4 +1,11 @@ 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' + } end diff --git a/app/models/task.rb b/app/models/task.rb index 58dd975656..037e6b15d7 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1578,12 +1578,15 @@ def overseer_enabled? (has_new_files? || has_done_file?) end - def mark_as_moderated + def mark_as_moderated(moderation_type: :sample) moderated_task = ModeratedTask.find_by(task_id: id) if moderated_task.nil? ModeratedTask.create!({ task: self, - moderation_type: :moderation, + task_definition: task_definition, + assessor_id: tutor.id, + state: :open, + moderation_type: moderation_type, last_moderated_date: Time.zone.now }) end diff --git a/app/models/tutor_feedback_score.rb b/app/models/tutor_feedback_score.rb new file mode 100644 index 0000000000..e18ed56749 --- /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/unit_role.rb b/app/models/unit_role.rb index affe78e2dc..d63271c922 100644 --- a/app/models/unit_role.rb +++ b/app/models/unit_role.rb @@ -313,4 +313,23 @@ def add_tutor_note(user, text, task_id = nil, reply_to_id = nil) 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 530db7fded..00c590f61e 100644 --- a/app/sidekiq/accept_submission_job.rb +++ b/app/sidekiq/accept_submission_job.rb @@ -49,7 +49,7 @@ def perform(task_id, user_id, accepted_tii_eula, test_submission) tutor_user = task.project.tutor_for(task.task_definition) if tutor_user tutor = task.unit.unit_role_for(tutor_user) - if tutor && rand(0..100) > tutor.trust_factor + if tutor&.should_moderate_task?(task) logger.info "Marking task #{task.id} for moderation (project #{task.project.id})" task.mark_as_moderated end diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb index ec1237a769..58ef39db81 100644 --- a/db/migrate/20260113033152_add_moderation_feat.rb +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -12,20 +12,51 @@ def change 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, foreign_key: true, index: { unique: true } + t.references :task, null: false + t.references :task_definition, null: false + t.datetime :last_moderated_date, null: true - # User that dismissed the moderated task - t.references :user, null: true - t.boolean :dismissed, null: false, default: false + # open | waiting_for_new_feedback | resolved + t.string :state, null: false + # sample | 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 - add_column :unit_roles, :trust_factor, :integer, default: 50, null: false end end diff --git a/db/schema.rb b/db/schema.rb index ce1b16cdee..fead37d309 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -210,14 +210,20 @@ 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.bigint "user_id" - t.boolean "dismissed", default: false, null: false + 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 ["task_id"], name: "index_moderated_tasks_on_task_id", unique: true - t.index ["user_id"], name: "index_moderated_tasks_on_user_id" + 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| @@ -599,6 +605,16 @@ 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" @@ -667,7 +683,6 @@ t.bigint "unit_id" t.boolean "observer_only", default: false t.bigint "mentor_id" - t.integer "trust_factor", default: 50, null: false 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" @@ -792,7 +807,6 @@ add_foreign_key "feedback_chips", "learning_outcomes" add_foreign_key "learning_outcome_links", "learning_outcomes", column: "source_id" add_foreign_key "learning_outcome_links", "learning_outcomes", column: "target_id" - add_foreign_key "moderated_tasks", "tasks" add_foreign_key "user_oauth_states", "users" add_foreign_key "user_oauth_tokens", "users" end diff --git a/test/factories/tutor_feedback_scores.rb b/test/factories/tutor_feedback_scores.rb new file mode 100644 index 0000000000..9ac2a7acd5 --- /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/models/tutor_feedback_score_test.rb b/test/models/tutor_feedback_score_test.rb new file mode 100644 index 0000000000..d7ea5680e5 --- /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 From 051b51e741dc3899a5d05366229a103748167389 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:02:25 +1100 Subject: [PATCH 21/26] feat: moderate first three tasks tutor gives feedback to --- app/models/moderated_task.rb | 6 ++++++ app/models/task.rb | 18 ++++++++++++++++++ .../20260113033152_add_moderation_feat.rb | 2 +- 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/app/models/moderated_task.rb b/app/models/moderated_task.rb index 4085093bad..c73af15d9e 100644 --- a/app/models/moderated_task.rb +++ b/app/models/moderated_task.rb @@ -8,4 +8,10 @@ class ModeratedTask < ApplicationRecord waiting_for_new_feedback: 'waiting_for_new_feedback', resolved: 'resolved' } + + enum :moderation_type, { + first_feedback: 'first_feedback', + sample: 'sample', + escalation: 'escalation' + } end diff --git a/app/models/task.rb b/app/models/task.rb index 037e6b15d7..16681158e6 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -660,6 +660,24 @@ 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 TaskEngagement.create!(task: self, engagement_time: Time.zone.now, engagement: task_status.name) # Grab the submission for the task if the user made one diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb index 58ef39db81..d7ce827cbd 100644 --- a/db/migrate/20260113033152_add_moderation_feat.rb +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -29,7 +29,7 @@ def change # open | waiting_for_new_feedback | resolved t.string :state, null: false - # sample | escalation + # sample | initial_feedback | escalation t.string :moderation_type, null: false t.bigint :assessor_id, null: true From 6577ed1e06ad3d3a955df3caca4ef36894f2169d Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 9 Feb 2026 12:08:09 +1100 Subject: [PATCH 22/26] fix: rename to random sample to avoid ruby conflicts --- app/models/moderated_task.rb | 2 +- app/models/task.rb | 2 +- app/models/unit.rb | 2 +- db/migrate/20260113033152_add_moderation_feat.rb | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/models/moderated_task.rb b/app/models/moderated_task.rb index c73af15d9e..adcc4be8b9 100644 --- a/app/models/moderated_task.rb +++ b/app/models/moderated_task.rb @@ -11,7 +11,7 @@ class ModeratedTask < ApplicationRecord enum :moderation_type, { first_feedback: 'first_feedback', - sample: 'sample', + random_sample: 'random_sample', escalation: 'escalation' } end diff --git a/app/models/task.rb b/app/models/task.rb index 16681158e6..1f16af8246 100644 --- a/app/models/task.rb +++ b/app/models/task.rb @@ -1596,7 +1596,7 @@ def overseer_enabled? (has_new_files? || has_done_file?) end - def mark_as_moderated(moderation_type: :sample) + def mark_as_moderated(moderation_type: :random_sample) moderated_task = ModeratedTask.find_by(task_id: id) if moderated_task.nil? ModeratedTask.create!({ diff --git a/app/models/unit.rb b/app/models/unit.rb index ecdeb85db5..755b048e41 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -2154,7 +2154,7 @@ def tasks_for_moderation(user) tasks = student_tasks .includes(:comments) .left_joins(:moderated_task) - .where(moderated_tasks: { dismissed: false }) + .where(moderated_tasks: { state: %w[open waiting_for_new_feedback] }) .where(projects: { unit_id: id }) .joins(:task_definition) .joins(project: { tutorial_enrolments: :tutorial }) diff --git a/db/migrate/20260113033152_add_moderation_feat.rb b/db/migrate/20260113033152_add_moderation_feat.rb index d7ce827cbd..e110e3eb41 100644 --- a/db/migrate/20260113033152_add_moderation_feat.rb +++ b/db/migrate/20260113033152_add_moderation_feat.rb @@ -29,7 +29,7 @@ def change # open | waiting_for_new_feedback | resolved t.string :state, null: false - # sample | initial_feedback | escalation + # random_sample | initial_feedback | escalation t.string :moderation_type, null: false t.bigint :assessor_id, null: true From 1a4919fee3aee0920c3597c561e1978593b80fe2 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Mon, 9 Feb 2026 13:21:41 +1100 Subject: [PATCH 23/26] feat: init escalation request --- app/api/entities/project_entity.rb | 2 ++ app/api/tasks_api.rb | 38 ++++++++++++++++++++++++++++++ app/models/project.rb | 6 +++++ 3 files changed, 46 insertions(+) diff --git a/app/api/entities/project_entity.rb b/app/api/entities/project_entity.rb index f390efd3c8..054e5bd1c5 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/tasks_api.rb b/app/api/tasks_api.rb index 47a41e64b4..6153c13a06 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/models/project.rb b/app/models/project.rb index 1e57824844..1444730f9b 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? From 21b93b99f2e23d0c7f12021bbed5157539523212 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 10 Feb 2026 11:01:14 +1100 Subject: [PATCH 24/26] test: ensure escalation attempts field is present --- test/api/projects_api_test.rb | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/test/api/projects_api_test.rb b/test/api/projects_api_test.rb index 4e49eced6a..30407838e6 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}" From dff2793697ae987c116394d88bb40c20a8be9511 Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Tue, 10 Feb 2026 12:00:46 +1100 Subject: [PATCH 25/26] refactor: ensure all escalated tasks are present in queue --- app/models/unit.rb | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/app/models/unit.rb b/app/models/unit.rb index 755b048e41..91833d86f0 100644 --- a/app/models/unit.rb +++ b/app/models/unit.rb @@ -2185,14 +2185,21 @@ def tasks_for_moderation(user) comment_threshold = 15.minutes.ago tasks = tasks.map do |task| - tutor_comments = task.comments.select { |c| c.user == task.tutor } - next nil if tutor_comments.empty? + mt = task.moderated_task + next if mt.nil? - 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)) + 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] + [task, most_recent.created_at] + end end.compact # Sort tasks: show tasks with the oldest feedback first From 709302c129ae799f1c9133eb23466eea316fb18f Mon Sep 17 00:00:00 2001 From: b0ink <40929320+b0ink@users.noreply.github.com> Date: Wed, 11 Feb 2026 11:24:01 +1100 Subject: [PATCH 26/26] chore: init action keyword --- app/api/unit_roles_api.rb | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) diff --git a/app/api/unit_roles_api.rb b/app/api/unit_roles_api.rb index 09fa9f9518..8e397359d9 100644 --- a/app/api/unit_roles_api.rb +++ b/app/api/unit_roles_api.rb @@ -116,7 +116,9 @@ class UnitRolesApi < Grape::API 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 :score, type: Integer, desc: 'Moderation for 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]) @@ -134,20 +136,32 @@ class UnitRolesApi < Grape::API error!({ error: 'You do not have permission to moderate this feedback' }, 400) end - score = params[:score].to_i - unless [-1, 0, 1].include?(score) - error!({ error: 'Invalid moderation score' }, 400) + 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 = 15.minutes.ago + 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 = score.to_i * 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)