Merge dae593f into 8341134

app/controllers/student_teams_controller.rb

@@ -51,11 +51,11 @@ def view
     @send_invs = Invitation.where from_id: student.user.id, assignment_id: student.assignment.id
     @received_invs = Invitation.where to_id: student.user.id, assignment_id: student.assignment.id, reply_status: 'W'
 
-    @current_due_date = DueDate.current_due_date(@student.assignment.due_dates)
+    @current_due_date = @student.assignment.current_due_date
 
     # this line generates a list of users on the waiting list for the topic of a student's team,
     @users_on_waiting_list = (SignUpTopic.find(@student.team.topic).users_on_waiting_list if student_team_requirements_met?)
-    @teammate_review_allowed = DueDate.teammate_review_allowed(@student)
+    @teammate_review_allowed = @student.teammate_review_allowed
   end
 
   def create

app/helpers/assignment_helper.rb

@@ -66,9 +66,9 @@ def due_date(assignment, type, round = 0)
       due_date = AssignmentDueDate.new
       due_date.deadline_type_id = DeadlineType.find_by(name: type).id
       # creating new round
-      due_date.submission_allowed_id = AssignmentDueDate.default_permission(type, 'submission_allowed')
-      due_date.review_allowed_id = AssignmentDueDate.default_permission(type, 'can_review')
-      due_date.review_of_review_allowed_id = AssignmentDueDate.default_permission(type, 'review_of_review_allowed')
+      due_date.submission_allowed_id = due_date.default_permission(type, 'submission_allowed')
+      due_date.review_allowed_id = due_date.default_permission(type, 'can_review')
+      due_date.review_of_review_allowed_id = due_date.default_permission(type, 'review_of_review_allowed')
       due_date
     else
       due_dates[round]

app/models/assignment.rb

@@ -190,7 +190,7 @@ def path
   # The permissions of TopicDueDate is the same as AssignmentDueDate.
   # Here, column is usually something like 'review_allowed_id'
   def check_condition(column, topic_id = nil)
-    next_due_date = DueDate.get_next_due_date(id, topic_id)
+    next_due_date = get_next_due_date(id, topic_id)
     return false if next_due_date.nil?
 
     right_id = next_due_date.send column
@@ -297,7 +297,7 @@ def create_node
   # if current  stage is submission or review, find the round number
   # otherwise, return 0
   def number_of_current_round(topic_id)
-    next_due_date = DueDate.get_next_due_date(id, topic_id)
+    next_due_date = get_next_due_date(id, topic_id)
     return 0 if next_due_date.nil?
 
     next_due_date.round ||= 0
@@ -351,7 +351,7 @@ def num_review_rounds
   end
 
   def find_current_stage(topic_id = nil)
-    next_due_date = DueDate.get_next_due_date(id, topic_id)
+    next_due_date = get_next_due_date(id, topic_id)
     return 'Finished' if next_due_date.nil?
 
     next_due_date
@@ -369,7 +369,7 @@ def current_stage(topic_id = nil)
   def review_questionnaire_id(round_number = nil, topic_id = nil)
     # If round is not given, try to retrieve current round from the next due date
     if round_number.nil?
-      next_due_date = DueDate.get_next_due_date(id)
+      next_due_date = get_next_due_date(id)
       round_number = next_due_date.try(:round)
     end
     # Create assignment_form that we can use to retrieve AQ with all the same attributes and questionnaire based on AQ
@@ -622,6 +622,39 @@ def pair_programming_enabled?
     self.enable_pair_programming
   end
 
