De-weird-ish contest + contest_relation

app/models/contest.rb

@@ -27,14 +27,9 @@ class Contest < ApplicationRecord
   scope :not_ended, -> { where("end_time > ?", Time.current) }
   scope :publicly_observable, -> { where(observation: OBSERVATION[:public]) }
 
-  before_save do # update the end time that was cached
-    if duration_changed? || end_time_changed?
-      contest_relations.find_each do |relation|
-        relation.finish_at = [end_time, relation.started_at.advance(hours: duration.to_f)].min.advance(seconds: relation.extra_time) unless relation.started_at.nil?
-        relation.save
-      end
-    end
+  after_save :update_contest_relations
 
+  after_save do
     update_contest_scores if finalized_at_was && finalized_at.nil?
     true
   end
@@ -172,4 +167,10 @@ def startcode=(code)
   def max_extra_time
     (duration * 3600).to_i
   end
+
+  def update_contest_relations
+    return unless duration_changed? || end_time_changed?
+
+    contest_relations.where.not(started_at: nil).find_each(&:set_finish_at!)
+  end
 end

app/models/contest_relation.rb

@@ -104,4 +104,9 @@ def recalculate_contest_scores_and_save
   after_save do
     recalculate_contest_scores_and_save if contest.finalized_at.nil? && extra_time_changed?
   end
+
+  def set_finish_at!
+    new_finish_at = [contest.end_time, started_at + contest.duration.hours].min
+    update!(finish_at: new_finish_at + extra_time.seconds)
+  end
 end

spec/models/contest_spec.rb

@@ -0,0 +1,135 @@
+require "spec_helper"
+
+describe Contest do
+  include ActiveSupport::Testing::TimeHelpers
+
+  let(:user) { FactoryBot.create(:user) }
+  let(:contest) { FactoryBot.create(:contest, duration: 3) }
+  let(:relation) { ContestRelation.create!(contest: contest, user: user) }
+
+  context "when the contest end_time is updated" do
+    context "when the contest can be completed within the window" do
+      before do
+        travel_to contest.start_time + 5.minutes
+
+        relation.start!
+      end
+
+      it "doesn't update contest_relation#finish_at" do
+        expect {
+          contest.update!(end_time: contest.end_time + 1.hour)
+        }.not_to change {
+          relation.reload.finish_at
+        }
+      end
+    end
+
+    context "when the competitor started late and has a shortened window" do
+      before do
+        travel_to contest.end_time - 1.hour
+
+        relation.start!
+      end
+
+      context "when the contest is extended by a small amount" do
+        it "updates contest_relation#finish_at" do
+          expect {
+            contest.update!(end_time: contest.end_time + 1.hour)
+          }.to change {
+            relation.reload.finish_at
+          }.by(1.hour)
+        end
+      end
+
+      context "when the contest is extended by a huge amount" do
+        it "updates contest_relation#finish_at to the maximum duration" do
+          expect {
+            contest.update!(end_time: contest.end_time + 10.weeks)
+          }.to change {
+            relation.reload.finish_at
+          }.by(2.hours)
+        end
+      end
+    end
+  end
+
+  context "when the contest can be completed within the window" do
+    before do
+      travel_to contest.start_time + 5.minutes
+    end
+
+    context "when the contest duration is increased" do
+      before do
+        relation.start!
+      end
+
+      it "gives the competitor more time" do
+        expect {
+          contest.update!(duration: 7)
+        }.to change {
+          relation.reload.finish_at
+        }.by(4.hours)
+      end
+    end
+
+    context "when the contest duration is decreased" do
+      before do
+        relation.start!
+      end
+
+      it "gives the competitor less time" do
+        expect {
+          contest.update!(duration: 2)
+        }.to change {
+          relation.reload.finish_at
+        }.by(-1.hours)
+      end
+    end
+  end
+
+  context "when the competitor started late and has a shortened window" do
+    before do
+      travel_to contest.end_time - 1.hour
+
+      relation.start!
+    end
+
+    context "when the contest duration is increased" do
+      it "gives the competitor no additional time" do
+        expect {
+          contest.update!(duration: 10.hours)
+        }.not_to change {
+          relation.reload.finish_at
+        }
+      end
+    end
+
+    context "when the contest duration is decreased" do
+      it "gives the competitor no additional time" do
+        expect {
+          contest.update!(duration: 3.hours)
+        }.not_to change {
+          relation.reload.finish_at
+        }
+      end
+    end
+  end
+
+  context "when the competitor started late but has full time" do
+    before do
+      travel_to contest.end_time - 4.hour
+
+      relation.start!
+    end
+
+    context "when the contest duration is increased" do
+      it "gives the competitor more time, but not the full duration" do
+        expect {
+          contest.update!(duration: 10.hours)
+        }.to change {
+          relation.reload.finish_at
+        }.by(1.hour)
+      end
+    end
+  end
+end