(stale) Lws49/feat marketplace pr7 attempt base record

app/controllers/concerns/course/assessment/submission/koditsu/submissions_concern.rb

@@ -39,7 +39,7 @@ def submission_status_hash
   def process_all_submissions
     create_new_submissions_if_not_existing
 
-    @submission_hash = Course::Assessment::Submission.where(assessment: @assessment).to_h do |s|
+    @submission_hash = @assessment.submissions.to_h do |s|
       [s.creator_id, s]
     end
 
@@ -59,8 +59,10 @@ def process_submission(submission, cm_submission)
   end
 
   def create_new_submissions_if_not_existing
-    existing_submission_user_ids = Course::Assessment::Submission.where(assessment: @assessment).
-                                   pluck(:creator_id)
+    # `creator_id` lives on the base (`course_assessment_submissions`), not on Submission's own
+    # table. Unqualified, `pluck` is ambiguous — `acts_as :experience_points_record` also joins
+    # `course_experience_points_records`, which also has `creator_id`. Qualify explicitly.
+    existing_submission_user_ids = @assessment.submissions.pluck('course_assessment_submissions.creator_id')
     koditsu_submission_user_ids = @cu_submission_hash.keys.map { |creator, _| creator.id }
     user_ids_without_submission = koditsu_submission_user_ids - existing_submission_user_ids
 
@@ -77,8 +79,7 @@ def create_new_submissions_if_not_existing
 
   def create_new_submission_for(creator, course_user)
     User.with_stamper(creator) do
-      new_submission = @assessment.submissions.new(creator: creator,
-                                                   course_user: course_user)
+      new_submission = @assessment.build_submission(creator: creator, course_user: course_user)
       success = @assessment.create_new_submission(new_submission, course_user)
 
       raise ActiveRecord::Rollback unless success
@@ -96,7 +97,10 @@ def update_submission(cm_submission, state, submitted_at)
   end
 
   def process_submission_answers(submission, cm_submission)
-    answers = Course::Assessment::Answer.includes(:question).where(submission_id: cm_submission.id)
+    # `cm_submission` is a Submission (extension), whose own `id` is an independent serial, NOT the
+    # attempt id that `answers.submission_id` references. Use `cm_submission.attempt_id` (the
+    # extension's FK to its attempt) to find the answers.
+    answers = Course::Assessment::Answer.includes(:question).where(submission_id: cm_submission.attempt_id)
 
     build_answer_hash(answers)
 

app/controllers/course/assessment/assessments_controller.rb

@@ -459,7 +459,7 @@ def submissions
       if @assessment.submissions.loaded?
         @assessment.submissions.select { |s| s.creator_id == current_user.id }
       else
-        @assessment.submissions.where(creator_id: current_user.id)
+        @assessment.submissions.by_user(current_user)
       end
   end
 

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

@@ -1,5 +1,5 @@
 # frozen_string_literal: true
-class Course::Assessment::Submission::Answer::Programming::AnnotationsController < \
+class Course::Assessment::Submission::Answer::Programming::AnnotationsController <
   Course::Assessment::Submission::Answer::Programming::Controller
   include Signals::EmissionConcern
 
@@ -66,7 +66,7 @@ def create_topic_subscription
 
     # Ensure all group managers get a notification when someone adds a programming annotation
     # to the answer.
-    answer_course_user = @answer.submission.course_user
+    answer_course_user = @answer.submission.submission.course_user
     answer_course_user.my_managers.each do |manager|
       @discussion_topic.ensure_subscribed_by(manager.user)
     end

app/controllers/course/assessment/submission/submissions_controller.rb

@@ -41,7 +41,7 @@ def index
   def create # rubocop:disable Metrics/AbcSize
     authorize! :access, @assessment
 
-    existing_submission = @assessment.submissions.find_by(creator: current_user)
+    existing_submission = @assessment.submissions.by_user(current_user).first
     create_success_response(existing_submission) and return if existing_submission
 
     ActiveRecord::Base.transaction do
