Merge pull request #155 from stormbreakerbg/order-challenge-solutions

Order challenge solutions chronologically

app/controllers/challenges_controller.rb

@@ -24,9 +24,16 @@ def create
   end
 
   def show
-    @challenge = Challenge.find_with_solutions_and_users params[:id]
+    @challenge = Challenge.find params[:id]
+
+    if @challenge.hidden? and not admin?
+      deny_access
+      return
+    end
 
-    deny_access if @challenge.hidden? and not admin?
+    if @challenge.closed? or admin?
+      @solutions = ChallengeSolution.for_challenge_with_users params[:id]
+    end
   end
 
   def edit

app/models/challenge.rb

@@ -11,10 +11,6 @@ def in_chronological_order
     def visible
       where(hidden: false).in_chronological_order
     end
-
-    def find_with_solutions_and_users(challenge_id)
-      includes(solutions: :user).find(challenge_id)
-    end
   end
 
   def closed?

app/models/challenge_solution.rb

@@ -2,11 +2,17 @@ class ChallengeSolution < ActiveRecord::Base
   belongs_to :user
   belongs_to :challenge
 
+  scope :in_chronological_order, -> { order 'created_at ASC' }
+
   class << self
     def for(challenge, user)
       where(challenge_id: challenge.id, user_id: user.id).first
     end
 
+    def for_challenge_with_users(challenge_id)
+      in_chronological_order.where(challenge_id: challenge_id).includes(:user)
+    end
+
     def correct?(passed_tests, failed_tests)
       !!(passed_tests.nonzero? and failed_tests.zero?)
     end

app/views/challenges/show.html.haml

@@ -26,10 +26,10 @@
 
   = markup @challenge.description, auto_link: false
 
-  - if @challenge.closed? or admin?
+  - if @solutions
     %h2 Решения
     .solutions
-      - @challenge.solutions.each do |solution|
+      - @solutions.each do |solution|
         %article
           %header
             .thumbnail= link_to user_thumbnail(solution.user, :size80), solution.user

spec/controllers/challenges_controller_spec.rb

@@ -94,10 +94,12 @@
     log_in_as :student
 
     let(:challenge) { double }
+    let(:solutions) { [double, double] }
 
     before do
-      Challenge.stub find_with_solutions_and_users: challenge
-      challenge.stub hidden?: false
+      Challenge.stub find: challenge
+      ChallengeSolution.stub for_challenge_with_users: solutions
+      challenge.stub hidden?: false, closed?: false
     end
 
     it "does not requrie a logged in user" do
@@ -107,7 +109,7 @@
     end
 
     it "looks up the challenge by id" do
-      Challenge.should_receive(:find_with_solutions_and_users).with('42')
+      Challenge.should_receive(:find).with('42')
       get :show, id: '42'
     end
 
@@ -116,6 +118,33 @@
       assigns(:challenge).should eq challenge
     end
 
+    it "does not assign the solutions" do
+      get :show, id: '1'
+      assigns(:solutions).should be_nil
+    end
+
+    context "when closed" do
+      log_in_as :student
+
+      before do
+        challenge.stub closed?: true
+      end
+
+      it "assigns the solutions" do
+        get :show, id: '1'
+        assigns(:solutions).should eq solutions
+      end
+    end
+
+    context "when the user is admin" do
+      log_in_as :admin
+
+      it "assigns the solutions" do
+        get :show, id: '1'
+        assigns(:solutions).should eq solutions
+      end
+    end
+
     context "when hidden" do
       log_in_as :student
 

spec/models/challenge_solution_spec.rb

@@ -11,6 +11,31 @@
     ChallengeSolution.for(challenge, another_user).should be_nil
   end
 
+  it "can retrieve solutions in chronological order" do
+    second = create :challenge_solution, created_at: 1.day.ago
+    first  = create :challenge_solution, created_at: 2.days.ago
+
+    ChallengeSolution.in_chronological_order.should eq [first, second]
+  end
+
+  describe '#for_challenge_with_users' do
+    before do
+      @second_solution    = create :challenge_solution, challenge_id: 1, user_id: 1, created_at: 1.day.ago
+      @first_solution     = create :challenge_solution, challenge_id: 1, user_id: 2, created_at: 2.day.ago
+      @unrelated_solution = create :challenge_solution, challenge_id: 2, user_id: 3, created_at: 1.day.ago
+    end
+
+    subject(:solutions) { ChallengeSolution.for_challenge_with_users(1) }
+
+    it "retrieves all solutions by challenge id" do
+      solutions.should match_array [@first_solution, @second_solution]
+    end
+
+    it "retrieves solutions chronologically" do
+      solutions.should eq [@first_solution, @second_solution]
+    end
+  end
+
   it "classifies a solution as correct only if it has run and all tests pass" do
     ChallengeSolution.correct?(1, 0).should be_true