PR7a — feat(assessment): Attempt base-record split (Phase 1, additive)#8510
Draft
LWS49 wants to merge 11 commits into
Draft
PR7a — feat(assessment): Attempt base-record split (Phase 1, additive)#8510LWS49 wants to merge 11 commits into
LWS49 wants to merge 11 commits into
Conversation
LWS49
force-pushed
the
lws49/feat-marketplace-pr6d-audit
branch
3 times, most recently
from
July 24, 2026 04:44
5230cdc to
86dac50
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr7a-attempt-base-record
branch
from
July 24, 2026 04:46
93216c7 to
68546f0
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr6d-audit
branch
from
July 24, 2026 05:02
86dac50 to
69326c2
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr7a-attempt-base-record
branch
from
July 24, 2026 05:03
68546f0 to
b1b31c1
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr6d-audit
branch
from
July 24, 2026 05:29
69326c2 to
74232c9
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr7a-attempt-base-record
branch
4 times, most recently
from
July 24, 2026 07:37
fbed966 to
f35dd0c
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr6d-audit
branch
from
July 24, 2026 09:29
74232c9 to
ec36419
Compare
LWS49
force-pushed
the
lws49/feat-marketplace-pr7a-attempt-base-record
branch
from
July 24, 2026 09:29
f35dd0c to
eb015b3
Compare
Phase 2 preview-leak gate: num_attempted/num_submitted/num_late/latest_submission read the base course_assessment_submissions directly, so a preview (an Attempt with no course_assessment_submission_details row) would inflate them. Add an INNER JOIN to the extension table to restrict to real submissions. No-op on current data (no previews exist yet); regression test seeds a bare preview attempt and asserts the counts hold.
…istics Phase 2 preview-leak gate (continued). grade/duration/answer-correctness raw SQL and the assessment submission_statistics/ancestor/live_feedback list builders read the base course_assessment_submissions (or Attempt directly), so a preview (Attempt with no extension row) would leak into staff-facing statistics. Add INNER JOINs to course_assessment_submission_details (raw SQL) and .joins(:submission) (AR) to restrict to real submissions. No-op on current data; regression test seeds a bare preview attempt and asserts submission_statistics reports the student as unstarted (mutation-verified).
…ussion from_user Phase 2 preview-leak gate (model layer): - grade_summary raw SQL reads the base directly; add INNER JOIN to the extension so a graded preview (Attempt with no extension row) is not summed into gradebook grades. - SubmissionQuestion.from_user and ProgrammingFileAnnotation.from_user join the Attempt base; join through to the extension (submission: :submission) to exclude previews. Also fixes a Task 2 regression that shipped in ff465f5 and the Phase 1 gate missed (topic_spec was not in the gate): ProgrammingFileAnnotation.from_user referenced Course::Assessment::Submission.arel_table[:creator_id], but post-split that arel_table is the column-less extension table -> PG::UndefinedTable. creator_id lives on Attempt now. Regression covered by spec/models/course/assessment/submission_spec.rb (grade_summary) and the now-green spec/models/course/discussion/topic_spec.rb (from_user).
…1 gate Three sites broke when Submission was split into Attempt + extension but were not in the 12-file acceptance gate, so they shipped red in ff465f5: - ProgrammingFileAnnotation.from_user referenced Submission.arel_table[:creator_id] (now the column-less extension table) -> PG::UndefinedTable (fixed in prior commit). - answer/comment_notifier/annotated email called submission.course_user on the Attempt base; route through attempt.submission (the extension) like its replied sibling. - ai_generated_post_service_spec called answer.submission.course_user (Attempt); the service already uses .submission.submission.course_user — update the spec to match.
assessment_ability.rb was not touched by the split (ff465f5) but its CanCan hash conditions navigate associations that moved: Course::Assessment::Submission (extension) no longer has assessment/creator_id/workflow_state (now on the Attempt base). Rules that did 'Submission, assessment:/creator_id:' raised CanCan::WrongAssociationName, and the attempting-hash (experience_points_record) applied via Answer.submission (now Attempt) raised NoMethodError. Route Submission conditions through attempt:, and express the attempting-owned check against the Attempt (workflow_state + creator_id, the owner per validate_consistent_user). assessment_ability_spec: 75 examples, 0 failures (was 11 red). Another gate-missed Task 2 regression (spec not in the 12-file acceptance gate).
More gate-missed Task 2 regressions (download/statistics/gradebook paths). The split left the extension Submission unable to serve two relation-level access patterns its callers use: - calculated attributes: register grade/grader_ids/log_count/graded_at on Submission (correlated on attempt_id) so @assessment.submissions.calculated(...) works — was raising 'undefined method call for nil' in submissions_controller#load_submissions and the statistics/csv/zip download services. - eager loading: Submission has no :answers/:assessment AR association (those moved to Attempt), so includes(:answers)/(:assessment) raised AssociationNotFound; route them through includes(attempt: ...). Verified green: zip/csv/statistics/ssid download services + jobs, submissions_controller.
Final batch of gate-missed Task 2 regressions: - base_auto_grading_job: update_exp?/update_exp received answer.submission (now the Attempt), whose awarder/awarded_at/points_awarded live on the extension -> the guard was always false and re-grade never recomputed EXP (points stale). Route through the extension. - programming_ability: :create/:destroy_programming_files checked can?(:update, attempt); :update rules are on the extension Submission -> check answer.submission.submission. - live_feedback ThreadConcern: SubmissionQuestion.where(submission_id: @submission) coerced the extension to its own id; match @submission.attempt_id (spec builds the SQ on the attempt). - skills_mastery_preload: plucked Submission#id but belonging_to_submissions matches the attempt id -> pluck(:attempt_id). - auto_grading_service/job specs: stub auto_grade_submission on the attempt, update_column :submitted_at on the attempt, reload the separate extension instance before points assertions.
…n questions) Final Phase 2 preview-leak gate items: - system-admin and instance-admin get-help usage reports join course_assessment_submissions directly; add INNER JOIN course_assessment_submission_details so preview attempts' live feedback never inflates get-help analytics. - submission_questions#all_answers fetched via @assessment.attempts.find(...).submission and dereferenced it unguarded; a preview attempt yields a nil submission (previously only the incidental authorize!(:read, nil) blocked it). Raise RecordNotFound explicitly; regression test covers it.
…wer/SubmissionQuestion#attempt alias The extension accessor chain read as answer.submission.submission because Answer#submission returns the Attempt base (FK column is misleadingly submission_id). Add a reader-only #attempt alias on Answer and SubmissionQuestion and rewrite the 8 chain sites to answer.attempt.submission. Attempt#submission (the extension has_one) is unchanged. Behavior-preserving. Also extract the identical submission= writer that coerces a Submission (extension) to its Attempt (base) — shared verbatim by Answer, SubmissionQuestion, and QuestionBundleAssignment — into the new Course::Assessment::CoercesSubmissionToAttempt concern. Behavior-preserving. Also correct the belongs_to :submission comment in SubmissionQuestion (it wrongly said the column was "renamed from submission_id").
LWS49
force-pushed
the
lws49/feat-marketplace-pr7a-attempt-base-record
branch
from
July 24, 2026 09:56
eb015b3 to
e8b9992
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
PR7a — Attempt base-record split (Phase 1)
Stacked on #8507 (
pr6d-audit). Phase 1 only: the additive base-record split — no preview feature.Splits
Course::Assessment::Submissioninto anAttemptbase record (keeps the unrenamedcourse_assessment_submissionstable viaself.table_name) plus aSubmissionextension (course_assessment_submission_details), repairs every regression the split touched, and pre-emptively guards base-table reads so no preview row (added in PR7b) can leak into staff-facing counts/stats/reports.Additive-only — no rename, no data risk. The extension table is new; answers/submission_questions keep
submission_id(the base id). Backfill is a gentle 1:1 side-table insert. The unique index(assessment_id, creator_id)stays.Commits
Attemptbase +Submissionextension)grade_summary, discussionfrom_user, get-help reports) — behavior-preserving today (no previews exist yet)Reviewer guide —
submission.rbnet changesThe
submission.rbdiff is large but almost entirely relocation, not deletion:Submissionshrank to the course-coupled EXP extension, and the attempt/answers/grading machinery moved toattempt.rb. Nothing below was deleted outright — every removed line either moved toattempt.rbor stayed with a query rewrite.Moved out to
attempt.rb(the base record):workflow do … end) + theWorkflow/WorkflowEventConcern/AnswersConcernincludesafter_save :auto_grade_submission,after_save :retrieve_codaveri_feedback,after_create :create_force_submission_jobbelongs_to :assessment;has_many :submission_questions,:answers(+ the MRQ / text / programming / scribing / forum-post actable answer associations),:question_bundle_assignments,:logs;accepts_nested_attributes_for :answersvalidate_unique_submission,validate_autograded_no_partial_answer;validates :submitted_at,:workflow_state,:creator,:updater,:assessmentauto_grade!,auto_feedback!,questions,assigned_questions,current_answers,current_programming_answers,answer_history,user_get_help_message_counts,evaluated_or_graded_answers,unsubmitting?,submission_view_blocked?,create_force_submission_job; privatesauto_grade_submission,retrieve_codaveri_feedbackStayed on
Submission(the EXP / course-coupled slice):acts_as_experience_points_record,belongs_to :publisher,validates :last_graded_timevalidate_consistent_user,validate_awarded_attributescurrent_points_awarded,self.grade_summary,self.on_dependent_status_change, thealias_methodworkflow setterscalculated :graded_at / :log_count / :grade / :grader_ids— kept, but the subquery now correlates onattempt_idinstead of the baseidby_user/by_usersrewritten as attempt subqueries;confirmed/pending_for_gradingnow join the baseNew (extension wiring the split introduced):
self.table_name = 'course_assessment_submission_details',belongs_to :attemptdelegate … to: :attempt— re-exposes the ~30 attempt-level readers/writers so existingsubmission.<x>call sites keep working unchangedself.record_userstamp = false+ creator/updater reflection cleanup (stamping belongs to the base)finalise!/publish!/unsubmit!/resubmit_programming!+ theafter_*_hookseams, with the EXP/email tail (assign_zero_experience_points,send_email_after_publishing,publish_delayed_posts, …) relocated here fromWorkflowEventConcernassessment_id,ensure_last_graded_time,repoint_attempt_errors_to_baseBEFORE (pre-split)
erDiagram COURSE_ASSESSMENT_SUBMISSIONS { int id PK int assessment_id FK string workflow_state int creator_id FK int updater_id FK int publisher_id FK datetime published_at string session_id datetime submitted_at datetime last_graded_time } COURSE_ASSESSMENT_ANSWERS { int id PK int submission_id FK } COURSE_ASSESSMENT_SUBMISSION_QUESTIONS { int id PK int submission_id FK int question_id FK } COURSE_EXPERIENCE_POINTS_RECORDS { int id PK int course_user_id FK int points_awarded int draft_points_awarded int awarder_id FK datetime awarded_at int actable_id FK "polymorphic" string actable_type "polymorphic" } COURSE_ASSESSMENT_SUBMISSIONS ||--o{ COURSE_ASSESSMENT_ANSWERS : "has" COURSE_ASSESSMENT_SUBMISSIONS ||--o{ COURSE_ASSESSMENT_SUBMISSION_QUESTIONS : "has" COURSE_ASSESSMENT_SUBMISSIONS ||--o| COURSE_EXPERIENCE_POINTS_RECORDS : "acts_as_experience_points_record (polymorphic actable)"Course::Assessment::Submissionis a single model mapped straight tocourse_assessment_submissions. It directlyacts_as_experience_points_record, so the EXP identity columns (course_user_id,points_awarded,draft_points_awarded,awarder_id,awarded_at) live oncourse_experience_points_records, linked back via a polymorphicactable. There is nocourse_assessment_submission_detailstable — it does not exist yet.AFTER — PR7a (base/extension split)
erDiagram COURSE_ASSESSMENT_SUBMISSIONS { int id PK int assessment_id FK string workflow_state int creator_id FK int updater_id FK int publisher_id FK "duplicated, deferred cleanup" datetime published_at string session_id "duplicated, deferred cleanup" datetime submitted_at datetime last_graded_time "duplicated, deferred cleanup" } COURSE_ASSESSMENT_SUBMISSION_DETAILS { int id PK int attempt_id FK "NEW, UNIQUE" int publisher_id FK string session_id datetime last_graded_time } COURSE_ASSESSMENT_ANSWERS { int id PK int submission_id FK "unchanged column name; now points to Attempt base id" } COURSE_ASSESSMENT_SUBMISSION_QUESTIONS { int id PK int submission_id FK "unchanged column name; now points to Attempt base id" int question_id FK } COURSE_EXPERIENCE_POINTS_RECORDS { int id PK int course_user_id FK int points_awarded int draft_points_awarded int awarder_id FK datetime awarded_at int actable_id FK "polymorphic" string actable_type "polymorphic" } COURSE_ASSESSMENT_SUBMISSIONS ||--o{ COURSE_ASSESSMENT_ANSWERS : "has" COURSE_ASSESSMENT_SUBMISSIONS ||--o{ COURSE_ASSESSMENT_SUBMISSION_QUESTIONS : "has" COURSE_ASSESSMENT_SUBMISSIONS ||--o| COURSE_ASSESSMENT_SUBMISSION_DETAILS : "0..1 (absent ⇒ preview) — NEW relationship" COURSE_ASSESSMENT_SUBMISSION_DETAILS ||--o| COURSE_EXPERIENCE_POINTS_RECORDS : "acts_as_experience_points_record moved here (polymorphic actable)"What's new in this diagram: the
COURSE_ASSESSMENT_SUBMISSION_DETAILSbox (a brand-new table) and the0..1 (absent ⇒ preview)relationship line connecting it to the base table — that single optional edge is the entire feature. Everything else (answers, submission_questions, their FK columns) is structurally untouched; only what the FK conceptually points to has shifted from "the one and only submission row" to "the Attempt base row, which may or may not have a submission extension."course_assessment_submissionsis nowCourse::Assessment::Attempt— the base record: workflow_state, answers, grading, and the unique(assessment_id, creator_id)index.course_assessment_submission_detailsis the newCourse::Assessment::Submissionextension,has_one-linked viaattempt_id(unique, so at most one extension row per base row). The extension is now whatacts_as_experience_points_record.The base table still physically carries
publisher_id,session_id, andlast_graded_timealongside the new extension's copies of the same names — this is intentional duplication left over from the additive, no-rename migration strategy. A later deferred-cleanup PR drops the base table's copies once all callers read from the extension. This is noted here in prose rather than cluttering the diagram above.ID identity — how old data stays compatible
The base table was never renamed or re-keyed, so every historical submission row keeps its id. The backfill inserts one extension row per base row with
attempt_id = base.id. The result:Attempt(base,course_assessment_submissions)attempt.idSubmission(extension,course_assessment_submission_details)submission.idSubmission(extension)submission.attempt_idSo
submission.attempt_id == attempt.id ==the pre-split submission's original id. Every child FK (answers.submission_id,submission_questions.submission_id,logs.submission_id, …) still references that base id, unchanged — which is why old data stays valid.Consequential rule for the whole split: whenever code holds a
Submission(extension) object and must match a child'ssubmission_id, use.attempt_id, not.id(e.g. thelive_feedback/thread_concernand koditsusubmissions_concernfixes in PR7a)..idis the extension's own serial and matches nothing on the children.