From 92dc26b45de737a4940394a8229568861a3c0c3a Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 17:01:27 +0800 Subject: [PATCH 01/11] feat(marketplace): preview attempt lifecycle (Increment A) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Design-C marketplace preview groundwork, no schema change: - Attempt#preview? (true when the attempt has no Submission extension row) — the single source of truth for preview-only branching. - Attempt#reset_attempt! — destroys answers, returns to attempting, rebuilds answers (backs the preview 'Reset attempt' action). - Course::Assessment::PreviewAttemptReapingJob (TTL 7d) + daily schedule — deletes only extension-less previews past TTL; real submissions never reaped (mutation-verified). - validate_unique_submission left unchanged: the DB unique index already forbids two attempts per (assessment, creator); normal preview creation hits no prior attempt, and the unscoped rule yields the friendly collision error create-or-reuse relies on. Plan: docs/superpowers/plans/2026-07-23-marketplace-preview-increment-a-lifecycle.md --- .../assessment/preview_attempt_reaping_job.rb | 11 ++++ app/models/course/assessment/attempt.rb | 20 ++++++ config/schedule.yml | 6 ++ .../preview_attempt_reaping_job_spec.rb | 37 +++++++++++ spec/models/course/assessment/attempt_spec.rb | 65 +++++++++++++++++++ 5 files changed, 139 insertions(+) create mode 100644 app/jobs/course/assessment/preview_attempt_reaping_job.rb create mode 100644 spec/jobs/course/assessment/preview_attempt_reaping_job_spec.rb create mode 100644 spec/models/course/assessment/attempt_spec.rb diff --git a/app/jobs/course/assessment/preview_attempt_reaping_job.rb b/app/jobs/course/assessment/preview_attempt_reaping_job.rb new file mode 100644 index 00000000000..481446337c1 --- /dev/null +++ b/app/jobs/course/assessment/preview_attempt_reaping_job.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true +# Deletes throwaway marketplace preview attempts (bare Attempts with no Submission extension) older than +# TTL, cascading their answers/submission_questions via `dependent: :destroy`. `Attempt.previews` scopes to +# extension-less rows, so real submissions are never touched regardless of age. +class Course::Assessment::PreviewAttemptReapingJob < ApplicationJob + TTL = 7.days + + def perform + Course::Assessment::Attempt.previews.where(created_at: ..TTL.ago).find_each(&:destroy!) + end +end diff --git a/app/models/course/assessment/attempt.rb b/app/models/course/assessment/attempt.rb index b63644ff1a3..88878e5b509 100644 --- a/app/models/course/assessment/attempt.rb +++ b/app/models/course/assessment/attempt.rb @@ -207,6 +207,26 @@ def submission_view_blocked?(course_user) !attempting? && !published? && assessment.block_student_viewing_after_submitted? && course_user&.student? end + # A preview is a throwaway attempt with no Submission extension row (marketplace "try it out"). Real + # attempts always have exactly one. This is the single source of truth for preview-only branching. + def preview? + submission.nil? + end + + # Throws away all work on this attempt and starts it over: destroys every answer, returns the attempt to + # the `attempting` state, and rebuilds fresh answers. Backs the marketplace preview "Reset attempt" action. + def reset_attempt! + transaction do + answers.destroy_all + self.submitted_at = nil + self.published_at = nil + self.workflow_state = 'attempting' + save! + answers.reload + create_new_answers + end + end + def questions assessment.randomization.nil? ? assessment.questions : assigned_questions end diff --git a/config/schedule.yml b/config/schedule.yml index 8023ae56211..6979564895b 100644 --- a/config/schedule.yml +++ b/config/schedule.yml @@ -15,3 +15,9 @@ user_email_database_cleanup_job: cron: '0 0 1 * *' class: 'UserEmailDatabaseCleanupJob' queue: 'default' + +# Reap expired marketplace preview attempts once a day at 22:00 UTC, 6:00 AM SGT. +preview_attempt_reaping_job: + cron: '0 22 * * *' + class: 'Course::Assessment::PreviewAttemptReapingJob' + queue: 'default' diff --git a/spec/jobs/course/assessment/preview_attempt_reaping_job_spec.rb b/spec/jobs/course/assessment/preview_attempt_reaping_job_spec.rb new file mode 100644 index 00000000000..5a6b39f595d --- /dev/null +++ b/spec/jobs/course/assessment/preview_attempt_reaping_job_spec.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::PreviewAttemptReapingJob, type: :job do + let(:instance) { Instance.default } + + with_tenant(:instance) do + let(:course) { create(:course) } + let(:assessment) { create(:assessment, :with_mcq_question, course: course) } + let(:previewer) { create(:course_manager, course: course).user } + let(:other_previewer) { create(:course_manager, course: course).user } + + it 'destroys preview attempts older than the TTL' do + old_preview = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + old_preview.update_column(:created_at, (described_class::TTL + 1.day).ago) + + expect { described_class.perform_now }. + to change { Course::Assessment::Attempt.where(id: old_preview.id).exists? }.from(true).to(false) + end + + it 'keeps preview attempts newer than the TTL' do + fresh_preview = create(:course_assessment_attempt, assessment: assessment, creator: other_previewer) + fresh_preview.update_column(:created_at, (described_class::TTL - 1.day).ago) + + expect { described_class.perform_now }. + not_to(change { Course::Assessment::Attempt.where(id: fresh_preview.id).exists? }) + end + + it 'never destroys a real submission even if it is old' do + submission = create(:course_assessment_submission, assessment: assessment) + submission.attempt.update_column(:created_at, (described_class::TTL + 1.day).ago) + + expect { described_class.perform_now }. + not_to(change { Course::Assessment::Attempt.where(id: submission.attempt_id).exists? }) + end + end +end diff --git a/spec/models/course/assessment/attempt_spec.rb b/spec/models/course/assessment/attempt_spec.rb new file mode 100644 index 00000000000..4d5af3b181d --- /dev/null +++ b/spec/models/course/assessment/attempt_spec.rb @@ -0,0 +1,65 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Attempt, type: :model do + let(:instance) { Instance.default } + + with_tenant(:instance) do + let(:course) { create(:course) } + let(:assessment) { create(:assessment, :with_mcq_question, course: course) } + let(:previewer) { create(:course_manager, course: course).user } + + describe '#preview?' do + it 'is true for a bare attempt with no submission extension' do + attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + expect(attempt.preview?).to be(true) + end + + it 'is false for an attempt that has a submission extension' do + submission = create(:course_assessment_submission, assessment: assessment) + expect(submission.attempt.preview?).to be(false) + end + end + + # Characterization of the EXISTING (unchanged) one-attempt-per-creator rule that preview create-or-reuse + # relies on: normal preview creation is allowed (no prior attempt), and a prior attempt yields the + # friendly collision error rather than a raw DB error. No production code changes for this describe. + describe '#validate_unique_submission (relied on by preview create-or-reuse)' do + it 'allows creating a preview attempt when the creator has no prior attempt' do + expect do + create(:course_assessment_attempt, assessment: assessment, creator: previewer) + end.to change { Course::Assessment::Attempt.where(creator: previewer).count }.by(1) + end + + it 'blocks a second attempt for the same creator with the friendly error' do + create(:course_assessment_attempt, assessment: assessment, creator: previewer) + second = build(:course_assessment_attempt, assessment: assessment, creator: previewer) + expect(second).not_to be_valid + expect(second.errors[:base]). + to include(I18n.t('activerecord.errors.models.course/assessment/' \ + 'submission.submission_already_exists')) + end + end + + describe '#reset_attempt!' do + it 'destroys existing answers, returns to attempting, and rebuilds answers' do + attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + attempt.create_new_answers + attempt.answers.each { |answer| answer.update_column(:workflow_state, 'submitted') } + attempt.update_columns(workflow_state: 'submitted', submitted_at: Time.zone.now) + original_answer_ids = attempt.answers.reload.map(&:id) + expect(original_answer_ids).not_to be_empty + + attempt.reset_attempt! + attempt.reload + + expect(attempt.workflow_state).to eq('attempting') + expect(attempt.submitted_at).to be_nil + expect(attempt.published_at).to be_nil + expect(attempt.answers.map(&:id) & original_answer_ids).to be_empty + expect(attempt.answers).not_to be_empty + expect(attempt.answers.map(&:workflow_state)).to all(eq('attempting')) + end + end + end +end From c923fd85a193d4973f0459f75e543ca512dd7ad9 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 17:08:46 +0800 Subject: [PATCH 02/11] feat(marketplace): delegate course-coupled interface on Attempt for preview serving MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Increment B foundation: an Attempt must stand in as @submission when serving a marketplace preview, but the shared submission jbuilders call course_user/current_points_awarded/ experience_points_record/publisher on the served object — the EXP/course-coupled slice the extension owns. Delegate those to the (optional) extension with allow_nil, so a preview (no extension) returns nil and a real submission returns the extension's values. Exercised only by previews; real submissions serve via the extension directly. Interface set audited against app/views/course/assessment/submission/submissions/*.jbuilder. --- app/models/course/assessment/attempt.rb | 7 +++++++ spec/models/course/assessment/attempt_spec.rb | 17 +++++++++++++++++ 2 files changed, 24 insertions(+) diff --git a/app/models/course/assessment/attempt.rb b/app/models/course/assessment/attempt.rb index 88878e5b509..061c194f60f 100644 --- a/app/models/course/assessment/attempt.rb +++ b/app/models/course/assessment/attempt.rb @@ -60,6 +60,13 @@ class Course::Assessment::Attempt < ApplicationRecord has_one :submission, class_name: 'Course::Assessment::Submission', inverse_of: :attempt, dependent: :destroy + # Course-coupled / EXP slice of the submission interface, owned by the extension. Delegating here lets + # an Attempt stand in as `@submission` for a marketplace preview: the shared submission + # jbuilders call these on the served object, and for a preview (no extension) they must be nil. For a + # real submission the serving path uses the extension directly, so these are exercised only by previews. + delegate :course_user, :current_points_awarded, :experience_points_record, :publisher, + to: :submission, allow_nil: true + has_many :submission_questions, class_name: 'Course::Assessment::SubmissionQuestion', foreign_key: 'submission_id', dependent: :destroy, inverse_of: :submission diff --git a/spec/models/course/assessment/attempt_spec.rb b/spec/models/course/assessment/attempt_spec.rb index 4d5af3b181d..82eaedf0779 100644 --- a/spec/models/course/assessment/attempt_spec.rb +++ b/spec/models/course/assessment/attempt_spec.rb @@ -41,6 +41,23 @@ end end + describe 'course-coupled interface delegation (lets an Attempt serve as @submission)' do + it 'returns nil for a preview attempt (no extension row)' do + attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + expect(attempt.course_user).to be_nil + expect(attempt.current_points_awarded).to be_nil + expect(attempt.experience_points_record).to be_nil + expect(attempt.publisher).to be_nil + end + + it 'delegates to the extension for a real submission' do + submission = create(:course_assessment_submission, assessment: assessment) + attempt = submission.attempt + expect(attempt.course_user).to eq(submission.course_user) + expect(attempt.experience_points_record).to eq(submission.experience_points_record) + end + end + describe '#reset_attempt!' do it 'destroys existing answers, returns to attempting, and rebuilds answers' do attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) From 4411182ca0de47417fc80d1b1aa40be8a45b14d8 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 18:08:01 +0800 Subject: [PATCH 03/11] feat(marketplace): courseless auto-grading service for preview attempts --- .../preview_auto_grading_service.rb | 20 +++++ .../preview_auto_grading_service_spec.rb | 90 +++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 app/services/course/assessment/marketplace/preview_auto_grading_service.rb create mode 100644 spec/services/course/assessment/marketplace/preview_auto_grading_service_spec.rb diff --git a/app/services/course/assessment/marketplace/preview_auto_grading_service.rb b/app/services/course/assessment/marketplace/preview_auto_grading_service.rb new file mode 100644 index 00000000000..fddb1fac7a6 --- /dev/null +++ b/app/services/course/assessment/marketplace/preview_auto_grading_service.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true +# Answer-centric grading for a marketplace preview Attempt. Mirrors the answer iteration of +# Course::Assessment::Submission::AutoGradingService but omits the EXP total + publish tail — a +# preview has no course_user or experience-points record to award, and grading it must not touch +# course state. +class Course::Assessment::Marketplace::PreviewAutoGradingService + class << self + def grade(attempt) + new.grade(attempt) + end + end + + def grade(attempt) + attempt.current_answers.each do |answer| + next if answer.attempting? + + answer.auto_grade!(reduce_priority: true) + end + end +end diff --git a/spec/services/course/assessment/marketplace/preview_auto_grading_service_spec.rb b/spec/services/course/assessment/marketplace/preview_auto_grading_service_spec.rb new file mode 100644 index 00000000000..342d2e8b41e --- /dev/null +++ b/spec/services/course/assessment/marketplace/preview_auto_grading_service_spec.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Marketplace::PreviewAutoGradingService do + let!(:instance) { Instance.default } + + with_tenant(:instance) do + let(:course) { create(:course) } + let(:assessment) { create(:assessment, :with_mcq_question, :autograded, course: course) } + let(:previewer) { create(:course_manager, course: course).user } + let(:other_previewer) { create(:course_manager, course: course).user } + let(:attempt) do + a = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + a.create_new_answers + # Finalise via the ATTEMPT event (not answer.finalise!): workflow persistence is deferred + # (lib/extensions/deferred_workflow_state_persistence), so a bare answer.finalise! is never + # written without an explicit save!. Attempt#finalise finalises current answers correctly. + a.finalise! + # The Workflow gem persists the new workflow_state (via persist_workflow_state) only AFTER the + # event's action callback (WorkflowEventConcern#finalise, which calls its own `save!`) returns — + # see workflow-3.1.1's `process_event!`. A real Submission#finalise! papers over this with its + # OWN trailing `save!`, autosave-cascaded onto `attempt` (submission.rb#253-257); a bare preview + # Attempt has no such wrapper, so `workflow_state` would otherwise never reach the DB. + a.save! + a + end + + with_active_job_queue_adapter(:test) do + it 'grades every answer WITHOUT publishing (no EXP/publish tail)' do + expect(attempt).to be_submitted + + described_class.grade(attempt) + + expect(attempt.answers.reload).to all(be_graded.or(be_evaluated)) + # The attempt itself is NOT advanced to graded/published — that tail is omitted. + expect(attempt.reload).to be_submitted + end + + it 'skips an answer left in the attempting state (never grades it)' do + # Different creator: the unique (assessment, creator) index forbids two attempts for previewer. + unfinalised = create(:course_assessment_attempt, assessment: assessment, creator: other_previewer) + unfinalised.create_new_answers + answer = unfinalised.answers.reload.first + expect(answer).to be_attempting + + expect { described_class.grade(unfinalised) }.not_to raise_error + + expect(answer.reload).to be_attempting + end + + it 'grades every current answer when there is more than one, not just the first' do + # Guards the `.each` loop: a service that graded only `current_answers.first` passes every + # single-question example above. `question_count: 2` is the assessment factory's transient. + multi = create(:assessment, :with_mcq_question, :autograded, course: course, question_count: 2) + multi_attempt = create(:course_assessment_attempt, assessment: multi, creator: other_previewer) + multi_attempt.create_new_answers + multi_attempt.finalise! + expect(multi_attempt.current_answers.size).to eq(2) + + described_class.grade(multi_attempt) + + expect(multi_attempt.answers.reload).to all(be_graded.or(be_evaluated)) + end + end + + context 'when the current answer grades asynchronously (programming)' do + let(:assessment) { create(:assessment, :with_programming_question, :autograded, course: course) } + + with_active_job_queue_adapter(:test) do + it 'enqueues a reduced-priority auto-grading job instead of grading inline' do + expect { described_class.grade(attempt) }. + to have_enqueued_job(Course::Assessment::Answer::ReducePriorityAutoGradingJob) + end + end + end + + context 'when a Codaveri programming question is present (paid grader runs, NOT skipped)' do + let(:assessment) { create(:assessment, :with_programming_question, :autograded, course: course) } + + before { assessment.questions.first.actable.update!(is_codaveri: true) } + + with_active_job_queue_adapter(:test) do + it 'still routes the answer through auto_grade! (no inert short-circuit)' do + expect { described_class.grade(attempt) }. + to have_enqueued_job(Course::Assessment::Answer::ReducePriorityAutoGradingJob) + end + end + end + end +end From f7a6376c8cdb10a28ec325837c7df065dfa2b903 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 18:57:49 +0800 Subject: [PATCH 04/11] fix(assessment): make auto-grading EXP tail extension-aware and preview-safe --- .../submission/auto_grading_service.rb | 15 ++++++- .../submission/auto_grading_service_spec.rb | 41 +++++++++++++++++++ 2 files changed, 54 insertions(+), 2 deletions(-) diff --git a/app/services/course/assessment/submission/auto_grading_service.rb b/app/services/course/assessment/submission/auto_grading_service.rb index 33ae2e82dca..9ba5a6dca3d 100644 --- a/app/services/course/assessment/submission/auto_grading_service.rb +++ b/app/services/course/assessment/submission/auto_grading_service.rb @@ -102,9 +102,20 @@ def unsubmit_answers(submission) end end + # Post-split, this service is driven by the Attempt base (the after_save trigger + AutoGradingJob pass + # the Attempt, not the Submission extension). Experience points live on the optional Submission + # extension: award them there for a real attempt, and for a preview (no extension) publish the graded + # work WITHOUT awarding EXP. The `is_a?(Submission)` branch keeps callers that pass a Submission + # directly (e.g. specs) working unchanged. def assign_exp_and_publish_grade(submission) - submission.points_awarded = Course::Assessment::Submission::CalculateExpService.calculate_exp(submission).to_i - submission.publish! + exp_record = submission.is_a?(Course::Assessment::Submission) ? submission : submission.submission + if exp_record + exp_record.points_awarded = + Course::Assessment::Submission::CalculateExpService.calculate_exp(exp_record).to_i + exp_record.publish! + else + submission.publish! + end end # Gets the ungraded answers for the given submission. diff --git a/spec/services/course/assessment/submission/auto_grading_service_spec.rb b/spec/services/course/assessment/submission/auto_grading_service_spec.rb index 1774361d126..68fe7390f3e 100644 --- a/spec/services/course/assessment/submission/auto_grading_service_spec.rb +++ b/spec/services/course/assessment/submission/auto_grading_service_spec.rb @@ -131,6 +131,47 @@ end end + # Post-split, production drives this service with the ATTEMPT base (the after_save trigger passes + # `self`), not the Submission extension. These examples pin that path — the one the existing + # `context 'when assessment is autograded'` masks by hand-passing a Submission. + describe '#grade driven by the Attempt base (production path)' do + let(:autograded_assessment) do + create(:assessment, :published_with_mcq_question, :autograded, course: course, question_count: 2) + end + + it 'awards EXP on the extension and publishes for a REAL autograded submission' do + submission = create(:submission, :attempting, assessment: autograded_assessment, creator: student_user) + # Suppress the async after_save re-trigger so we exercise grade() directly (mirrors the existing + # autograded context). The stub is on the Attempt base, where the callback lives. + allow(submission.attempt).to receive(:auto_grade_submission).and_return(true) + submission.finalise! + submission.save! + + expect { subject.grade(submission.attempt) }.not_to raise_error + + submission.reload + expect(submission).to be_published + expect(submission.points_awarded).not_to be_nil + end + + it 'grades and publishes a PREVIEW attempt (no extension) WITHOUT awarding EXP, no crash' do + previewer = create(:course_manager, course: course).user + preview = create(:course_assessment_attempt, assessment: autograded_assessment, creator: previewer) + allow(preview).to receive(:auto_grade_submission).and_return(true) + preview.create_new_answers + preview.finalise! + preview.save! + expect(preview.submission).to be_nil # it is a preview: no Submission extension row + + expect { subject.grade(preview) }.not_to raise_error + + preview.reload + expect(preview).to be_published + expect(preview.submission).to be_nil + expect(preview.current_answers.map(&:reload)).to all(be_graded.or(be_evaluated)) + end + end + context 'when a sub job fails' do let(:answer) do create(:course_assessment_answer_multiple_response, :submitted, From e560a2609f3aadd59d45a043e0d910802e2fb17a Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 19:08:37 +0800 Subject: [PATCH 05/11] refactor(assessment): extract shared _attempt jbuilder partial for preview reuse --- .../submissions/_attempt.json.jbuilder | 27 ++++++++++++++ .../submissions/_questions.json.jbuilder | 6 +++- .../submissions/_submission.json.jbuilder | 35 ++++++------------- 3 files changed, 42 insertions(+), 26 deletions(-) create mode 100644 app/views/course/assessment/submission/submissions/_attempt.json.jbuilder diff --git a/app/views/course/assessment/submission/submissions/_attempt.json.jbuilder b/app/views/course/assessment/submission/submissions/_attempt.json.jbuilder new file mode 100644 index 00000000000..facedd0f8d0 --- /dev/null +++ b/app/views/course/assessment/submission/submissions/_attempt.json.jbuilder @@ -0,0 +1,27 @@ +# frozen_string_literal: true +json.id attempt.id +json.canGrade can_grade +json.canUpdate can_update +json.isCreator current_user.id == attempt.creator_id +json.isStudent current_course_user&.student? || false + +if assessment.autograded? && !assessment.skippable? + question = attempt.questions.next_unanswered(attempt) + # If question does not exist, means the student have answered all questions + json.maxStep attempt.questions.index(question) if question +end + +# Show submission as submitted to students if grading is not published yet +apparent_workflow_state = if cannot?(:grade, attempt) && attempt.graded? + 'submitted' + else + attempt.workflow_state + end + +json.workflowState apparent_workflow_state +json.attemptedAt attempt.created_at&.iso8601 +json.submittedAt attempt.submitted_at&.iso8601 +json.maximumGrade attempt.questions.sum(:maximum_grade).to_f + +json.showPublicTestCasesOutput current_course.show_public_test_cases_output +json.showStdoutAndStderr current_course.show_stdout_and_stderr diff --git a/app/views/course/assessment/submission/submissions/_questions.json.jbuilder b/app/views/course/assessment/submission/submissions/_questions.json.jbuilder index 8ac11c7becd..1ee774d4d47 100644 --- a/app/views/course/assessment/submission/submissions/_questions.json.jbuilder +++ b/app/views/course/assessment/submission/submissions/_questions.json.jbuilder @@ -16,7 +16,11 @@ json.questions question_assessments.each_with_index.to_a do |(question_assessmen answer = answer_ids_hash[question.id] answer_id = answer&.id submission_question = sq_topic_ids_hash[question.id][0] - json.partial! 'question', question: question, can_grade: can_grade, answer: answer + # Fully-qualified: this partial is reused from a non-submissions controller (the marketplace preview + # attempts controller), whose view-path prefixes don't include this directory, so a bare + # 'question' reference would fail to resolve there. + json.partial! 'course/assessment/submission/submissions/question', question: question, can_grade: can_grade, + answer: answer json.questionNumber index + 1 json.questionTitle question.title diff --git a/app/views/course/assessment/submission/submissions/_submission.json.jbuilder b/app/views/course/assessment/submission/submissions/_submission.json.jbuilder index b516f743940..2388f49e89a 100644 --- a/app/views/course/assessment/submission/submissions/_submission.json.jbuilder +++ b/app/views/course/assessment/submission/submissions/_submission.json.jbuilder @@ -1,25 +1,8 @@ # frozen_string_literal: true json.submission do - json.id submission.id - json.canGrade can_grade - json.canUpdate can_update - json.isCreator current_user.id == submission.creator_id - json.isStudent current_course_user&.student? || false + json.partial! 'course/assessment/submission/submissions/attempt', attempt: submission, assessment: assessment, + can_grade: can_grade, can_update: can_update - if assessment.autograded? && !assessment.skippable? - question = submission.questions.next_unanswered(submission) - # If question does not exist, means the student have answered all questions - json.maxStep submission.questions.index(question) if question - end - - # Show submission as submitted to students if grading is not published yet - apparent_workflow_state = if cannot?(:grade, submission) && submission.graded? - 'submitted' - else - submission.workflow_state - end - - json.workflowState apparent_workflow_state json.submitter do json.name display_course_user(submission.course_user) json.id submission.course_user.id @@ -30,8 +13,14 @@ json.submission do bonus_end_at = assessment.time_for(submitter_course_user).bonus_end_at json.bonusEndAt bonus_end_at&.iso8601 json.dueAt end_at&.iso8601 - json.attemptedAt submission.created_at&.iso8601 - json.submittedAt submission.submitted_at&.iso8601 + + # Show submission as submitted to students if grading is not published yet + apparent_workflow_state = if cannot?(:grade, submission) && submission.graded? + 'submitted' + else + submission.workflow_state + end + if ['graded', 'published'].include? apparent_workflow_state # Display the published time first, else show the graded time if available. # For showing timestamps from delayed grade publication. @@ -45,10 +34,6 @@ json.submission do end json.grade submission.grade.to_f end - json.maximumGrade submission.questions.sum(:maximum_grade).to_f - - json.showPublicTestCasesOutput current_course.show_public_test_cases_output - json.showStdoutAndStderr current_course.show_stdout_and_stderr json.late end_at && submission.submitted_at && submission.submitted_at.iso8601 > end_at From e17df0ba518fc0f49c62e96997eac7d97ef40ccd Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 19:19:36 +0800 Subject: [PATCH 06/11] feat(marketplace): abilities for preview attempt serving and grading --- ...ssessment_marketplace_ability_component.rb | 25 ++++- ...ment_marketplace_ability_component_spec.rb | 94 +++++++++++++++++++ 2 files changed, 118 insertions(+), 1 deletion(-) create mode 100644 spec/models/course/assessment_marketplace_ability_component_spec.rb diff --git a/app/models/components/course/assessment_marketplace_ability_component.rb b/app/models/components/course/assessment_marketplace_ability_component.rb index 45dc1e7922b..f5acc97a52e 100644 --- a/app/models/components/course/assessment_marketplace_ability_component.rb +++ b/app/models/components/course/assessment_marketplace_ability_component.rb @@ -48,7 +48,7 @@ def allow_admins_publish_to_marketplace can :publish_to_marketplace, Course::Assessment end - def allow_managers_access_marketplace + def allow_managers_access_marketplace # rubocop:disable Metrics/AbcSize can :access_marketplace, Course, id: course.id can :duplicate_from_marketplace, Course::Assessment do |assessment| assessment.marketplace_listing&.published? || false @@ -56,5 +56,28 @@ def allow_managers_access_marketplace can :preview_in_marketplace, Course::Assessment do |assessment| assessment.marketplace_listing&.published? || false end + + # Marketplace preview serving. Subject is Course::Assessment::Attempt (not a separate + # model). Block conditions gate on `preview?` (no extension row) + creator ownership, so real + # attempts — served via Submission and gated by AssessmentAbility — are never over-granted here. + # + # `:create` is class-level (CanCan can't evaluate a block without an instance); the published-listing + # scoping of creation lives in PreviewAttemptsController#authorize_attempt!, not here. It cannot + # over-authorize real submissions: those authorize `:attempt` on the assessment, never `:create` on + # Attempt. + can :create, Course::Assessment::Attempt + can [:read, :update, :grade, :reset, :reload_answer, :reevaluate_answer, :generate_feedback, + :generate_live_feedback, :create_live_feedback_chat, :fetch_live_feedback_chat, + :fetch_live_feedback_status, :save_live_feedback, :create_scribing_scribble, :read_tests], + Course::Assessment::Attempt do |attempt| + attempt.preview? && attempt.creator_id == user.id + end + # `Course::Assessment::Answer::UpdateAnswerConcern` gates answer-editing/grading params behind + # `can?(:update/:grade, answer)` on the ANSWER — without this, PATCH .../attempt/:id would silently + # drop every answer param for a preview-owned answer. Block form (not a hash) because a preview is the + # absence of an extension; `answer.submission` is the Attempt. + can [:update, :grade], Course::Assessment::Answer do |answer| + answer.submission.preview? && answer.submission.creator_id == user.id + end end end \ No newline at end of file diff --git a/spec/models/course/assessment_marketplace_ability_component_spec.rb b/spec/models/course/assessment_marketplace_ability_component_spec.rb new file mode 100644 index 00000000000..eb2c7f6cf23 --- /dev/null +++ b/spec/models/course/assessment_marketplace_ability_component_spec.rb @@ -0,0 +1,94 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::AssessmentMarketplaceAbilityComponent do + let!(:instance) { Instance.default } + with_tenant(:instance) do + let(:course) { create(:course) } + let(:manager) { create(:course_manager, course: course).user } + let(:other_user) { create(:user) } + let(:source_assessment) { create(:assessment, :with_mcq_question) } + let(:own_preview) do + create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + end + + before { create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: manager) } + + subject(:ability) { Ability.new(manager, course, course.course_users.find_by(user: manager)) } + + it 'lets a manager create and act on their own preview attempt (every granted verb)' do + expect(ability).to be_able_to(:create, Course::Assessment::Attempt.new(creator: manager)) + # Assert the FULL verb array so dropping any single entry from the `can [...]` list fails here. + %i[read update grade reset reload_answer reevaluate_answer generate_feedback + generate_live_feedback create_live_feedback_chat fetch_live_feedback_chat + fetch_live_feedback_status save_live_feedback create_scribing_scribble read_tests].each do |verb| + expect(ability).to be_able_to(verb, own_preview) + end + end + + it 'lets a manager update and grade answers belonging to their own preview attempt' do + own_preview.create_new_answers + answer = own_preview.answers.reload.first + expect(ability).to be_able_to(:update, answer) + expect(ability).to be_able_to(:grade, answer) + end + + it "denies acting on another user's preview attempt answers" do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: other_user) + others = create(:course_assessment_attempt, assessment: source_assessment, creator: other_user) + others.create_new_answers + other_answer = others.answers.reload.first + expect(ability).not_to be_able_to(:update, other_answer) + expect(ability).not_to be_able_to(:grade, other_answer) + end + + # The rule is one `can [...verbs...] { creator scoping }` array. Denying only :grade would let a + # future edit silently drop creator scoping from another verb — deny a representative spread. + it "denies acting on another user's preview attempt" do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: other_user) + others = create(:course_assessment_attempt, assessment: source_assessment, creator: other_user) + expect(ability).not_to be_able_to(:read, others) + expect(ability).not_to be_able_to(:update, others) + expect(ability).not_to be_able_to(:grade, others) + expect(ability).not_to be_able_to(:reset, others) + expect(ability).not_to be_able_to(:reevaluate_answer, others) + expect(ability).not_to be_able_to(:read_tests, others) + end + + # preview? scoping: a REAL submission the manager created (not a preview) must not receive the + # preview-only verbs from THIS component. `:reset` is preview-only (no other component grants it), + # so it is the clean proof the block's `preview?` guard is load-bearing — not just creator scoping. + it "denies preview-only verbs on the manager's own real (non-preview) submission" do + real_submission = create(:submission, creator: manager) + expect(real_submission.attempt.preview?).to be(false) + expect(ability).not_to be_able_to(:reset, real_submission.attempt) + end + + # Answer-rule analog of the above: the Answer block also gates on `answer.submission.preview?`. + # Vary preview-vs-real (not just creator), so dropping `preview?` from the Answer block fails here. + # Assert `:grade` ONLY — NOT `:update`: `AssessmentAbility#allow_update_own_assessment_answer` + # (assessment_ability.rb:89) grants `:update` course-unscoped on any *attempting* answer whose + # submission is the user's own, so `:update` is masked here and is not a clean discriminator. + # `:grade` is only ever granted by THIS component's preview rule, so it is the valid preview? proof. + it "denies grading a real submission's answers even when the manager is its creator" do + real_submission = create(:submission, creator: manager) + real_submission.attempt.create_new_answers + real_answer = real_submission.attempt.answers.reload.first + expect(real_answer.submission.preview?).to be(false) + expect(ability).not_to be_able_to(:grade, real_answer) + end + + it 'denies a non-baseline-capable user (nil course_user) entirely' do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: other_user) + plain = Ability.new(other_user, course, nil) + expect(plain).not_to be_able_to(:create, Course::Assessment::Attempt.new(creator: other_user)) + end + + it 'denies create to a student (real course_user, manager_or_owner? false)' do + student = create(:course_student, course: course).user + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: student) + student_ability = Ability.new(student, course, course.course_users.find_by(user: student)) + expect(student_ability).not_to be_able_to(:create, Course::Assessment::Attempt.new(creator: student)) + end + end +end From 57687face6ba7131a9db7927686833a909972041 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 19:47:18 +0800 Subject: [PATCH 07/11] feat(marketplace): preview attempt serving controller, update service, jbuilders Increment B: PreviewAttemptsController serves marketplace preview attempts by reusing the platform submission edit/update pipeline, treating a bare Course::Assessment::Attempt (no Submission extension) as @submission. Covers create/edit/update/auto_grade/reset (core routes); live-feedback/scribing actions are authored here (single-file authoring) but wired up by a later task. PreviewUpdateService subclasses Submission::UpdateService to drop the EXP-only draft_points_awarded/points_awarded params an Attempt has no columns for, and to build SubmissionQuestion rows against the Attempt directly (the parent assumes a Submission extension with its own #attempt association). Also covers a repo-wide deferred-workflow-state-persistence gotcha in the new spec: a bare `attempt.finalise!` only transitions workflow_state in memory (lib/extensions/ deferred_workflow_state_persistence) and needs an explicit save to be visible to a subsequent reload/re-find, same as Submission#finalise! already does internally. --- .../assessment/marketplace/controller.rb | 5 + .../preview_attempts_controller.rb | 370 ++++++++++++++++++ app/models/course/assessment/attempt.rb | 11 + .../marketplace/preview_update_service.rb | 13 + .../_preview_attempt.json.jbuilder | 30 ++ .../preview_attempts/edit.json.jbuilder | 51 +++ config/routes.rb | 7 + .../preview_attempts_controller_spec.rb | 233 +++++++++++ spec/models/course/assessment/attempt_spec.rb | 7 + 9 files changed, 727 insertions(+) create mode 100644 app/controllers/course/assessment/marketplace/preview_attempts_controller.rb create mode 100644 app/services/course/assessment/marketplace/preview_update_service.rb create mode 100644 app/views/course/assessment/marketplace/preview_attempts/_preview_attempt.json.jbuilder create mode 100644 app/views/course/assessment/marketplace/preview_attempts/edit.json.jbuilder create mode 100644 spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb diff --git a/app/controllers/course/assessment/marketplace/controller.rb b/app/controllers/course/assessment/marketplace/controller.rb index 489a3ec7cd8..f2874e1bbb7 100644 --- a/app/controllers/course/assessment/marketplace/controller.rb +++ b/app/controllers/course/assessment/marketplace/controller.rb @@ -4,6 +4,11 @@ class Course::Assessment::Marketplace::Controller < Course::ComponentController # preview views reuse it, but Rails only auto-includes a controller's own matching helper. helper Course::Assessment::AssessmentsHelper + # last_attempt (used by reused answer partials, e.g. _multiple_response.json.jbuilder) is defined in + # Course::Assessment::Submission::SubmissionsHelper; Rails only auto-includes a controller's own + # matching helper, so the preview attempt views need it declared here. + helper Course::Assessment::Submission::SubmissionsHelper + private def component diff --git a/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb new file mode 100644 index 00000000000..4eadd4a09a4 --- /dev/null +++ b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb @@ -0,0 +1,370 @@ +# frozen_string_literal: true +class Course::Assessment::Marketplace::PreviewAttemptsController < # rubocop:disable Metrics/ClassLength + Course::Assessment::Marketplace::Controller + include Course::Assessment::LiveFeedback::MessageConcern + include Course::Assessment::LiveFeedback::ThreadConcern + include Course::Assessment::Submission::SubmissionsControllerServiceConcern + + MEMBER_ACTIONS = [ + :edit, :update, :auto_grade, :reset, :reload_answer, :reevaluate_answer, :generate_feedback, + :generate_live_feedback, :create_live_feedback_chat, :create_scribing_scribble + ].freeze + + before_action :load_listing, only: [:create] + before_action :load_attempt, only: MEMBER_ACTIONS + before_action :authorize_attempt!, only: [:create, *MEMBER_ACTIONS] + # Questions may be added to the source assessment after an attempt already exists; without this, + # newly added questions have no submission_question row and _questions.json.jbuilder (reused) raises. + # Mirrors submissions_controller. auto_grade also renders 'edit' directly, so it needs the same guard. + before_action :load_or_create_submission_questions, + only: [:edit, :update, :auto_grade, :reset, :reload_answer, :reevaluate_answer, + :generate_feedback, :generate_live_feedback, :create_live_feedback_chat, + :create_scribing_scribble] + + # Defines #update: service.update → answer saves + workflow transitions + render 'edit'. @assessment/ + # @submission are snapshot at service memoization, hence set in load_attempt, never in the action. + delegate_to_service(:update) + delegate_to_service(:load_or_create_submission_questions) + + def create + attempt = existing_attempt || build_attempt + attempt.create_new_answers + render json: { id: attempt.id, assessmentId: attempt.assessment_id } + rescue ActiveRecord::RecordInvalid => e + # The previewer already has an attempt for this assessment (e.g. a real submission because they are + # also a student in the source course). validate_unique_submission (unscoped) fails the create with + # the friendly collision error; never reuse their real submission as a preview. + render json: { errors: e.record.errors.full_messages }, status: :conflict + end + + def edit + # implicit render of preview_attempts/edit.json.jbuilder + end + + def auto_grade + Course::Assessment::Marketplace::PreviewAutoGradingService.grade(@attempt) + # Re-load (NOT @attempt.reload): reload drops the calculated `grade` select that edit.json needs. + load_attempt + render 'edit' + end + + def reset + @attempt.reset_attempt! + load_attempt + render 'edit' + end + + def reload_answer + @answer = @submission.answers.find_by(id: reload_answer_params[:answer_id]) + + if @answer.nil? + head :bad_request + elsif reload_answer_params[:reset_answer] + @new_answer = @answer.reset_answer + render @new_answer + else + render @answer + end + end + + def reevaluate_answer + @answer = @submission.answers.find_by(id: reload_answer_params[:answer_id]) + return head :bad_request if @answer.nil? + + job = @answer.auto_grade!(redirect_to_path: nil, reduce_priority: true) + if job.nil? + @answer.reload + render @answer + else + render partial: 'jobs/submitted', locals: { job: job.job } + end + end + + def generate_feedback + @answer = @submission.answers.find_by(id: reload_answer_params[:answer_id]) + return head :bad_request if @answer.nil? + + job = @answer.generate_feedback + render partial: 'jobs/submitted', locals: { job: job } + end + + def generate_live_feedback # rubocop:disable Metrics/AbcSize, Metrics/MethodLength + @answer = @submission.answers.find_by(id: live_feedback_params[:answer_id]) + return head :bad_request if @answer.nil? + + system_thread = active_live_feedback_thread + return head :bad_request if system_thread.nil? + + @thread_id = system_thread.codaveri_thread_id + + user_messages_count = system_thread.messages. + where(creator_id: system_thread.submission_creator_id).count + if current_course.codaveri_get_help_usage_limited? && + user_messages_count >= current_course.codaveri_max_get_help_user_messages + head :too_many_requests and return + end + + @message = live_feedback_params[:message] + @options = live_feedback_params[:options] + @option_id = live_feedback_params[:option_id] + + handle_save_user_message + + status, response = @answer.generate_live_feedback(@thread_id, @message) + + render json: response, status: status + end + + def create_live_feedback_chat + @answer = @submission.answers.find_by(id: answer_params[:answer_id]) + return head :bad_request if @answer.nil? + + status, body = safe_create_and_save_thread_info + + @thread_id = body['thread']['id'] + @thread_status = body['thread']['status'] + + render 'course/assessment/submission/submissions/create_live_feedback_chat', status: status + end + + def fetch_live_feedback_chat + @answer_id = live_feedback_params[:answer_id] + answer = Course::Assessment::Answer.find(@answer_id) + authorize_preview_attempt_for_answer!(answer, :fetch_live_feedback_chat) + + @thread = live_feedback_thread_for_answer(answer) + return head :bad_request if @thread.nil? + + render 'course/assessment/submission/submissions/fetch_live_feedback_chat' + end + + def fetch_live_feedback_status + thread_id = thread_params[:thread_id] + codaveri_api_service = CodaveriAsyncApiService.new("chat/feedback/threads/#{thread_id}", nil) + + response_status, response_body = codaveri_api_service.get + + raise CodaveriError, { status: response_status, body: response_body } if response_status != 200 + + @thread_status = response_body['data']['thread']['status'] + + @thread = Course::Assessment::LiveFeedback::Thread.find_by(codaveri_thread_id: thread_id) + return head :bad_request if @thread.nil? + + authorize_preview_attempt_for_thread!(@thread, :fetch_live_feedback_status) + @thread.update!(is_active: @thread_status == 'active') + + render 'course/assessment/submission/submissions/fetch_live_feedback_status', status: response_status + end + + def save_live_feedback + current_thread_id = params[:current_thread_id] + content = params[:content] + is_error = params[:is_error] + + @thread = Course::Assessment::LiveFeedback::Thread.find_by(codaveri_thread_id: current_thread_id) + return head :bad_request if @thread.nil? + + authorize_preview_attempt_for_thread!(@thread, :save_live_feedback) + + @thread.class.transaction do + @new_message = save_new_feedback(content, is_error) + + associate_new_message_with_existing_files + end + end + + def create_scribing_scribble + @scribing_answer = preview_scribing_answer + return head :bad_request unless @scribing_answer + return head :bad_request if scribing_scribble_params[:answer_id].to_i != @scribing_answer.id + + @scribble = Course::Assessment::Answer::ScribingScribble. + find_or_initialize_by(creator: current_user, answer_id: @scribing_answer.id) + @scribble.assign_attributes(scribing_scribble_params) + @scribble.save + + render json: @scribing_answer + end + + # The action delegated by delegate_to_service cannot authorize inline — gate everything here. + MEMBER_ACTION_ABILITIES = { + 'edit' => :read, + 'update' => :update, + 'auto_grade' => :grade, + 'reset' => :reset, + 'reload_answer' => :reload_answer, + 'reevaluate_answer' => :reevaluate_answer, + 'generate_feedback' => :generate_feedback, + 'generate_live_feedback' => :generate_live_feedback, + 'create_live_feedback_chat' => :create_live_feedback_chat, + 'create_scribing_scribble' => :create_scribing_scribble + }.freeze + + private + + def authorize_attempt! + if action_name == 'create' + authorize! :create, Course::Assessment::Attempt + # Published-listing / course gate: creation is only for a published marketplace listing's source + # assessment (the ability's :create grant is class-level and course-agnostic on its own). + authorize! :preview_in_marketplace, source_assessment + elsif MEMBER_ACTION_ABILITIES.key?(action_name) + authorize! MEMBER_ACTION_ABILITIES.fetch(action_name), @attempt + else + authorize! action_name.to_sym, @attempt + end + end + + # Swap the platform UpdateService for the preview subclass (the service concern reads this). + def service_class + Course::Assessment::Marketplace::PreviewUpdateService + end + + def load_listing + @listing = Course::Assessment::Marketplace::Listing.find(params[:listing_id]) + end + + def source_assessment + @source_assessment ||= @listing.assessment + end + + # Key on previews only, so a real submission for the same (assessment, creator) is never mistaken for + # a reusable preview. + def existing_attempt + Course::Assessment::Attempt.previews.find_by(assessment: source_assessment, creator: current_user) + end + + def build_attempt + User.with_stamper(current_user) do + Course::Assessment::Attempt.create!( + assessment: source_assessment, creator: current_user, updater: current_user + ) + end + end + + # Member routes are shallow (no listing_id): the attempt resolves its own assessment. Load with the + # calculated grade/graded_at/log_count/grader_ids the reused edit payload reads. + def load_attempt + @attempt = Course::Assessment::Attempt.all. + calculated(:grade, :graded_at, :log_count, :grader_ids).find(params[:id]) + @submission = @attempt + @assessment = @attempt.assessment + end + + def reload_answer_params + params.permit(:answer_id, :reset_answer) + end + + def answer_params + params.permit(:answer_id) + end + + def live_feedback_params + params.permit(:message, :answer_id, :option_id, options: []) + end + + def thread_params + params.permit(:thread_id) + end + + def scribing_scribble_params + params.require(:scribble).permit(:answer_id, :content) + end + + def preview_scribing_answer + answer = @submission.answers.find_by(id: params[:answer_id]) + return unless answer&.actable.is_a?(Course::Assessment::Answer::Scribing) + + answer.actable + end + + # Design C: submission_questions reference the attempt via submission_id (no polymorphic + # attemptable_type), so the thread lookups filter on submission_id alone. + def active_live_feedback_thread + Course::Assessment::LiveFeedback::Thread. + joins(:submission_question). + where( + submission_question: { + submission_id: @submission.id, + question_id: @answer.question.id + }, + is_active: true + ). + first + end + + def safe_create_and_save_thread_info + submission_question = Course::Assessment::SubmissionQuestion.where( + submission_id: @submission.id, + question_id: @answer.question.id + ).first + + submission_question.with_lock do + existing_active_threads = Course::Assessment::LiveFeedback::Thread. + where(submission_question_id: submission_question.id, is_active: true) + + return existing_thread_status(existing_active_threads.first) unless existing_active_threads.empty? + + create_and_save_thread_if_empty(submission_question) + end + end + + def live_feedback_thread_for_answer(answer) + submission = answer.submission + question = answer.question + + submission_question = Course::Assessment::SubmissionQuestion. + where(submission_id: submission.id, question_id: question.id).first + + Course::Assessment::LiveFeedback::Thread.where(submission_question_id: submission_question.id). + order(created_at: :desc).preload(:messages).first + end + + def authorize_preview_attempt_for_answer!(answer, action) + attempt = answer.submission + raise CanCan::AccessDenied unless attempt.preview? + + authorize! action, attempt + end + + def authorize_preview_attempt_for_thread!(thread, action) + attempt = thread.submission_question.submission + raise CanCan::AccessDenied unless attempt.preview? + + authorize! action, attempt + end + + def save_new_feedback(content, is_error) + new_message = Course::Assessment::LiveFeedback::Message.create({ + thread_id: @thread.id, + is_error: is_error, + content: content, + creator_id: 0, + created_at: Time.zone.now, + option_id: nil + }) + + raise ActiveRecord::Rollback unless new_message.persisted? + + new_message + end + + def associate_new_message_with_existing_files + last_message = @thread.messages.where.not(id: @new_message.id).max_by(&:id) + return [] if last_message.nil? + + file_ids = Course::Assessment::LiveFeedback::MessageFile. + where(message_id: last_message.id).pluck(:file_id) + + new_message_files = file_ids.map do |file_id| + { + message_id: @new_message.id, + file_id: file_id + } + end + + files = Course::Assessment::LiveFeedback::MessageFile.insert_all(new_message_files) + raise ActiveRecord::Rollback if !new_message_files.empty? && (files.nil? || files.rows.empty?) + end +end diff --git a/app/models/course/assessment/attempt.rb b/app/models/course/assessment/attempt.rb index 061c194f60f..ece2feb7783 100644 --- a/app/models/course/assessment/attempt.rb +++ b/app/models/course/assessment/attempt.rb @@ -67,6 +67,17 @@ class Course::Assessment::Attempt < ApplicationRecord delegate :course_user, :current_points_awarded, :experience_points_record, :publisher, to: :submission, allow_nil: true + # A preview's `@submission` IS this base Attempt, but the shared submission pipeline sometimes reaches + # for `.attempt` (e.g. Submission::UpdateService#create_missing_submission_questions builds + # `SubmissionQuestion.new(submission: @submission.attempt)`). For a real Submission that returns the base + # Attempt; for an Attempt standing in as @submission it is simply itself. Returning `self` lets that + # shared code work unmodified for both kinds, extending the "Attempt serves as @submission" + # seam. `SubmissionQuestion#submission` / `Answer#submission` both target Attempt, so `self` is the + # correct object to hand them. + def attempt + self + end + has_many :submission_questions, class_name: 'Course::Assessment::SubmissionQuestion', foreign_key: 'submission_id', dependent: :destroy, inverse_of: :submission diff --git a/app/services/course/assessment/marketplace/preview_update_service.rb b/app/services/course/assessment/marketplace/preview_update_service.rb new file mode 100644 index 00000000000..2f687e2c1ac --- /dev/null +++ b/app/services/course/assessment/marketplace/preview_update_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true +# Reuses the platform submission update path (answer saves, workflow transitions via the param-style +# event writers) for a marketplace preview Attempt. Overrides ONLY the submission-params reader: the +# parent permits points_awarded/draft_points_awarded, and the grader UI sends draft_points_awarded on +# mark/publish — an EXP column an Attempt does not have. Dropping the param here lets the same frontend +# payload drive both attempt kinds. +class Course::Assessment::Marketplace::PreviewUpdateService < Course::Assessment::Submission::UpdateService + protected + + def update_submission_params + params.require(:submission).permit(*workflow_state_params) + end +end diff --git a/app/views/course/assessment/marketplace/preview_attempts/_preview_attempt.json.jbuilder b/app/views/course/assessment/marketplace/preview_attempts/_preview_attempt.json.jbuilder new file mode 100644 index 00000000000..c4e80ae7131 --- /dev/null +++ b/app/views/course/assessment/marketplace/preview_attempts/_preview_attempt.json.jbuilder @@ -0,0 +1,30 @@ +# frozen_string_literal: true +json.submission do + json.partial! 'course/assessment/submission/submissions/attempt', attempt: preview_attempt, assessment: assessment, + can_grade: can_grade, can_update: can_update + + json.submitter do + json.name display_user(preview_attempt.creator) + json.id nil + end + + json.bonusEndAt nil + json.dueAt nil + + if ['graded', 'published'].include? preview_attempt.workflow_state + json.gradedAt preview_attempt.published_at&.iso8601 + if preview_attempt.workflow_state == 'published' + json.grader do + json.name display_user(preview_attempt.creator) + json.id nil + end + end + json.grade preview_attempt.grade.to_f + end + + json.late nil + + json.basePoints assessment.base_exp + json.bonusPoints assessment.time_bonus_exp + json.pointsAwarded preview_attempt.current_points_awarded +end diff --git a/app/views/course/assessment/marketplace/preview_attempts/edit.json.jbuilder b/app/views/course/assessment/marketplace/preview_attempts/edit.json.jbuilder new file mode 100644 index 00000000000..76bf4daaca8 --- /dev/null +++ b/app/views/course/assessment/marketplace/preview_attempts/edit.json.jbuilder @@ -0,0 +1,51 @@ +# frozen_string_literal: true +can_grade = can?(:grade, @submission) +can_update = can?(:update, @submission) + +json.partial! 'course/assessment/marketplace/preview_attempts/preview_attempt', preview_attempt: @submission, + assessment: @assessment, + can_grade: can_grade, + can_update: can_update + +json.assessment do + json.categoryId @assessment.tab.category_id + json.tabId @assessment.tab_id + json.(@assessment, :title, :description, :autograded, :skippable) + json.showMcqMrqSolution @assessment.show_mcq_mrq_solution + json.showRubricToStudents @assessment.show_rubric_to_students + json.timeLimit @assessment.time_limit + json.delayedGradePublication @assessment.delayed_grade_publication + json.tabbedView @assessment.tabbed_view || @assessment.autograded + json.showPrivate @assessment.show_private + json.allowPartialSubmission @assessment.allow_partial_submission + # If submitting with incorrect answers is not allowed, we must show the answer to students regardless + json.showMcqAnswer !@assessment.allow_partial_submission || @assessment.show_mcq_answer + json.showEvaluation @assessment.show_evaluation + json.blockStudentViewingAfterSubmitted @assessment.block_student_viewing_after_submitted + json.questionIds @submission.questions.pluck(:id) + json.passwordProtected @assessment.session_password_protected? + json.gamified @assessment.course.gamified? + json.isKoditsuEnabled current_course.component_enabled?(Course::KoditsuPlatformComponent) && + @assessment.is_koditsu_enabled && + @assessment.koditsu_assessment_id + json.files @assessment.folder.materials do |material| + json.url url_to_material(@assessment.course, @assessment.folder, material) + json.name format_inline_text(material.name) + end + json.isCodaveriEnabled current_course.component_enabled?(Course::CodaveriComponent) +end + +current_answer_ids = @submission.current_answers.pluck(:id) +answers = @submission.answers.where(id: current_answer_ids).includes(:actable, { question: { actable: :files } }) +submission_questions = @submission.submission_questions. + where(question: @submission.questions).includes({ discussion_topic: :posts }) + +json.partial! 'course/assessment/submission/submissions/questions', assessment: @assessment, submission: @submission, + can_grade: can_grade, + submission_questions: submission_questions, + answers: answers +json.partial! 'course/assessment/submission/submissions/answers', submission: @submission, answers: answers +json.partial! 'course/assessment/submission/submissions/topics', submission: @submission, + submission_questions: submission_questions, + can_grade: can_grade +json.partial! 'course/assessment/submission/submissions/history', submission: @submission diff --git a/config/routes.rb b/config/routes.rb index e40eeb54d47..53c337952b4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -626,6 +626,13 @@ resources :listings, only: [:show], path: 'marketplace/listings' do post 'duplicate', on: :collection resources :questions, only: [:show] + resource :attempt, only: [:create], controller: 'preview_attempts' + end + resources :preview_attempts, only: [:edit, :update], path: 'marketplace/attempt' do + member do + post :auto_grade + post :reset + end end end end diff --git a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb new file mode 100644 index 00000000000..4854f006bc7 --- /dev/null +++ b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb @@ -0,0 +1,233 @@ +# frozen_string_literal: true +require 'rails_helper' + +RSpec.describe Course::Assessment::Marketplace::PreviewAttemptsController, type: :controller do + let!(:instance) { Instance.default } + with_tenant(:instance) do + render_views + let(:course) { create(:course) } + let(:manager) { create(:course_manager, course: course).user } + let(:other_manager) { create(:course_manager, course: course).user } + let(:source_assessment) { create(:assessment, :published, :with_mcq_question) } + let!(:listing) do + create(:course_assessment_marketplace_listing, assessment: source_assessment) + end + + before do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: manager) + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: other_manager) + controller_sign_in(controller, manager) + end + + describe 'POST #create' do + subject { post :create, params: { course_id: course.id, listing_id: listing.id, format: :json } } + + it 'creates a preview attempt owned by the previewer and returns id + assessmentId + answers' do + expect { subject }.to change { Course::Assessment::Attempt.previews.count }.by(1) + body = response.parsed_body + expect(body['id']).to be_present + expect(body['assessmentId']).to eq(source_assessment.id) + attempt = Course::Assessment::Attempt.find(body['id']) + expect(attempt.creator).to eq(manager) + expect(attempt.assessment).to eq(source_assessment) + expect(attempt.answers).not_to be_empty + end + + it 'resumes the existing preview (same id), not a second row' do + subject + original_id = response.parsed_body['id'] + expect { post :create, params: { course_id: course.id, listing_id: listing.id, format: :json } }. + not_to(change { Course::Assessment::Attempt.previews.count }) + expect(response.parsed_body['id']).to eq(original_id) + end + + it 'gives a different manager their own separate preview on the same listing' do + subject + first_id = response.parsed_body['id'] + controller_sign_in(controller, other_manager) + expect { post :create, params: { course_id: course.id, listing_id: listing.id, format: :json } }. + to change { Course::Assessment::Attempt.previews.count }.by(1) + expect(response.parsed_body['id']).not_to eq(first_id) + end + + context 'when the listing is not published' do + let(:unpublished_assessment) { create(:assessment, :with_mcq_question) } + let!(:unpublished_listing) do + create(:course_assessment_marketplace_listing, assessment: unpublished_assessment, published: false) + end + it 'is denied (published-listing gate)' do + expect do + post :create, params: { course_id: course.id, listing_id: unpublished_listing.id, format: :json } + end.to raise_exception(CanCan::AccessDenied) + end + end + + context 'when the previewer already has a real submission for the assessment' do + before do + create(:course_student, course: source_assessment.course, user: manager) + create(:submission, assessment: source_assessment, creator: manager) + end + it 'returns 409 without reusing the real submission or creating a preview' do + expect { subject }.not_to(change { Course::Assessment::Attempt.previews.count }) + expect(response).to have_http_status(:conflict) + end + end + + context 'when the requester is not a manager' do + let(:student) { create(:course_student, course: course).user } + before do + create(:course_assessment_marketplace_allowlist_rule, rule_type: :user, user: student) + controller_sign_in(controller, student) + end + it 'is denied' do + expect { subject }.to raise_exception(CanCan::AccessDenied) + end + end + end + + describe 'GET #edit' do + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a.answers.reload + a + end + subject { get :edit, params: { course_id: course.id, id: attempt.id, format: :json } } + + it 'renders the reused submission edit payload for the preview attempt' do + subject + expect(response).to have_http_status(:ok) + expect(response.parsed_body['submission']['id']).to eq(attempt.id) + expect(response.parsed_body['submission']['submitter']).to have_key('name') + # preview submitter has no course_user id + expect(response.parsed_body['submission']['submitter']['id']).to be_nil + expect(response.parsed_body['questions']).not_to be_empty + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied (proves the creator scoping on :read)' do + expect { subject }.to raise_exception(CanCan::AccessDenied) + end + end + end + + describe 'POST #auto_grade' do + let(:source_assessment) { create(:assessment, :published, :with_mcq_question, :autograded) } + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + # Must finalise first: the grading service SKIPS attempting answers, so without this an inert + # `grade` would still pass a "renders 200" test. Finalise via the ATTEMPT event (deferred + # workflow persistence — a bare answer.finalise! never hits the DB). + a.finalise! + a + end + subject { post :auto_grade, params: { course_id: course.id, id: attempt.id, format: :json } } + + it 'grades the attempt for the owner (the answer actually transitions state)' do + answer = attempt.current_answers.first + expect(answer.workflow_state).to eq('submitted') + subject + expect(response).to have_http_status(:ok) + # Autograded assessment: Answer::AutoGradingService#evaluate publishes straight through to + # 'graded' (not just 'evaluated') once evaluate! finds the assessment autograded. + expect(answer.reload.workflow_state).to eq('graded') + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + expect { subject }.to raise_exception(CanCan::AccessDenied) + end + end + end + + describe 'POST #reset' do + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + # `finalise!` alone only transitions workflow_state in memory (the app-wide deferred workflow + # persistence adapter, lib/extensions/deferred_workflow_state_persistence) — an explicit `save!` + # is needed to persist it, mirroring how Submission#finalise! chains `attempt.finalise!; save!`. + a.finalise! + a.save! + a + end + subject { post :reset, params: { course_id: course.id, id: attempt.id, format: :json } } + + it 'returns the attempt to a fresh attempting state with new answers, reusing the same row' do + original_answer_ids = attempt.current_answers.map(&:id) + expect(attempt.reload.workflow_state).to eq('submitted') + + expect { subject }.not_to(change { Course::Assessment::Attempt.previews.count }) + + expect(response).to have_http_status(:ok) + attempt.reload + expect(attempt.workflow_state).to eq('attempting') + expect(attempt.current_answers).not_to be_empty + expect(attempt.current_answers.all?(&:attempting?)).to be(true) + expect(attempt.current_answers.map(&:id)).not_to match_array(original_answer_ids) + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + expect { subject }.to raise_exception(CanCan::AccessDenied) + end + end + end + + # PATCH #update goes through the REAL platform path: delegate_to_service(:update) → + # PreviewUpdateService. Each example asserts a persisted state change, not just a 200. + describe 'PATCH #update' do + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a.answers.reload + a + end + + it 'saves answer changes (the update is not a no-op)' do + answer = attempt.current_answers.first + option = answer.question.actable.options.first + patch :update, params: { + course_id: course.id, id: attempt.id, format: :json, + submission: { answers: [id: answer.id, option_ids: [option.id]] } + } + expect(response).to have_http_status(:ok) + expect(answer.reload.actable.options).to include(option) + end + + it 'finalises the attempt via the platform update path (finalise= alias)' do + patch :update, params: { course_id: course.id, id: attempt.id, format: :json, + submission: { finalise: true } } + expect(response).to have_http_status(:ok) + expect(attempt.reload).to be_submitted + end + + it 'drops the Submission-only EXP param the grader UI sends instead of crashing' do + # mark/publish payloads include draft_points_awarded; the parent UpdateService would permit it → + # UnknownAttributeError on an Attempt (no such column). The subclass drops it while mark fires. + # See the POST #reset note above: finalise! alone doesn't persist workflow_state; the mark + # event the controller triggers next needs it committed, since it re-`find`s the attempt. + attempt.finalise! + attempt.save! + patch :update, params: { course_id: course.id, id: attempt.id, format: :json, + submission: { mark: true, draft_points_awarded: 100 } } + expect(response).to have_http_status(:ok) + expect(attempt.reload).to be_graded + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + expect do + patch :update, params: { course_id: course.id, id: attempt.id, format: :json, + submission: { finalise: true } } + end.to raise_exception(CanCan::AccessDenied) + end + end + end + end +end diff --git a/spec/models/course/assessment/attempt_spec.rb b/spec/models/course/assessment/attempt_spec.rb index 82eaedf0779..3a8a1592c33 100644 --- a/spec/models/course/assessment/attempt_spec.rb +++ b/spec/models/course/assessment/attempt_spec.rb @@ -58,6 +58,13 @@ end end + describe '#attempt' do + it 'returns itself so the Attempt can stand in as its own base in the shared pipeline' do + attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) + expect(attempt.attempt).to be(attempt) + end + end + describe '#reset_attempt!' do it 'destroys existing answers, returns to attempting, and rebuilds answers' do attempt = create(:course_assessment_attempt, assessment: assessment, creator: previewer) From 3042cc01d1580cc6c2d27ff1b62ddae13d62e65d Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 20:05:22 +0800 Subject: [PATCH 08/11] feat(marketplace): preview attempt live-feedback and scribing endpoints --- config/routes.rb | 12 + .../preview_attempts_controller_spec.rb | 221 ++++++++++++++++++ 2 files changed, 233 insertions(+) diff --git a/config/routes.rb b/config/routes.rb index 53c337952b4..478a8ed804a 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -629,9 +629,21 @@ resource :attempt, only: [:create], controller: 'preview_attempts' end resources :preview_attempts, only: [:edit, :update], path: 'marketplace/attempt' do + collection do + get :fetch_live_feedback_chat + get :fetch_live_feedback_status + post :save_live_feedback + end + member do post :auto_grade post :reset + post :reload_answer + post :reevaluate_answer + post :generate_feedback + post :generate_live_feedback + post :create_live_feedback_chat + post 'answers/:answer_id/scribing/scribbles' => 'preview_attempts#create_scribing_scribble' end end end diff --git a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb index 4854f006bc7..437d486f31b 100644 --- a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb +++ b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb @@ -229,5 +229,226 @@ end end end + + describe 'live feedback endpoints' do + let(:source_assessment) { create(:assessment, :published_with_programming_question) } + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a + end + let(:answer) { attempt.answers.where(actable_type: 'Course::Assessment::Answer::Programming').first } + let(:question) { answer.question } + let!(:submission_question) do + create(:submission_question, submission: attempt, question: question) + end + let!(:thread) do + Course::Assessment::LiveFeedback::Thread.create!({ + codaveri_thread_id: SecureRandom.hex(12), + submission_question: submission_question, + is_active: true, + submission_creator_id: attempt.creator_id, + created_at: Time.zone.now + }) + end + + it 'fetches a preview attempt live feedback chat without an assessment_id route param' do + get :fetch_live_feedback_chat, params: { course_id: course.id, answer_id: answer.id, format: :json } + + expect(response).to have_http_status(:ok) + expect(response.parsed_body['threadId']).to eq(thread.codaveri_thread_id) + end + + it 'saves preview attempt live feedback without an assessment_id route param' do + post :save_live_feedback, params: { + course_id: course.id, + current_thread_id: thread.codaveri_thread_id, + content: 'Feedback from Codaveri', + is_error: false, + format: :json + } + + expect(response).to have_http_status(:no_content) + expect(thread.messages.last.content).to eq('Feedback from Codaveri') + end + + context 'when the thread belongs to another manager\'s preview' do + let(:others_attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: other_manager) + a.create_new_answers + a + end + let(:others_answer) do + others_attempt.answers.where(actable_type: 'Course::Assessment::Answer::Programming').first + end + let!(:others_submission_question) do + create(:submission_question, submission: others_attempt, question: others_answer.question) + end + let!(:others_thread) do + Course::Assessment::LiveFeedback::Thread.create!({ + codaveri_thread_id: SecureRandom.hex(12), + submission_question: others_submission_question, + is_active: true, + submission_creator_id: others_attempt.creator_id, + created_at: Time.zone.now + }) + end + + it 'denies saving into it (creator scoping via authorize_preview_attempt_for_thread!)' do + expect do + post :save_live_feedback, params: { + course_id: course.id, current_thread_id: others_thread.codaveri_thread_id, + content: 'nope', is_error: false, format: :json + } + end.to raise_exception(CanCan::AccessDenied) + end + end + + context 'when the answer belongs to another manager\'s preview attempt' do + let(:others_attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: other_manager) + a.create_new_answers + a + end + let(:others_answer) do + others_attempt.answers.where(actable_type: 'Course::Assessment::Answer::Programming').first + end + let!(:others_submission_question) do + create(:submission_question, submission: others_attempt, question: others_answer.question) + end + + it 'denies fetching its chat (creator scoping via authorize_preview_attempt_for_answer!)' do + expect do + get :fetch_live_feedback_chat, + params: { course_id: course.id, answer_id: others_answer.id, format: :json } + end.to raise_exception(CanCan::AccessDenied) + end + end + + context 'when the answer/thread belongs to a real (non-preview) submission' do + let(:real_student) { create(:course_student, course: source_assessment.course).user } + let(:real_submission) do + create(:submission, :attempting, assessment: source_assessment, creator: real_student) + end + let(:real_answer) do + real_submission.answers.where(actable_type: 'Course::Assessment::Answer::Programming').first + end + let!(:real_submission_question) do + create(:submission_question, submission: real_submission.attempt, question: real_answer.question) + end + let!(:real_thread) do + Course::Assessment::LiveFeedback::Thread.create!({ + codaveri_thread_id: SecureRandom.hex(12), + submission_question: real_submission_question, + is_active: true, + submission_creator_id: real_submission.creator_id, + created_at: Time.zone.now + }) + end + + it 'denies fetching its chat (attempt.preview? guard)' do + expect do + get :fetch_live_feedback_chat, + params: { course_id: course.id, answer_id: real_answer.id, format: :json } + end.to raise_exception(CanCan::AccessDenied) + end + + it 'denies saving into its thread (attempt.preview? guard)' do + expect do + post :save_live_feedback, params: { + course_id: course.id, current_thread_id: real_thread.codaveri_thread_id, + content: 'nope', is_error: false, format: :json + } + end.to raise_exception(CanCan::AccessDenied) + end + end + + context 'when there is no live feedback thread for the answer yet' do + # Own new assessment (not `source_assessment`) to avoid the (assessment, manager) unique-index + # collision with the outer `attempt` let. + let(:untried_assessment) { create(:assessment, :published_with_programming_question) } + let(:untried_attempt) do + a = create(:course_assessment_attempt, assessment: untried_assessment, creator: manager) + a.create_new_answers + a + end + let(:untried_answer) do + untried_attempt.answers.where(actable_type: 'Course::Assessment::Answer::Programming').first + end + let!(:untried_submission_question) do + create(:submission_question, submission: untried_attempt, question: untried_answer.question) + end + + it 'returns bad_request instead of a thread payload' do + get :fetch_live_feedback_chat, + params: { course_id: course.id, answer_id: untried_answer.id, format: :json } + expect(response).to have_http_status(:bad_request) + end + end + + it 'returns bad_request when the saved thread id matches no thread' do + # The nil-thread guard runs before authorize_preview_attempt_for_thread!, so no auth setup needed. + post :save_live_feedback, params: { + course_id: course.id, current_thread_id: 'nonexistent-thread-id', + content: 'ignored', is_error: false, format: :json + } + expect(response).to have_http_status(:bad_request) + end + end + + describe 'POST #create_scribing_scribble' do + let(:source_assessment) do + create(:assessment, :published).tap do |assessment| + create(:course_assessment_question_scribing, assessment: assessment) + end + end + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a + end + let(:answer) { attempt.answers.where(actable_type: 'Course::Assessment::Answer::Scribing').first } + + it 'updates preview attempt scribbles without an assessment_id route param' do + post :create_scribing_scribble, params: { + course_id: course.id, + id: attempt.id, + answer_id: answer.id, + scribble: { answer_id: answer.actable.id, content: '{"objects":[]}' }, + format: :json + } + + expect(response).to have_http_status(:ok) + expect(answer.actable.scribbles.last.content).to eq('{"objects":[]}') + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + expect do + post :create_scribing_scribble, params: { + course_id: course.id, id: attempt.id, answer_id: answer.id, + scribble: { answer_id: answer.actable.id, content: '{"objects":[]}' }, format: :json + } + end.to raise_exception(CanCan::AccessDenied) + end + end + + it 'returns bad_request when answer_id resolves to no scribing answer' do + post :create_scribing_scribble, params: { + course_id: course.id, id: attempt.id, answer_id: -1, + scribble: { answer_id: -1, content: '{"objects":[]}' }, format: :json + } + expect(response).to have_http_status(:bad_request) + end + + it 'returns bad_request when the scribble answer_id mismatches the resolved scribing answer' do + post :create_scribing_scribble, params: { + course_id: course.id, id: attempt.id, answer_id: answer.id, + scribble: { answer_id: answer.actable.id + 1, content: '{"objects":[]}' }, format: :json + } + expect(response).to have_http_status(:bad_request) + end + end end end From 9d4ed7ce78440c12d7d8e339df90289a8c53f434 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 20:19:56 +0800 Subject: [PATCH 09/11] fix(marketplace): authorize preview live-feedback status before external call; scope preview loading to previews --- .../preview_attempts_controller.rb | 17 ++++++++-------- .../preview_attempts_controller_spec.rb | 20 +++++++++++++++++++ 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb index 4eadd4a09a4..f374b0cf09d 100644 --- a/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb +++ b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb @@ -140,18 +140,19 @@ def fetch_live_feedback_chat def fetch_live_feedback_status thread_id = thread_params[:thread_id] - codaveri_api_service = CodaveriAsyncApiService.new("chat/feedback/threads/#{thread_id}", nil) + @thread = Course::Assessment::LiveFeedback::Thread.find_by(codaveri_thread_id: thread_id) + return head :bad_request if @thread.nil? - response_status, response_body = codaveri_api_service.get + # Authorize BEFORE the external (paid) Codaveri call: this collection action has no upfront + # authorize_attempt! gate, and the thread is resolvable from the DB alone. Doing the API call first + # would let an unauthorized caller trigger outbound calls / use the result as an existence oracle. + authorize_preview_attempt_for_thread!(@thread, :fetch_live_feedback_status) + codaveri_api_service = CodaveriAsyncApiService.new("chat/feedback/threads/#{thread_id}", nil) + response_status, response_body = codaveri_api_service.get raise CodaveriError, { status: response_status, body: response_body } if response_status != 200 @thread_status = response_body['data']['thread']['status'] - - @thread = Course::Assessment::LiveFeedback::Thread.find_by(codaveri_thread_id: thread_id) - return head :bad_request if @thread.nil? - - authorize_preview_attempt_for_thread!(@thread, :fetch_live_feedback_status) @thread.update!(is_active: @thread_status == 'active') render 'course/assessment/submission/submissions/fetch_live_feedback_status', status: response_status @@ -246,7 +247,7 @@ def build_attempt # Member routes are shallow (no listing_id): the attempt resolves its own assessment. Load with the # calculated grade/graded_at/log_count/grader_ids the reused edit payload reads. def load_attempt - @attempt = Course::Assessment::Attempt.all. + @attempt = Course::Assessment::Attempt.previews. calculated(:grade, :graded_at, :log_count, :grader_ids).find(params[:id]) @submission = @attempt @assessment = @attempt.assessment diff --git a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb index 437d486f31b..94d66f2fc02 100644 --- a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb +++ b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb @@ -110,6 +110,15 @@ expect { subject }.to raise_exception(CanCan::AccessDenied) end end + + context 'when the id is a real submission (not a preview)' do + let(:real_submission) { create(:submission, assessment: source_assessment, creator: other_manager) } + it 'is not found (load scoped to previews)' do + expect do + get :edit, params: { course_id: course.id, id: real_submission.attempt_id, format: :json } + end.to raise_exception(ActiveRecord::RecordNotFound) + end + end end describe 'POST #auto_grade' do @@ -394,6 +403,17 @@ } expect(response).to have_http_status(:bad_request) end + + context 'GET #fetch_live_feedback_status by a non-creator' do + before { controller_sign_in(controller, other_manager) } + it 'denies BEFORE making any Codaveri API call' do + expect(CodaveriAsyncApiService).not_to receive(:new) + expect do + get :fetch_live_feedback_status, + params: { course_id: course.id, thread_id: thread.codaveri_thread_id, format: :json } + end.to raise_exception(CanCan::AccessDenied) + end + end end describe 'POST #create_scribing_scribble' do From 1f4c9cd32bfe882102f64cec1da6fb2c681db0d5 Mon Sep 17 00:00:00 2001 From: lws49 Date: Thu, 23 Jul 2026 21:05:53 +0800 Subject: [PATCH 10/11] refactor(assessment): de-stutter auto-grading EXP param (submission.submission -> gradable.submission) The dual-typed param (Attempt in production, Submission in legacy specs) read as submission.submission when resolving the extension. Rename to `gradable` so the extension access reads gradable.submission. Behavior-preserving. --- .../course/assessment/submission/auto_grading_service.rb | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/app/services/course/assessment/submission/auto_grading_service.rb b/app/services/course/assessment/submission/auto_grading_service.rb index 9ba5a6dca3d..8a5e143eae5 100644 --- a/app/services/course/assessment/submission/auto_grading_service.rb +++ b/app/services/course/assessment/submission/auto_grading_service.rb @@ -107,14 +107,16 @@ def unsubmit_answers(submission) # extension: award them there for a real attempt, and for a preview (no extension) publish the graded # work WITHOUT awarding EXP. The `is_a?(Submission)` branch keeps callers that pass a Submission # directly (e.g. specs) working unchanged. - def assign_exp_and_publish_grade(submission) - exp_record = submission.is_a?(Course::Assessment::Submission) ? submission : submission.submission + # `gradable` is either a Submission extension (legacy callers/specs) or an Attempt base (production); + # `gradable.submission` resolves the extension for an Attempt without the `submission.submission` stutter. + def assign_exp_and_publish_grade(gradable) + exp_record = gradable.is_a?(Course::Assessment::Submission) ? gradable : gradable.submission if exp_record exp_record.points_awarded = Course::Assessment::Submission::CalculateExpService.calculate_exp(exp_record).to_i exp_record.publish! else - submission.publish! + gradable.publish! end end From 4c282864c3db04955a175f473861af74cf4f5e91 Mon Sep 17 00:00:00 2001 From: lws49 Date: Fri, 24 Jul 2026 12:45:18 +0800 Subject: [PATCH 11/11] feat(marketplace): preview attempt per-answer save and submit The preview reuses the submission edit UI, whose per-answer autosave and per-question submit hit dedicated answer endpoints. Add the preview equivalents so saving/submitting an answer in a preview no longer 404s on the platform /assessments/:aid/submissions/... route. - New shallow routes + #save_draft / #submit_answer actions on the preview attempts controller, reusing the platform answer write path (UpdateAnswerConcern) and the extracted SubmitAnswerConcern. - Extract the submit auto-grade helpers from AnswersController into Course::Assessment::Answer::SubmitAnswerConcern (renamed auto_grade -> auto_grade_answer to avoid colliding with the preview controller's whole-attempt #auto_grade action). - Grant :save_draft/:submit_answer on a preview-owned Attempt. --- .../answer/submit_answer_concern.rb | 74 +++++++++++++++++++ .../preview_attempts_controller.rb | 50 ++++++++++++- .../submission/answer/answers_controller.rb | 63 +--------------- ...ssessment_marketplace_ability_component.rb | 3 +- config/routes.rb | 2 + .../preview_attempts_controller_spec.rb | 64 ++++++++++++++++ 6 files changed, 191 insertions(+), 65 deletions(-) create mode 100644 app/controllers/concerns/course/assessment/answer/submit_answer_concern.rb diff --git a/app/controllers/concerns/course/assessment/answer/submit_answer_concern.rb b/app/controllers/concerns/course/assessment/answer/submit_answer_concern.rb new file mode 100644 index 00000000000..618908bbb18 --- /dev/null +++ b/app/controllers/concerns/course/assessment/answer/submit_answer_concern.rb @@ -0,0 +1,74 @@ +# frozen_string_literal: true +# Shared auto-grade-on-submit behaviour for per-answer submission. Extracted from +# Course::Assessment::Submission::Answer::AnswersController so the marketplace preview +# controller can reuse the exact same submit path. Expects @assessment and @submission to +# be set, and the including controller to be able to `render`. +module Course::Assessment::Answer::SubmitAnswerConcern + extend ActiveSupport::Concern + + protected + + def should_auto_grade_on_submit(answer) + mcq = [I18n.t('course.assessment.question.multiple_responses.question_type.multiple_response'), + I18n.t('course.assessment.question.multiple_responses.question_type.multiple_choice')] + + !mcq.include?(answer.question.question_type_readable) || + !@submission.assessment.allow_partial_submission || + @submission.assessment.show_mcq_answer + end + + # Named auto_grade_answer (not auto_grade) so it does not collide with a whole-submission + # `auto_grade` action a host controller may define (the marketplace preview controller does). + def auto_grade_answer(answer) + return unless valid_for_grading?(answer) + + # Check if the last attempted answer is still being evaluated, then dont reattempt. + job = last_attempt_answer_submitted_job(answer) || reattempt_and_grade_answer(answer)&.job + if job + render partial: 'jobs/submitted', locals: { job: job } + else + render answer + end + end + + # Test whether the answer can be graded or not. + def valid_for_grading?(answer) + return true if @assessment.autograded? + return true unless answer.specific.is_a?(Course::Assessment::Answer::Programming) + + answer.specific.attempting_times_left > 0 || can?(:manage, @assessment) + end + + def last_attempt_answer_submitted_job(answer) + submission = answer.submission + + attempts = submission.answers.from_question(answer.question_id) + last_non_current_answer = attempts.reject(&:current_answer?).last + auto_grading = last_non_current_answer&.auto_grading + job = auto_grading&.job + (job&.status == 'submitted') ? job : nil + end + + def reattempt_and_grade_answer(answer) + # The transaction is to make sure that the new attempt, auto grading and job are present when + # the current answer is submitted. + # + # If the latest answer has an errored job, the user may still modify current_answer before + # grading again. Failed autograding jobs should not count towards their answer attempt limit, + # so destroy the failed job answer and re-grade the current entry. + answer.class.transaction do + last_answer = answer.submission.answers.select { |ans| ans.question_id == answer.question_id }.last + last_answer_job = last_answer&.auto_grading&.job + last_answer.destroy! if last_answer_job&.errored? + new_answer = reattempt_answer(answer, finalise: true) + new_answer.auto_grade!(redirect_to_path: nil, reduce_priority: false) + end + end + + def reattempt_answer(answer, finalise: true) + new_answer = answer.question.attempt(answer.submission, answer) + new_answer.finalise! if finalise + new_answer.save! + new_answer + end +end diff --git a/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb index f374b0cf09d..8e9c16e1a1f 100644 --- a/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb +++ b/app/controllers/course/assessment/marketplace/preview_attempts_controller.rb @@ -4,10 +4,15 @@ class Course::Assessment::Marketplace::PreviewAttemptsController < # rubocop:dis include Course::Assessment::LiveFeedback::MessageConcern include Course::Assessment::LiveFeedback::ThreadConcern include Course::Assessment::Submission::SubmissionsControllerServiceConcern + # Per-answer draft save (#save_draft) and submit (#submit_answer) reuse the same answer + # write path as the platform AnswersController. + include Course::Assessment::Answer::UpdateAnswerConcern + include Course::Assessment::Answer::SubmitAnswerConcern MEMBER_ACTIONS = [ :edit, :update, :auto_grade, :reset, :reload_answer, :reevaluate_answer, :generate_feedback, - :generate_live_feedback, :create_live_feedback_chat, :create_scribing_scribble + :generate_live_feedback, :create_live_feedback_chat, :create_scribing_scribble, + :save_draft, :submit_answer ].freeze before_action :load_listing, only: [:create] @@ -19,7 +24,7 @@ class Course::Assessment::Marketplace::PreviewAttemptsController < # rubocop:dis before_action :load_or_create_submission_questions, only: [:edit, :update, :auto_grade, :reset, :reload_answer, :reevaluate_answer, :generate_feedback, :generate_live_feedback, :create_live_feedback_chat, - :create_scribing_scribble] + :create_scribing_scribble, :save_draft, :submit_answer] # Defines #update: service.update → answer saves + workflow transitions + render 'edit'. @assessment/ # @submission are snapshot at service memoization, hence set in load_attempt, never in the action. @@ -88,6 +93,37 @@ def generate_feedback render partial: 'jobs/submitted', locals: { job: job } end + # Per-answer autosave. Mirrors AnswersController#update; the whole-submission #update above + # is a different (finalise/mark) path. + def save_draft + @answer = @submission.answers.find_by(id: params[:answer_id]) + return head :bad_request if @answer.nil? + + if update_answer(@answer, draft_answer_params) + render @answer + else + render json: { errors: @answer.errors }, status: :bad_request + end + end + + # Per-question submit + autograde. Mirrors AnswersController#submit_answer. The per-answer + # auto_grade path (answer.auto_grade!) is already exercised by #reevaluate_answer above and + # PreviewAutoGradingService, so it is safe on a bare preview Attempt (no EXP/publish tail). + def submit_answer + @answer = @submission.answers.find_by(id: params[:answer_id]) + return head :bad_request if @answer.nil? + + if update_answer(@answer, draft_answer_params) + if should_auto_grade_on_submit(@answer) + auto_grade_answer(@answer) + else + render @answer + end + else + render json: { errors: @answer.errors }, status: :bad_request + end + end + def generate_live_feedback # rubocop:disable Metrics/AbcSize, Metrics/MethodLength @answer = @submission.answers.find_by(id: live_feedback_params[:answer_id]) return head :bad_request if @answer.nil? @@ -199,7 +235,9 @@ def create_scribing_scribble 'generate_feedback' => :generate_feedback, 'generate_live_feedback' => :generate_live_feedback, 'create_live_feedback_chat' => :create_live_feedback_chat, - 'create_scribing_scribble' => :create_scribing_scribble + 'create_scribing_scribble' => :create_scribing_scribble, + 'save_draft' => :save_draft, + 'submit_answer' => :submit_answer }.freeze private @@ -257,6 +295,12 @@ def reload_answer_params params.permit(:answer_id, :reset_answer) end + # UpdateAnswerConcern#update_answer permits its own answer-type fields off this root; require + # the wrapper key only. (The existing #answer_params permits :answer_id for live feedback.) + def draft_answer_params + params.require(:answer) + end + def answer_params params.permit(:answer_id) end diff --git a/app/controllers/course/assessment/submission/answer/answers_controller.rb b/app/controllers/course/assessment/submission/answer/answers_controller.rb index 7dca3f93169..f75838f47c4 100644 --- a/app/controllers/course/assessment/submission/answer/answers_controller.rb +++ b/app/controllers/course/assessment/submission/answer/answers_controller.rb @@ -4,6 +4,7 @@ class Course::Assessment::Submission::Answer::AnswersController < Course::Assessment::Submission::Answer::Controller include Course::Assessment::SubmissionConcern include Course::Assessment::Answer::UpdateAnswerConcern + include Course::Assessment::Answer::SubmitAnswerConcern before_action :authorize_submission! before_action :check_password, only: [:update] @@ -27,7 +28,7 @@ def submit_answer if update_answer(@answer, answer_params) if should_auto_grade_on_submit(@answer) - auto_grade(@answer) + auto_grade_answer(@answer) else render @answer end @@ -41,64 +42,4 @@ def submit_answer def answer_params params.require(:answer) end - - def should_auto_grade_on_submit(answer) - mcq = [I18n.t('course.assessment.question.multiple_responses.question_type.multiple_response'), - I18n.t('course.assessment.question.multiple_responses.question_type.multiple_choice')] - - !mcq.include?(answer.question.question_type_readable) || - !@submission.assessment.allow_partial_submission || - @submission.assessment.show_mcq_answer - end - - def auto_grade(answer) - return unless valid_for_grading?(answer) - - # Check if the last attempted answer is still being evaluated, then dont reattempt. - job = last_attempt_answer_submitted_job(answer) || reattempt_and_grade_answer(answer)&.job - if job - render partial: 'jobs/submitted', locals: { job: job } - else - render answer - end - end - - # Test whether the answer can be graded or not. - def valid_for_grading?(answer) - return true if @assessment.autograded? - return true unless answer.specific.is_a?(Course::Assessment::Answer::Programming) - - answer.specific.attempting_times_left > 0 || can?(:manage, @assessment) - end - - def last_attempt_answer_submitted_job(answer) - submission = answer.submission - - attempts = submission.answers.from_question(answer.question_id) - last_non_current_answer = attempts.reject(&:current_answer?).last - job = last_non_current_answer&.auto_grading&.job - job&.status == 'submitted' ? job : nil - end - - def reattempt_and_grade_answer(answer) - # The transaction is to make sure that the new attempt, auto grading and job are present when - # the current answer is submitted. - # - # If the latest answer has an errored job, the user may still modify current_answer before - # grading again. Failed autograding jobs should not count towards their answer attempt limit, - # so destroy the failed job answer and re-grade the current entry. - answer.class.transaction do - last_answer = answer.submission.answers.select { |ans| ans.question_id == answer.question_id }.last - last_answer.destroy! if last_answer&.auto_grading&.job&.errored? - new_answer = reattempt_answer(answer, finalise: true) - new_answer.auto_grade!(redirect_to_path: nil, reduce_priority: false) - end - end - - def reattempt_answer(answer, finalise: true) - new_answer = answer.question.attempt(answer.submission, answer) - new_answer.finalise! if finalise - new_answer.save! - new_answer - end end diff --git a/app/models/components/course/assessment_marketplace_ability_component.rb b/app/models/components/course/assessment_marketplace_ability_component.rb index f5acc97a52e..857067ab447 100644 --- a/app/models/components/course/assessment_marketplace_ability_component.rb +++ b/app/models/components/course/assessment_marketplace_ability_component.rb @@ -68,7 +68,8 @@ def allow_managers_access_marketplace # rubocop:disable Metrics/AbcSize can :create, Course::Assessment::Attempt can [:read, :update, :grade, :reset, :reload_answer, :reevaluate_answer, :generate_feedback, :generate_live_feedback, :create_live_feedback_chat, :fetch_live_feedback_chat, - :fetch_live_feedback_status, :save_live_feedback, :create_scribing_scribble, :read_tests], + :fetch_live_feedback_status, :save_live_feedback, :create_scribing_scribble, :read_tests, + :save_draft, :submit_answer], Course::Assessment::Attempt do |attempt| attempt.preview? && attempt.creator_id == user.id end diff --git a/config/routes.rb b/config/routes.rb index 478a8ed804a..8cff26210a7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -643,6 +643,8 @@ post :generate_feedback post :generate_live_feedback post :create_live_feedback_chat + patch 'answers/:answer_id' => 'preview_attempts#save_draft' + patch 'answers/:answer_id/submit_answer' => 'preview_attempts#submit_answer' post 'answers/:answer_id/scribing/scribbles' => 'preview_attempts#create_scribing_scribble' end end diff --git a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb index 94d66f2fc02..7259bd8c1e4 100644 --- a/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb +++ b/spec/controllers/course/assessment/marketplace/preview_attempts_controller_spec.rb @@ -239,6 +239,70 @@ end end + describe 'PATCH #save_draft' do + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a.answers.reload + a + end + + it 'saves the per-answer draft (not a no-op)' do + answer = attempt.current_answers.first + option = answer.question.actable.options.first + patch :save_draft, params: { + course_id: course.id, id: attempt.id, answer_id: answer.id, format: :json, + answer: { id: answer.id, option_ids: [option.id] } + } + expect(response).to have_http_status(:ok) + expect(answer.reload.actable.options).to include(option) + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + answer = attempt.current_answers.first + expect do + patch :save_draft, params: { course_id: course.id, id: attempt.id, answer_id: answer.id, + format: :json, answer: { id: answer.id, option_ids: [] } } + end.to raise_exception(CanCan::AccessDenied) + end + end + end + + describe 'PATCH #submit_answer' do + let(:source_assessment) { create(:assessment, :published, :with_mrq_question, :autograded) } + let(:attempt) do + a = create(:course_assessment_attempt, assessment: source_assessment, creator: manager) + a.create_new_answers + a.answers.reload + a + end + + it 'submits and autogrades on the bare preview attempt without an EXP-tail crash' do + answer = attempt.current_answers.first + expect do + patch :submit_answer, params: { + course_id: course.id, id: attempt.id, answer_id: answer.id, format: :json, + answer: { id: answer.id } + } + end.to change { attempt.answers.count }.by(1) + expect(response).to have_http_status(:ok) + expect(attempt.reload.answers.last.workflow_state).to eq('graded') + end + + context 'when the requester is a different manager (not the creator)' do + before { controller_sign_in(controller, other_manager) } + it 'is denied' do + answer = attempt.current_answers.first + expect do + patch :submit_answer, params: { course_id: course.id, id: attempt.id, answer_id: answer.id, + format: :json, answer: { id: answer.id } } + end.to raise_exception(CanCan::AccessDenied) + end + end + end + describe 'live feedback endpoints' do let(:source_assessment) { create(:assessment, :published_with_programming_question) } let(:attempt) do