+  def get_next_due_date(assignment_id, topic_id = nil)
+    assignment = Assignment.find(assignment_id)
+    if assignment.staggered_deadline?
+      next_due_date = TopicDueDate.find_by(['parent_id = ? and due_at >= ?', topic_id, Time.zone.now])
+      upcoming_due_dates_after_topic_date = assignment.upcoming_due_dates_after_topic_date(topic_id)
+      # if certion TopicDueDate is not exist, we should query next corresponding AssignmentDueDate.
+      # eg. Time.now is 08/28/2016
+      # One topic uses following deadlines:
+      # TopicDueDate      08/01/2016
+      # TopicDueDate      08/02/2016
+      # TopicDueDate      08/03/2016
+      # AssignmentDueDate 09/04/2016
+      # In this case, we cannot find due_at later than Time.now in TopicDueDate.
+      # So we should find next corresponding AssignmentDueDate, starting with the 4th one, not the 1st one!
+      if next_due_date.nil? && upcoming_due_dates_after_topic_date
+        next_due_date = upcoming_due_dates_after_topic_date.detect { |assignment_due_date| assignment_due_date.due_at >= Time.zone.now }
+      end
+    else
+      next_due_date = AssignmentDueDate.find_by(['parent_id = ? && due_at >= ?', assignment_id, Time.zone.now])
+    end
+    next_due_date
+  end
+
+  # Create separate method to get next dates after topic dates to follow single responsibility principle
+  def upcoming_due_dates_after_topic_date(topic_id = nil)
+    topic_due_date_size = TopicDueDate.where(parent_id: topic_id).size
+    upcoming_due_dates_after_topic_date = AssignmentDueDate.where(parent_id: id)[topic_due_date_size..-1]
+  end
+
+  def current_due_date
+    due_dates.detect { |due_date| due_date.due_at > Time.now }
+  end
+
   private
 
   # returns true if assignment has staggered deadline and topic_id is nil

app/models/deadline_type.rb

@@ -1,4 +1,4 @@
 class DeadlineType < ApplicationRecord
   has_many :assignment_due_dates, class_name: 'AssignmentDueDate', foreign_key: 'deadline_type_id'
   has_many :topic_due_dates, class_name: 'TopicDueDate', foreign_key: 'deadline_type_id'
-end
+end

app/models/due_date.rb

@@ -1,47 +1,12 @@
+# Class to manage Due Dates of Assignments & Signup Sheet Topics
 class DueDate < ApplicationRecord
-  validate :due_at_is_valid_datetime
+  validates :due_at, presence: true, if: -> { :due_at.to_s.is_a?(DateTime) }
   #  has_paper_trail
 
-  def self.default_permission(deadline_type, permission_type)
+  def default_permission(deadline_type, permission_type)
     DeadlineRight::DEFAULT_PERMISSION[deadline_type][permission_type]
   end
 
-  def self.current_due_date(due_dates)
-    # Get the current due date from list of due dates
-    due_dates.each do |due_date|
-      if due_date.due_at > Time.now
-        current_due_date = due_date
-        return current_due_date
-      end
-    end
-    # in case current due date not found
-    nil
-  end
-
-  def self.teammate_review_allowed(student)
-    # time when teammate review is allowed
-    due_date = current_due_date(student.assignment.due_dates)
-    student.assignment.find_current_stage == 'Finished' ||
-      due_date &&
-        (due_date.teammate_review_allowed_id == 3 ||
-        due_date.teammate_review_allowed_id == 2) # late(2) or yes(3)
-  end
-
-  def set_flag
-    self.flag = true
-    save
-  end
-
-  def due_at_is_valid_datetime
-    if due_at.present?
-      errors.add(:due_at, 'must be a valid datetime') if (begin
-                                                            DateTime.strptime(due_at.to_s, '%Y-%m-%d %H:%M:%S')
-                                                          rescue StandardError
-                                                            ArgumentError
-                                                          end) == ArgumentError
-    end
-  end
-
   def self.copy(old_assignment_id, new_assignment_id)
     duedates = where(parent_id: old_assignment_id)
     duedates.each do |orig_due_date|
@@ -51,70 +16,13 @@ def self.copy(old_assignment_id, new_assignment_id)
     end
   end
 
