Skip to content

Commit fbed966

Browse files
committed
refactor(assessment): de-stutter answer.submission.submission via Answer/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.
1 parent 5bc3309 commit fbed966

11 files changed

Lines changed: 37 additions & 39 deletions

File tree

app/controllers/course/assessment/submission/answer/programming/annotations_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,7 @@ def create_topic_subscription
6666

6767
# Ensure all group managers get a notification when someone adds a programming annotation
6868
# to the answer.
69-
answer_course_user = @answer.submission.submission.course_user
69+
answer_course_user = @answer.attempt.submission.course_user
7070
answer_course_user.my_managers.each do |manager|
7171
@discussion_topic.ensure_subscribed_by(manager.user)
7272
end

app/controllers/course/assessment/submission_question/comments_controller.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ def create_topic_subscription
3434
@discussion_topic.ensure_subscribed_by(@submission_question.submission.creator)
3535

3636
# Ensure all group managers get a notification when someone comments on this submission question
37-
submission_question_course_user = @submission_question.submission.submission.course_user
37+
submission_question_course_user = @submission_question.attempt.submission.course_user
3838
submission_question_course_user.my_managers.each do |manager|
3939
@discussion_topic.ensure_subscribed_by(manager.user)
4040
end

app/jobs/course/assessment/answer/base_auto_grading_job.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ def perform_tracked(answer, redirect_to_path = nil)
4646
# `answer.submission` is the Attempt base; EXP (awarder/awarded_at/points_awarded) lives on its
4747
# Submission extension. Recompute against the extension (nil for a preview attempt, which has no
4848
# EXP and must be skipped).
49-
submission = answer.submission.submission
49+
submission = answer.attempt.submission
5050
if submission && update_exp?(submission)
5151
Course::Assessment::Submission::CalculateExpService.update_exp(submission)
5252
end
Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
# frozen_string_literal: true
2+
# Lets a caller assign a `Course::Assessment::Submission` (the extension) to a `submission`
3+
# association whose real target is the `Course::Assessment::Attempt` base. The `belongs_to` writer
4+
# strictly checks `record.is_a?(reflection.klass)`, so a `Submission` would otherwise raise
5+
# `ActiveRecord::AssociationTypeMismatch`. Coerce it to its `Attempt` so every caller works
6+
# unchanged. Shared verbatim by Answer, SubmissionQuestion, and QuestionBundleAssignment.
7+
module Course::Assessment::CoercesSubmissionToAttempt
8+
extend ActiveSupport::Concern
9+
10+
def submission=(value)
11+
value = value.attempt if value.is_a?(Course::Assessment::Submission)
12+
super
13+
end
14+
end

app/models/course/assessment/answer.rb

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -57,18 +57,15 @@ class Course::Assessment::Answer < ApplicationRecord
5757
# base record: `submission_id` identifies an Attempt row (the base), not a Submission row.
5858
belongs_to :submission, class_name: 'Course::Assessment::Attempt', inverse_of: :answers,
5959
foreign_key: 'submission_id'
60+
include Course::Assessment::CoercesSubmissionToAttempt
61+
6062
belongs_to :question, class_name: 'Course::Assessment::Question', inverse_of: nil
6163

62-
# Coerce a `Course::Assessment::Submission` passed here into its `Attempt` (the association's real
63-
# target). Several question helpers accept either a `Submission` or an `Attempt` as their
64-
# "submission" argument and thread it straight into `Answer::<Type>.new(submission: ...)`; without
65-
# this coercion, passing the `Submission` raises `ActiveRecord::AssociationTypeMismatch`, since
66-
# `belongs_to`'s writer strictly checks `record.is_a?(reflection.klass)`. Coercing here keeps every
67-
# caller working without a per-call-site change.
68-
def submission=(value)
69-
value = value.attempt if value.is_a?(Course::Assessment::Submission)
70-
super
71-
end
64+
# `attempt` is the accurate name for what `:submission` returns — the Attempt base record. Prefer it in
65+
# new code: `answer.attempt.submission` reads clearly where `answer.submission.submission` stuttered (the
66+
# FK column is misleadingly named `submission_id`). Reader-only alias; the association stays `:submission`
67+
# for existing call sites.
68+
alias_method :attempt, :submission
7269
belongs_to :grader, class_name: 'User', inverse_of: nil, optional: true
7370
has_one :auto_grading, class_name: 'Course::Assessment::Answer::AutoGrading',
7471
dependent: :destroy, inverse_of: :answer, autosave: true