@@ -423,7 +423,10 @@ def course_user_ids
 
   def user_ids_without_submission
     existing_submissions = @assessment.submissions.by_users(course_user_ids.pluck(:user_id))
-    user_ids_with_submission = existing_submissions.pluck(:creator_id)
+    # `creator_id` lives on the base (`course_assessment_submissions`), not on Submission's own
+    # table. Unqualified, `pluck` is ambiguous — `acts_as :experience_points_record` also joins
+    # `course_experience_points_records`, which also has `creator_id`. Qualify explicitly.
+    user_ids_with_submission = existing_submissions.pluck('course_assessment_submissions.creator_id')
     course_user_ids.pluck(:user_id) - user_ids_with_submission
   end
 

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

@@ -34,7 +34,7 @@ def create_topic_subscription
     @discussion_topic.ensure_subscribed_by(@submission_question.submission.creator)
 
     # Ensure all group managers get a notification when someone comments on this submission question
-    submission_question_course_user = @submission_question.submission.course_user
+    submission_question_course_user = @submission_question.submission.submission.course_user
     submission_question_course_user.my_managers.each do |manager|
       @discussion_topic.ensure_subscribed_by(manager.user)
     end

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

@@ -3,7 +3,10 @@ class Course::Assessment::SubmissionQuestion::SubmissionQuestionsController < Co
   load_resource :assessment, class: 'Course::Assessment', through: :course, parent: false
 
   def all_answers
-    @submission = @assessment.submissions.find(all_answers_params[:submission_id])
+    # `all_answers_params[:submission_id]` is an Attempt id — the wire key `submissionId` carries
+    # the attempt's id, not the extension table's own id. Find by attempt, then navigate to the real
+    # Submission for `authorize!`, whose `can :read, ...Submission` rules match on subject class.
+    @submission = @assessment.attempts.find(all_answers_params[:submission_id]).submission
     authorize!(:read, @submission)
     @submission_question = @submission.
                            submission_questions.

app/controllers/course/material/materials_controller.rb

@@ -44,9 +44,9 @@ def material_params
   def create_submission
     current_course_user = current_course.course_users.find_by(user: current_user)
     @assessment = @folder.owner
-    existing_submission = @assessment.submissions.find_by(creator: current_user)
+    existing_submission = @assessment.submissions.by_user(current_user).first
     unless existing_submission
-      @submission = @assessment.submissions.new(course_user: current_course_user)
+      @submission = @assessment.build_submission(course_user: current_course_user)
       @submission.session_id = authentication_service.generate_authentication_token
       success = @assessment.create_new_submission(@submission, current_user)
 

app/controllers/course/statistics/aggregate_controller.rb

@@ -65,14 +65,17 @@ def fetch_course_get_help_data(start_date, end_date)
     get_help_data = Course::Assessment::LiveFeedback::Message.find_by_sql(<<-SQL)
       SELECT DISTINCT ON (t.submission_creator_id, s.assessment_id, sq.question_id)
       m.id, m.content, m.created_at, t.submission_creator_id,
-      s.assessment_id, sq.submission_id, sq.question_id,
+      s.assessment_id, sub.id AS submission_id, sq.question_id,
       COUNT(*) OVER (
         PARTITION BY t.submission_creator_id, s.assessment_id, sq.question_id
       ) AS message_count
     FROM live_feedback_messages m
     INNER JOIN live_feedback_threads t ON m.thread_id = t.id
     INNER JOIN course_assessment_submission_questions sq ON t.submission_question_id = sq.id
     INNER JOIN course_assessment_submissions s ON sq.submission_id = s.id
+    -- `s.id` is the attempt's id (what the response's `submissionId` carries), which is a different
+    -- id space from the extension table's own serial `id`. Join to the extension via its `attempt_id`.
+    INNER JOIN course_assessment_submission_details sub ON sub.attempt_id = s.id
     INNER JOIN course_assessments a ON s.assessment_id = a.id
     INNER JOIN course_assessment_tabs tab ON a.tab_id = tab.id
     INNER JOIN course_assessment_categories cat ON tab.category_id = cat.id