-  def self.set_duedate(duedate, deadline, assign_id, max_round)
-    submit_duedate = DueDate.new(duedate)
-    submit_duedate.deadline_type_id = deadline
-    submit_duedate.parent_id = assign_id
-    submit_duedate.round = max_round
-    submit_duedate.save
-  end
-
-  def self.deadline_sort(due_dates)
-    due_dates.sort do |m1, m2|
-      if m1.due_at && m2.due_at
-        m1.due_at <=> m2.due_at
-      elsif m1.due_at
-        -1
-      else
-        1
-      end
-    end
-  end
-
-  def self.done_in_assignment_round(assignment_id, response)
-    # for author feedback, quiz, teammate review and metareview, Expertiza only support one round, so the round # should be 1
-    return 0 if ResponseMap.find(response.map_id).type != 'ReviewResponseMap'
-
-    due_dates = DueDate.where(parent_id: assignment_id)
-    # sorted so that the earliest deadline is at the first
-    sorted_deadlines = deadline_sort(due_dates)
-    due_dates.reject { |due_date| due_date.deadline_type_id != 1 && due_date.deadline_type_id != 2 }
-    round = 1
-    sorted_deadlines.each do |due_date|
-      break if response.created_at < due_date.due_at
-
-      round += 1 if due_date.deadline_type_id == 2
-    end
-    round
-  end
-
-  def self.get_next_due_date(assignment_id, topic_id = nil)
-    if Assignment.find(assignment_id).staggered_deadline?
-      next_due_date = TopicDueDate.find_by(['parent_id = ? and due_at >= ?', topic_id, Time.zone.now])
-      # if certion TopicDueDate is not exist, we should query next corresponding AssignmentDueDate.
-      # eg. Time.now is 08/28/2016
-      # One topic uses following deadlines:
-      # TopicDueDate      08/01/2016
-      # TopicDueDate      08/02/2016
-      # TopicDueDate      08/03/2016
-      # AssignmentDueDate 09/04/2016
-      # In this case, we cannot find due_at later than Time.now in TopicDueDate.
-      # So we should find next corresponding AssignmentDueDate, starting with the 4th one, not the 1st one!
-      if next_due_date.nil?
-        topic_due_date_size = TopicDueDate.where(parent_id: topic_id).size
-        following_assignment_due_dates = AssignmentDueDate.where(parent_id: assignment_id)[topic_due_date_size..-1]
-        unless following_assignment_due_dates.nil?
-          following_assignment_due_dates.each do |assignment_due_date|
-            if assignment_due_date.due_at >= Time.zone.now
-              next_due_date = assignment_due_date
-              break
-            end
-          end
-        end
-      end
+  def <=>(other)
+    if due_at && other.due_at
+      due_at <=> other.due_at
+    elsif due_at
+      -1
     else
-      next_due_date = AssignmentDueDate.find_by(['parent_id = ? && due_at >= ?', assignment_id, Time.zone.now])
+      1
     end
-    next_due_date
   end
 end

app/models/participant.rb

@@ -118,6 +118,15 @@ def authorization
     authorization
   end
 
+  def teammate_review_allowed
+    # time when teammate review is allowed
+    due_date = assignment.current_due_date
+    assignment.find_current_stage == 'Finished' ||
+      due_date &&
+        (due_date.teammate_review_allowed_id == 3 ||
+        due_date.teammate_review_allowed_id == 2) # late(2) or yes(3)
+  end
+
   # Sort a set of participants based on their user names.
   # Please make sure there is no duplicated participant in this input array.
   # There should be a more beautiful way to handle this, though.  -Yang

app/models/response_map.rb

@@ -101,4 +101,18 @@ def find_team_member
     end
     team
   end
+
+  def assignment_latest_review_round(assignment_id)
+    # for author feedback, quiz, teammate reviews, rounds # should be 1
+    maps = ResponseMap.where(id: map_id, type: 'ReviewResponseMap')
+    return 0 if maps.empty?
+
+    # sorted so that the earliest deadline is at the first
+    sorted_deadlines = DueDate.where(parent_id: assignment_id).sort
+    round = 1
+    sorted_deadlines.each do |due_date|
+      round += 1 if due_date.deadline_type_id == 2 && created_at >= due_date.due_at
+    end
+    round
+  end
 end

app/views/shared/responses/_response_actions.html.erb

@@ -10,7 +10,7 @@
     <% @latest_response = @sorted_responses.last %>
 
     <% if @latest_response.round.nil? %>
-      <% last_response_round = AssignmentDueDate.done_in_assignment_round(@assignment.id, @latest_response) %>
+      <% last_response_round = @latest_response.assignment_latest_review_round(@assignment.id) %>
     <% else %>
       <% last_response_round = @latest_response.round %>
     <% end %>

app/views/sign_up_sheet/review_bids_others_work.html.erb

@@ -62,7 +62,7 @@
                     <% @latest_response = @sorted_responses.last %>
 
                     <%if @latest_response.round.nil?%>
-                      <% last_response_round = AssignmentDueDate.done_in_assignment_round(@assignment.id,@latest_response) %>
+                      <% last_response_round = @latest_response.assignment_latest_review_round(@assignment.id) %>
                     <%else%>
                       <% last_response_round = @latest_response.round %>
                     <%end%>

app/views/submitted_content/_self_review.html.erb

@@ -31,7 +31,7 @@
                 <% @latest_response = @sorted_responses.last %>
 
                 <% if @latest_response.round.nil? %>