app/models/course/assessment/answer/programming_ability.rb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@ def allow_create_programming_files
1313
can :create_programming_files, Course::Assessment::Answer::Programming do |programming_answer|
1414
multiple_file_submission?(programming_answer.question) &&
1515
creator?(programming_answer.submission) &&
16-
can_update_submission?(programming_answer.submission.submission) &&
16+
can_update_submission?(programming_answer.attempt.submission) &&
1717
current_answer?(programming_answer)
1818
end
1919
end
@@ -22,7 +22,7 @@ def allow_destroy_programming_files
2222
can :destroy_programming_file, Course::Assessment::Answer::Programming do |programming_answer|
2323
multiple_file_submission?(programming_answer.question) &&
2424
creator?(programming_answer.submission) &&
25-
can_update_submission?(programming_answer.submission.submission) &&
25+
can_update_submission?(programming_answer.attempt.submission) &&
2626
current_answer?(programming_answer)
2727
end
2828
end

app/models/course/assessment/question_bundle_assignment.rb

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5,21 +5,13 @@ class Course::Assessment::QuestionBundleAssignment < ApplicationRecord
55
foreign_key: :assessment_id, inverse_of: :question_bundle_assignments
66
belongs_to :submission, class_name: 'Course::Assessment::Attempt', optional: true,
77
foreign_key: :submission_id, inverse_of: :question_bundle_assignments
8+
include Course::Assessment::CoercesSubmissionToAttempt
9+
810
belongs_to :question_bundle, class_name: 'Course::Assessment::QuestionBundle',
911
foreign_key: :bundle_id, inverse_of: :question_bundle_assignments
1012

1113
validate :submission_belongs_to_assessment_and_user
1214

13-
# Coerce a `Course::Assessment::Submission` passed here into its `Attempt` (the association's
14-
# real target post-repoint) — mirrors the same coercion on `Course::Assessment::Answer`/
15-
# `Course::Assessment::SubmissionQuestion` (see `Answer#submission=` for the full rationale).
16-
# `spec/controllers/course/assessment/submission/submissions_controller_spec.rb`'s
17-
# `randomized_submission` fixture passes the `Submission` half directly.
18-
def submission=(value)
19-
value = value.attempt if value.is_a?(Course::Assessment::Submission)
20-
super
21-
end
22-
2315
private
2416

2517
def submission_belongs_to_assessment_and_user

app/models/course/assessment/submission.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -225,7 +225,7 @@ def self.on_dependent_status_change(answer)
225225
# `answer.submission` resolves to an Attempt (the association name is `:submission`, its
226226
# `class_name` is Attempt). `last_graded_time` is a course-coupled column that lives only on the
227227
# real Submission, reached via the attempt's `has_one :submission`.
228-
answer.submission.submission&.last_graded_time = Time.now
228+
answer.attempt.submission&.last_graded_time = Time.now
229229
end
230230

231231
# Returns an array of submission rows for the given students and assessments.

app/models/course/assessment/submission_question.rb

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,20 +12,15 @@ class Course::Assessment::SubmissionQuestion < ApplicationRecord
1212
# since the association name stays `submission`.
1313
belongs_to :submission, class_name: 'Course::Assessment::Attempt', foreign_key: 'submission_id',
1414
inverse_of: :submission_questions
15+
include Course::Assessment::CoercesSubmissionToAttempt
16+
1517
belongs_to :question, class_name: 'Course::Assessment::Question',
1618
inverse_of: :submission_questions
1719

18-
# Coerce a `Course::Assessment::Submission` passed here into its `Attempt` (the association's
19-
# real target post-repoint) — mirrors the same coercion on `Course::Assessment::Answer` (see its
20-
# comment for the full rationale). Both `spec/factories/course_assessment_submission_questions.rb`'s
21-
# own default `submission { create(:submission, ...) }` and
22-
# `spec/models/course/assessment/submission_spec.rb`'s own `create(:course_assessment_submission_question,
23-
# submission: submission, ...)` pass the `Submission` half, which otherwise raises
24-
# `ActiveRecord::AssociationTypeMismatch`.
25-
def submission=(value)
26-
value = value.attempt if value.is_a?(Course::Assessment::Submission)
27-
super
28-
end
20+
# `attempt` is the accurate name for what `:submission` returns — the Attempt base record. Prefer it in
21+
# new code (e.g. `@submission_question.attempt.submission`); the association stays `:submission` for
22+
# existing call sites. Reader-only alias.
23+
alias_method :attempt, :submission
2924

3025
has_many :threads, class_name: 'Course::Assessment::LiveFeedback::Thread',
3126
inverse_of: :submission_question, dependent: :destroy

app/services/course/assessment/answer/ai_generated_post_service.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ def create_topic_subscription(discussion_topic)
8080
# Ensure the student who wrote the answer amd all group managers
8181
# gets notified when someone comments on his answer
8282
discussion_topic.ensure_subscribed_by(@answer.submission.creator)
83-
answer_course_user = @answer.submission.submission.course_user
83+
answer_course_user = @answer.attempt.submission.course_user
8484
answer_course_user.my_managers.each do |manager|
8585
discussion_topic.ensure_subscribed_by(manager.user)
8686
end

0 commit comments

Comments
 (0)