E2566 Finish Due Dates

app/models/assignment.rb

@@ -12,7 +12,7 @@ class Assignment < ApplicationRecord
   has_many :response_maps, foreign_key: 'reviewed_object_id', dependent: :destroy, inverse_of: :assignment
   has_many :review_mappings, class_name: 'ReviewResponseMap', foreign_key: 'reviewed_object_id', dependent: :destroy, inverse_of: :assignment
   has_many :sign_up_topics , class_name: 'SignUpTopic', foreign_key: 'assignment_id', dependent: :destroy
-  # Note: due_dates association is provided by DueDateActions mixin
+  has_many :due_dates, as: :parent, dependent: :destroy
   belongs_to :course, optional: true
   belongs_to :instructor, class_name: 'User', inverse_of: :assignments
 

app/models/concerns/due_date_actions.rb

@@ -48,17 +48,31 @@ def drop_topic_permissible?
     activity_permissible?(:drop_topic)
   end
 
-  # Get the next due date for this parent
+  # Get the next due date for this assignment
+  #
+  # This method abstracts away whether the assignment has topic-specific deadlines
+  # or assignment-level deadlines. The caller doesn't need to know the implementation
+  # details, they just ask for the next due date and get the appropriate one.
+  #
+  # @param topic_id [Integer, nil] Optional topic ID. If provided and the assignment
+  #   has topic-specific deadlines, returns the next deadline for that topic.
+  #   If the topic has no upcoming deadlines, falls back to assignment-level deadlines.
+  # @return [DueDate, nil] The next upcoming due date, or nil if none exist
   def next_due_date(topic_id = nil)
+    # If a topic is specified and this assignment has topic-specific deadlines,
+    # look for topic due dates first
     if topic_id && has_topic_specific_deadlines?
       topic_deadline = due_dates.where(parent_id: topic_id, parent_type: 'ProjectTopic')
-                               .where('due_at >= ?', Time.current)
-                               .order(:due_at)
+                               .upcoming
                                .first
       return topic_deadline if topic_deadline
     end
 
-    due_dates.where('due_at >= ?', Time.current).order(:due_at).first
+    # Fall back to assignment-level due dates
+    # This handles both cases:
+    # 1. No topic specified
+    # 2. Topic specified but no topic-specific deadlines found
+    due_dates.upcoming.first
   end
 
   # Get the current stage name for display purposes

app/models/concerns/due_date_permissions.rb

@@ -2,8 +2,19 @@
 
 module DueDatePermissions
   # Permission checking methods that combine deadline-based and role-based logic
-  # These methods also need to check the can_submit, can_review, etc. fields of the Participant object
-
+  #
+  # As Ed explained, these methods must check both:
+  # 1. Whether the action is permitted by the current deadline (submission_allowed_id, review_allowed_id, etc.)
+  # 2. Whether the participant has the necessary permissions (can_submit, can_review, can_take_quiz fields)
+  #
+  # The participant object represents how a user is participating in the assignment.
+  # Not all participants can do all actions - some might only submit, others only review,
+  # and others might only take quizzes. The can_* fields control these permissions.
+
+  # Check if submission is allowed based on both deadline and participant permissions
+  # @param participant [Participant, nil] The participant to check permissions for.
+  #   If nil, only checks deadline-based permissions.
+  # @return [Boolean] true if submission is allowed, false otherwise
   def can_submit?(participant = nil)
     return false unless submission_allowed_id
     return false if participant && !participant.can_submit
@@ -12,6 +23,10 @@ def can_submit?(participant = nil)
     deadline_right&.name&.in?(%w[OK Late])
   end
 
+  # Check if review is allowed based on both deadline and participant permissions
+  # @param participant [Participant, nil] The participant to check permissions for.
+  #   If nil, only checks deadline-based permissions.
+  # @return [Boolean] true if review is allowed, false otherwise
   def can_review?(participant = nil)
     return false unless review_allowed_id
     return false if participant && !participant.can_review
@@ -20,6 +35,10 @@ def can_review?(participant = nil)
     deadline_right&.name&.in?(%w[OK Late])
   end
 
+  # Check if taking a quiz is allowed based on both deadline and participant permissions
+  # @param participant [Participant, nil] The participant to check permissions for.
+  #   If nil, only checks deadline-based permissions.
+  # @return [Boolean] true if taking quiz is allowed, false otherwise
   def can_take_quiz?(participant = nil)
     return false unless quiz_allowed_id
     return false if participant && !participant.can_take_quiz
@@ -28,6 +47,11 @@ def can_take_quiz?(participant = nil)
     deadline_right&.name&.in?(%w[OK Late])
   end
 
+  # Check if teammate review is allowed based on both deadline and participant permissions
+  # Note: teammate review uses the can_review permission field
+  # @param participant [Participant, nil] The participant to check permissions for.
+  #   If nil, only checks deadline-based permissions.
+  # @return [Boolean] true if teammate review is allowed, false otherwise
   def can_review_teammate?(participant = nil)
     return false unless teammate_review_allowed_id
     return false if participant && !participant.can_review

app/models/due_date.rb

@@ -92,15 +92,9 @@ def copy(from_assignment_id, to_assignment_id)
 
 
 
-    def next_due_date(parent_id, topic_id = nil)
-      if topic_id
-        topic_deadline = where(parent_id: topic_id, parent_type: 'ProjectTopic')
-                         .upcoming.first
-        return topic_deadline if topic_deadline
-      end
-
-      where(parent_id: parent_id).upcoming.first
-    end
+    # This method is no longer needed as the functionality has been moved
+    # to the DueDateActions concern which provides a cleaner interface
+    # that doesn't require the caller to know about topic vs assignment due dates
   end
 
   private

app/models/topic_due_date.rb

@@ -1,15 +1,11 @@
 # frozen_string_literal: true
 
 class TopicDueDate < DueDate
-  def self.next_due_date(assignment_id, topic_id)
-    topic_deadline = where(parent_id: topic_id, parent_type: 'ProjectTopic')
-                     .where('due_at >= ?', Time.current)
-                     .order(:due_at)
-                     .first
-
-    topic_deadline || DueDate.where(parent_id: assignment_id, parent_type: 'Assignment')
-                             .where('due_at >= ?', Time.current)
-                             .order(:due_at)
-                             .first
-  end
+  # TopicDueDate is a subclass of DueDate that uses single table inheritance
+  # The 'type' field in the database will be automatically set to 'TopicDueDate'
+  # when instances of this class are created.
+  #
+  # This class inherits all functionality from DueDate and doesn't need
+  # any additional methods since the parent class and DueDateActions concern
+  # already handle topic-specific due date logic properly.
 end