-                    <% last_response_round = AssignmentDueDate.done_in_assignment_round(@assignment.id, @latest_response) %>
+                    <% last_response_round = @latest_response.assignment_latest_review_round(@assignment.id) %>
                 <% else %>
                     <% last_response_round = @latest_response.round %>
                 <% end %>

spec/lib/scoring_spec.rb

@@ -51,7 +51,7 @@
     context 'when current assignment does not vary rubrics by round' do
       it 'scores rubrics and returns review scores' do
         allow(assignment).to receive(:vary_by_round?).and_return(false)
-        allow(DueDate).to receive(:get_next_due_date).with(assignment.id).and_return(double(:DueDate, round: 1))
+        allow(assignment).to receive(:get_next_due_date).with(assignment.id).and_return(double(:DueDate, round: 1))
         expect(compute_reviews_hash(assignment)).to eq(1 => { 1 => 50 }, 2 => { 1 => 30 })
       end
     end

spec/models/assignment_spec.rb

@@ -248,15 +248,15 @@
   describe '#check_condition' do
     context 'when the next due date is nil' do
       it 'returns false ' do
-        allow(DueDate).to receive(:get_next_due_date).with(1, nil).and_return(nil)
+        allow(assignment).to receive(:get_next_due_date).with(1, nil).and_return(nil)
         expect(assignment.check_condition('review_allowed_id')).to be false
       end
     end
 
     context 'when the next due date is allowed to review submissions' do
       it 'returns true' do
         assignment_due_date = double('AssignmentDueDate')
-        allow(DueDate).to receive(:get_next_due_date).with(1, nil).and_return(assignment_due_date)
+        allow(assignment).to receive(:get_next_due_date).with(1, nil).and_return(assignment_due_date)
         allow(assignment_due_date).to receive(:send).with('review_allowed_id').and_return(1)
         allow(DeadlineRight).to receive(:find).with(1).and_return(double('DeadlineRight', name: 'OK'))
         expect(assignment.check_condition('review_allowed_id')).to be true
@@ -392,14 +392,14 @@
   describe '#number_of_current_round' do
     context 'when next_due_date is nil' do
       it 'returns 0' do
-        allow(DueDate).to receive(:get_next_due_date).with(1, 1).and_return(nil)
+        allow(assignment).to receive(:get_next_due_date).with(1, 1).and_return(nil)
         expect(assignment.number_of_current_round(1)).to eq(0)
       end
     end
 
     context 'when next_due_date is not nil' do
       it 'returns the round of next_due_date' do
-        allow(DueDate).to receive(:get_next_due_date).with(1, 1).and_return(double('DueDate', round: 2))
+        allow(assignment).to receive(:get_next_due_date).with(1, 1).and_return(double('DueDate', round: 2))
         expect(assignment.number_of_current_round(1)).to eq(2)
       end
     end
@@ -501,14 +501,14 @@
   describe '#find_current_stage' do
     context 'when next due date is nil' do
       it 'returns Finished' do
-        allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(nil)
+        allow(assignment).to receive(:get_next_due_date).with(1, 123).and_return(nil)
         expect(assignment.find_current_stage(123)).to eq('Finished')
       end
     end
 
     context 'when next due date is nil' do
       it 'returns next due date object' do
-        allow(DueDate).to receive(:get_next_due_date).with(1, 123).and_return(assignment_due_date)
+        allow(assignment).to receive(:get_next_due_date).with(1, 123).and_return(assignment_due_date)
         expect(assignment.find_current_stage(123)).to eq(assignment_due_date)
       end
     end
@@ -540,7 +540,7 @@
       end
 
       it 'returns correct questionnaire id found by topic_id if only topic_id is given and there is no current round used in the due date' do
-        allow(DueDate).to receive(:get_next_due_date).with(assignment.id).and_return(nil)
+        allow(assignment).to receive(:get_next_due_date).with(assignment.id).and_return(nil)
         allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: assignment.id, used_in_round: nil, topic_id: 1).and_return(
           [assignment_questionnaire1]
         )
@@ -549,7 +549,7 @@
       end
 
       it 'returns correct questionnaire id found by used_in_round and topic_id if only topic_id is given, but current round is found by the due date' do
-        allow(DueDate).to receive(:get_next_due_date).with(assignment.id).and_return(assignment_due_date)
+        allow(assignment).to receive(:get_next_due_date).with(assignment.id).and_return(assignment_due_date)
         allow(AssignmentQuestionnaire).to receive(:where).with(assignment_id: assignment.id, used_in_round: 1, topic_id: 1).and_return(
           [assignment_questionnaire1]
         )