app/controllers/course/statistics/assessments_controller.rb

@@ -21,7 +21,11 @@ def submission_statistics
                   includes(programming_questions: [:language]).
                   calculated(:maximum_grade, :question_count).
                   find(assessment_params[:id])
-    submissions = Course::Assessment::Submission.unscoped.
+    # `Course::Assessment::Submission` has no `assessment_id` column (it's Attempt-only, reached via
+    # the delegate), so querying it directly here raises `PG::UndefinedColumn`. `Attempt` carries
+    # `assessment_id`, `grade`, and `grader_ids` natively/as `calculated`, and this action's jbuilder
+    # only reads columns present on Attempt, so query Attempt directly.
+    submissions = Course::Assessment::Attempt.unscoped.
                   where(assessment_id: assessment_params[:id]).
                   calculated(:grade, :grader_ids)
     @course_users_hash = preload_course_users_hash(current_course)
@@ -39,7 +43,9 @@ def ancestor_statistics
                   calculated(:maximum_grade).
                   find(assessment_params[:id])
     authorize!(:read_ancestor, @assessment)
-    submissions = Course::Assessment::Submission.unscoped.
+    # Same `assessment_id`-column bug as `submission_statistics` above — `Attempt` is the
+    # drop-in replacement (see the comment there).
+    submissions = Course::Assessment::Attempt.unscoped.
                   preload(creator: :course_users).
                   where(assessment_id: assessment_params[:id]).
                   calculated(:grade)
@@ -52,7 +58,10 @@ def ancestor_statistics
   def live_feedback_statistics
     @assessment = Course::Assessment.unscoped.includes(:questions).
                   find(assessment_params[:id])
-    @submissions = Course::Assessment::Submission.unscoped.
+    # id/creator_id/workflow_state all live on Attempt; this action only reads those three columns
+    # (unscoped + a narrow .select), so querying Attempt directly is equivalent (every course-member
+    # attempt has exactly one submission).
+    @submissions = Course::Assessment::Attempt.unscoped.
                    select(:id, :creator_id, :workflow_state).
                    where(assessment_id: assessment_params[:id])
 
@@ -64,7 +73,7 @@ def live_feedback_statistics
 
   def live_feedback_history
     user_id = CourseUser.joins(:user).where(id: params[:course_user_id]).pluck('users.id').first
-    @submissions = Course::Assessment::Submission.where(assessment_id: assessment_params[:id], creator_id: user_id)
+    @submissions = Course::Assessment::Attempt.where(assessment_id: assessment_params[:id], creator_id: user_id)
     @question = Course::Assessment::Question.find(params[:question_id])
 
     create_submission_question_id_hash([@question])
@@ -238,7 +247,7 @@ def feedback_messages_cte(student_ids, submission_question_ids)
   def feedback_answers_cte
     <<-SQL
       SELECT
-        a.submission_id,
+        a.submission_id AS submission_id,
         a.question_id,
         a.created_at,
         a.grade,

app/controllers/system/admin/marketplace_access_blocks_controller.rb

@@ -0,0 +1,22 @@
+# frozen_string_literal: true
+class System::Admin::MarketplaceAccessBlocksController < System::Admin::Controller
+  def create
+    block = Course::Assessment::Marketplace::AccessBlock.new(
+      user_id: params[:user_id], creator: current_user
+    )
+    if block.save
+      render json: { id: block.id, userId: block.user_id }, status: :ok
+    else
+      render json: { errors: block.errors.full_messages.to_sentence }, status: :bad_request
+    end
+  end
+
+  def destroy
+    block = Course::Assessment::Marketplace::AccessBlock.find(params[:id])
+    if block.destroy
+      head :ok
+    else
+      render json: { errors: block.errors.full_messages.to_sentence }, status: :bad_request
+    end
+  end
+end

app/controllers/system/admin/marketplace_access_controller.rb

@@ -0,0 +1,8 @@
+# frozen_string_literal: true
+class System::Admin::MarketplaceAccessController < System::Admin::Controller
+  def index
+    query = Course::Assessment::Marketplace::AccessListQuery.new
+    @rows = query.rows
+    @summary = query.summary
+  end
+end

app/controllers/system/admin/marketplace_allowlist_rules_controller.rb

@@ -0,0 +1,52 @@
+# frozen_string_literal: true
+class System::Admin::MarketplaceAllowlistRulesController < System::Admin::Controller
+  # `preview` is a collection action with no id, so CanCan's default loader would try
+  # `find(params[:id])`. It builds its own unsaved rule; System::Admin::Controller's
+  # `authorize_admin` already gates the whole controller.
+  load_and_authorize_resource :allowlist_rule,
+                              class: 'Course::Assessment::Marketplace::AllowlistRule',
+                              parent: false, except: [:preview]
+
+  def index
+    # "Everyone" is a page-level mode, not a table row: expose its presence as `@everyone_rule`
+    # and show only the scoped rules in the table.
+    @everyone_rule = @allowlist_rules.rule_type_everyone.first
+    @allowlist_rules = @allowlist_rules.where.not(rule_type: :everyone).includes(:user, :instance)
+  end
+
+  def create
+    if @allowlist_rule.save
+      # `render partial:` (not `render 'rule'`) — the view is the `_rule` partial. Mirrors
+      # System::Admin::AnnouncementsController#create (`render partial: '.../announcement_data'`).
+      render partial: 'rule', locals: { rule: @allowlist_rule }, status: :ok
+    else
+      render json: { errors: @allowlist_rule.errors.full_messages.to_sentence }, status: :bad_request
+    end
+  end
+
+  def preview
+    rule = Course::Assessment::Marketplace::AllowlistRule.new(allowlist_rule_params)
+    unless rule.valid?
+      render json: { errors: rule.errors.full_messages.to_sentence }, status: :bad_request
+      return
+    end
+
+    query = Course::Assessment::Marketplace::RulePreviewQuery.new(rule)
+    @rows = query.rows
+    @summary = query.summary
+  end
+
+  def destroy
+    if @allowlist_rule.destroy
+      head :ok
+    else
+      render json: { errors: @allowlist_rule.errors.full_messages.to_sentence }, status: :bad_request
+    end
+  end
+
+  private
+
+  def allowlist_rule_params
+    params.require(:allowlist_rule).permit(:rule_type, :user_id, :instance_id, :email_domain, :email)
+  end
+end

app/helpers/system/admin/marketplace_access_helper.rb

@@ -0,0 +1,13 @@
+# frozen_string_literal: true
+module System::Admin::MarketplaceAccessHelper
+  # The value half of a rule's label ("Email domain · <value>"), for the audit list's reason column.
+  # @param [Course::Assessment::Marketplace::AllowlistRule] rule
+  # @return [String, nil]
+  def marketplace_rule_label_value(rule)
+    case rule.rule_type
+    when 'user' then rule.user&.name
+    when 'instance' then rule.instance&.name
+    when 'email_domain' then rule.email_domain
+    end
+  end
+end

app/jobs/course/assessment/submission/force_submit_timed_submission_job.rb

@@ -10,7 +10,11 @@ def perform_tracked(assessment, submission_id, submitter)
     instance = Course.unscoped { assessment.course.instance }
 
     ActsAsTenant.with_tenant(instance) do
-      submission = Course::Assessment::Submission.find_by(id: submission_id)
+      # `submission_id` here is the id `create_force_submission_job` (Attempt#create_force_submission_job)
+      # scheduled with, which is `attempt.id` — this job's own param name predates the split and is
+      # not renamed here (renaming it is exactly the kind of cosmetic diff the repo's diff-hygiene
+      # rule forbids without a functional reason).
+      submission = Course::Assessment::Attempt.find_by(id: submission_id)&.submission
       return unless submission
 
       force_submit(submission, submitter)

app/jobs/course/assessment/submission/force_submitting_job.rb

@@ -50,9 +50,7 @@ def force_create_and_submit_submissions(assessment, user_ids, user_ids_without_s
   # @param [Course::Assessment] assessment The assessment of which a submission is to be created.
   # @param [CourseUser] course_user The course user whose submission is to be created.
   def create_submission(assessment, course_user)
-    submission = assessment.submissions.new(creator: course_user.user, course_user: course_user)
-
-    assessment.submissions.new(creator: course_user.user)
+    submission = assessment.build_submission(creator: course_user.user, course_user: course_user)
     success = assessment.create_new_submission(submission, course_user)
 
     raise ActiveRecord::Rollback unless success

app/models/components/course/assessment_marketplace_ability_component.rb

@@ -4,12 +4,46 @@ module Course::AssessmentMarketplaceAbilityComponent
 
   def define_permissions
     allow_admins_publish_to_marketplace if user&.administrator?
-    allow_managers_access_marketplace if course_user&.manager_or_owner?
+    # System admins keep marketplace access via `can :manage, :all` (Ability#initialize); do not
+    # emit a `cannot` for them or it would revoke that. For everyone else, access is per-person.
+    if course && !user&.administrator?
+      if can_access_marketplace?
+        allow_managers_access_marketplace
+      else
+        # `Course::CourseAbilityComponent` grants managers/owners a blanket `can :manage, Course`,
+        # which (CanCan's `:manage` matches any action) would otherwise satisfy `:access_marketplace`
+        # regardless of the allow-list. This component runs after that one in the `define_permissions`
+        # super chain, so a `cannot` here takes precedence. This line is load-bearing.
+        cannot :access_marketplace, Course, id: course.id
+      end
+    end
     super
   end
 
   private
 
+  # Access is per-person, not per-current-course-role: anyone who is baseline-capable (manages/owns
+  # >=1 course anywhere, OR is an instructor/administrator in any instance) and passes the allow-list
+  # may browse, whatever their role in the course they are viewing.
+  def can_access_marketplace?
+    marketplace_baseline_capable? && marketplace_visible_to_user?
+  end
+
+  # The two peer baseline capabilities for the marketplace. Either qualifies; the allow-list narrows.
+  def marketplace_baseline_capable?
+    user&.course_manager_or_owner? || user&.instance_instructor_or_administrator?
+  end
+
+  # Part of the TEMPORARY allow-list gate (see the retirement seam on `can_access_marketplace?`).
+  # When the allow-list is retired this whole method is deleted; the block check goes with it.
+  def marketplace_visible_to_user?
+    return true if user&.administrator?
+
+    Course::Assessment::Marketplace::AllowlistRule.grants_access?(user) &&
+      !Course::Assessment::Marketplace::AccessBlock.blocked?(user)
+  end
+
+
   def allow_admins_publish_to_marketplace
     can :publish_to_marketplace, Course::Assessment
   end
@@ -23,4 +57,4 @@ def allow_managers_access_marketplace
       assessment.marketplace_listing&.published? || false
     end
   end
-end
+end

app/models/concerns/course/assessment/new_submission_concern.rb

@@ -12,7 +12,10 @@ def create_new_submission(new_submission, current_user)
           raise ActiveRecord::Rollback
         end
         raise ActiveRecord::Rollback unless new_submission.save
-        raise ActiveRecord::Rollback unless qbas.update_all(submission_id: new_submission.id)
+        # `question_bundle_assignments.submission_id` references the attempt base, so it must hold an
+        # Attempt id — not the extension table's own (different) id. `attempt_id` is the extension's
+        # own FK column to its attempt.
+        raise ActiveRecord::Rollback unless qbas.update_all(submission_id: new_submission.attempt_id)
 
         success = true
       end