show quiz results at top of quiz show page

This commit also moves the quiz results (who has/hasn't taken a quiz or
survey) to a table with the styleguide in place.

test plan:
  - As a teacher, create a quiz with an essay question.
  - While the quiz is stil unpublished, go to the quiz show page. it
    should look like it did before this patchset (e.g. dates visible,
    you can still find the edit/manage buttons, etc)
  - have some students in the course take the quiz, but make sure some
    students in the course haven't taken the quiz.
  - Click "Show Student Quiz Results" from the cog menu.
  - The list should look good, and display students under Taken/Haven't
    Taken when the list loads asynchronously.
  - Try different combinations of have/haven't taken including all taken and
    non taken
  - Try with an anonymous survey as well.

fixes CNVS-4381

Change-Id: Ida743125abaf836da5038fc7bc5f376597f47534
Reviewed-on: https://gerrit.instructure.com/18454
Reviewed-by: Simon Williams <simon@instructure.com>
Product-Review: Simon Williams <simon@instructure.com>
QA-Review: Amber Taniuchi <amber@instructure.com>
Tested-by: Jenkins <jenkins@instructure.com>

app/controllers/quizzes_controller.rb

@@ -202,17 +202,24 @@ class QuizzesController < ApplicationController
     if authorized_action(@quiz, @current_user, [:grade, :read_statistics])
       students = @context.students_visible_to(@current_user).order_by_sortable_name.to_a.uniq
       @submissions = @quiz.quiz_submissions.for_user_ids(students.map(&:id)).where("workflow_state<>'settings_only'").all
-      submission_ids = {}
-      @submissions.each{|s| submission_ids[s.user_id] = s.id }
-      @submitted_students = students.select{|stu| submission_ids[stu.id] }
-      if @quiz.survey? && @quiz.anonymous_submissions
-        @submitted_students = @submitted_students.sort_by{|stu| submission_ids[stu.id] }
+      @submissions = Hash[@submissions.map { |s| [s.user_id,s] }]
+      @submitted_students, @unsubmitted_students = students.partition do |stud|
+        @submissions[stud.id]
       end
-      @unsubmitted_students = students.reject{|stu| submission_ids[stu.id] }
-      submitted_students_json = @submitted_students.map { |u| user_json(u, @current_user, session) }
-      unsubmitted_students_json = @unsubmitted_students.map { |u| user_json(u, @current_user, session) }
-      @quiz_submission_list = {:UNSUBMITTED_STUDENTS => unsubmitted_students_json,
-                                         :SUBMITTED_STUDENTS => submitted_students_json}.to_json
+
+      if @quiz.anonymous_survey?
+        @submitted_students = @submitted_students.sort_by do |student|
+          @submissions[student.id].id
+        end
+        submitted_students_json = @submitted_students.map &:id
+        unsubmitted_students_json = @unsubmitted_students.map &:id
+      else
+        submitted_students_json = @submitted_students.map { |u| user_json(u, @current_user, session) }
+        unsubmitted_students_json = @unsubmitted_students.map { |u| user_json(u, @current_user, session) }
+      end
+
+      @quiz_submission_list = { :UNSUBMITTED_STUDENTS => unsubmitted_students_json,
+                                :SUBMITTED_STUDENTS => submitted_students_json }.to_json
       render :layout => false
     end
   end

app/helpers/quizzes_helper.rb

@@ -78,20 +78,76 @@ module QuizzesHelper
     end
   end
 
-  def render_unsubmitted_students(student_count)
-    I18n.t('#quizzes.headers.unsubmitted_students_count',
-      { :one => "1 student hasn't taken the survey",
-        :other => "%{count} students haven't taken the survey" },
+  def submitted_students_title(quiz, students)
+    if quiz.survey?
+      submitted_students_survey_title(students.length)
+    else
+      submitted_students_quiz_title(students.length)
+    end
+  end
+
+  def submitted_students_quiz_title(student_count)
+    I18n.t('#quizzes.headers.submitted_students_quiz_title',
+      { :zero => "Students who have taken the quiz",
+        :one => "Students who have taken the quiz (%{count})",
+        :other => "Students who have taken the quiz (%{count})" },
       { :count => student_count })
   end
 
+  def submitted_students_survey_title(student_count)
+    I18n.t('#quizzes.headers.submitted_students_survey_title',
+      { :zero => "Students who have taken the survey",
+        :one => "Students who have taken the survey (%{count})",
+        :other => "Students who have taken the survey (%{count})" },
+      { :count => student_count })
+  end
+
+  def no_submitted_students_msg(quiz)
+    if quiz.survey?
+      t('#quizzes.messages.no_submitted_students_survey', "No Students have taken the survey yet")
+    else
+      t('#quizzes.messages.no_submitted_students_quiz', "No Students have taken the quiz yet")
+    end
+  end
+
+  def unsubmitted_students_title(quiz, students)
+    if quiz.survey?
+      unsubmitted_students_survey_title(students.length)
+    else
+      unsubmitted_students_quiz_title(students.length)
+    end
+  end
+
+  def unsubmitted_students_quiz_title(student_count)
+    I18n.t('#quizzes.headers.unsubmitted_students_quiz_title',
+      { :zero => "Student who haven't taken the quiz",
+        :one => "Students who haven't taken the quiz (%{count})",
+        :other => "Students who haven't taken the quiz (%{count})" },
+      { :count => student_count })
+  end
+
+  def unsubmitted_students_survey_title(student_count)
+    I18n.t('#quizzes.headers.unsubmitted_students_survey_title',
+      { :zero => "Student who haven't taken the survey",
+        :one => "Students who haven't taken the survey (%{count})",
+        :other => "Students who haven't taken the survey (%{count})" },
+      { :count => student_count })
+  end
+
+  def no_unsubmitted_students_msg(quiz)
+    if quiz.survey?
+      t('#quizzes.messages.no_unsubmitted_students_survey', "All Students have taken the survey")
+    else
+      t('#quizzes.messages.no_unsubmitted_students_quiz', "All Students have taken the quiz")
+    end
+  end
+
   QuestionType = Struct.new(:question_type,
-                          :entry_type,
-                          :display_answers,
-                          :answer_type,
-                          :multiple_sets,
-                          :unsupported
-                         )
+                            :entry_type,
+                            :display_answers,
+                            :answer_type,
+                            :multiple_sets,
+                            :unsupported)
 
   def answer_type(question)
     return QuestionType.new unless question

app/models/quiz.rb

@@ -1393,4 +1393,8 @@ class Quiz < ActiveRecord::Base
   def self.mark_quiz_edited(id)
     where(:id =>id).update_all(:last_edited_at => Time.now.utc)
   end
+
+  def anonymous_survey?
+    survey? && anonymous_submissions
+  end
 end

app/stylesheets/quizzes.sass

@@ -44,18 +44,16 @@ $questionHeaderBackground: #F5F5F5
   #quiz_details
     // display: none so jquery ui toggles correctly
     display: none
-    margin: 0 auto 10px auto
-    max-height: 500px
+    margin: 0 auto 10px 0
     overflow: auto
-
-    .submitted_list
-      float: left
-      width: 50%
-
-    .name
-      display: block
-      padding: 4px 12px
-      border-top: 1px solid white
+    max-height: 400px
+    tr
+      th
+        font-weight: bold
+        text-transform: capitalize
+      td
+        border-top: 1px solid #bbb
+        border-bottom: 1px solid #bbb
 
   .control-group
     margin-bottom: 0px

app/views/quizzes/managed_quiz_data.html.erb

@@ -1,74 +1,51 @@
-<div id="quiz_details" class="clearfix">
-  <div class="student_list">
-    <div class="list-view border border-trbl submitted_list">
-        <header>
-          <% if @quiz.survey? %>
-            <%= t('headers.submitted_students_survey', "Students who have taken the survey") %>
-          <% else %>
-            <%= t('headers.submitted_students_quiz', "Students who have taken the quiz") %>
-          <% end %>
-        </header>
-        <ul class="quiz_results_list">
-          <% if @submitted_students.empty? %>
-            <li>
-              <span class="name">
-                <%= @quiz.survey? ?
-                  t(:survey_no_submissions, "No Students have taken the survey yet") :
-                  t(:quiz_no_submissions, "No Students have taken the quiz yet")
-                %>
-              </span>
-            </li>
-          <% end %>
-          <% @submitted_students.each_with_index do |s, idx| %>
-            <% submission = @submissions.find{|qs| qs.user_id == s.id} %>
-            <li class="student" data-id="<%= s.id %>">
-              <a href="<%= context_url(@context, :context_quiz_history_url, @quiz, :quiz_submission_id => submission.id) %>" class="name">
-                <% if submission.pending_review? %>
-                  <i class="icon-warning" title="<%= t('titles.submission_needs_review', "This submission needs review") %>"></i>
-                <% end %>
-                <% if @quiz.survey? && @quiz.anonymous_submissions %>
-                <%= t(:anonymous_student, "Student %{n}", :n => idx + 1) %>
-                <% else %>
-                <%= s.name %>
-                <% end %>
-              </a>
-            </li>
-        <% end %>
-        </ul>
-    </div>
-    <div class="list-view border border-trbl">
-      <% if @quiz.survey? && @quiz.anonymous_submissions %>
-        <header>
-          <%= render_unsubmitted_students(@unsubmitted_students.length) %>
-        </header>
-      <% else %>
-        <header>
-          <% if @quiz.survey? %>
-          <%= t('headers.survey_unsubmitted_students', "Students who haven't taken the survey") %>
-          <% else %>
-          <%= t('headers.quiz_unsubmitted_students', "Students who haven't taken the quiz") %>
-          <% end %>
-        </header>
-        <ul class="student_list">
-          <% if @unsubmitted_students.empty? && !@submitted_students.empty? %>
-            <li><span class="name">
-            <% if @quiz.survey? %>
-            <%= t(:survey_all_students_submitted, "All Students have taken the survey") %>
-            <% else %>
-            <%= t(:quiz_all_students_submitted, "All Students have taken the quiz") %>
+<div id="quiz_details">
+  <table class="table">
+    <thead>
+      <th>
+        <%= submitted_students_title(@quiz, @submitted_students) %>
+      </th>
+      <th>
+        <%= unsubmitted_students_title(@quiz, @unsubmitted_students) %>
+      </th>
+    </thead>
+    <tbody>
+      <%
+        [@submitted_students.size, @unsubmitted_students.size, 1].max.times do |index|
+          student = @submitted_students[index]
+          unsubmitted_student = @unsubmitted_students[index]
+          submission = student.present? ? @submissions[student.id] : nil
+      %>
+      <tr>
+        <td>
+          <% if @submitted_students.empty? && index == 0 %>
+            <%= no_submitted_students_msg(@quiz) %>
+          <% elsif student.present? && submission.present? %>
+            <% if submission.pending_review? %>
+              <% warn_msg = t('titles.submission_needs_review', "This submission needs review") %>
+              <i class="icon-warning" title="<%= warn_msg %>"></i>
             <% end %>
-          </span></li>
+          <a href="<%= polymorphic_path([@context, @quiz, :history], :quiz_submission_id => submission.id) %>" class="name">
+            <% if @quiz.anonymous_survey? %>
+              <%= t(:anonymous_student, "Student %{n}", :n => index + 1) %>
+            <% else %>
+              <%= student.name %>
+            <% end %>
+          </a>
           <% end %>
-          <% @unsubmitted_students.each do |s| %>
-          <li class="student" data-id="<%= s.id %>">
-            <span class="name"><%= s.name %></span>
-          </li>
-          <% end %>
-        </ul>
+        </td>
+        <td>
+          <span>
+            <% if @unsubmitted_students.empty? && !@submitted_students.empty? && index == 0 %>
+              <%= no_unsubmitted_students_msg(@quiz) %>
+            <% elsif unsubmitted_student.present? && !@quiz.anonymous_survey? %>
+              <%= unsubmitted_student.name %>
+            <% end %>
+          </span>
+        </td>
+      </tr>
       <% end %>
-    </div>
-
-  </div>
+    </tbody>
+  </table>
 </div>
 
 <script>

public/javascripts/quizzes.js

@@ -1167,7 +1167,7 @@ define([
       });
       if (!$dialog.hasClass('loaded')) {
         $dialog.find(".searching_message").text(I18n.t('retrieving_filters', "Retrieving Filters..."));
-        var url = $("#quiz_urls .filters_url").attr('href');
+        var url = ENV.QUIZZES_URL;
         $.ajaxJSON(url, 'GET', {}, function(data) {
           $dialog.addClass('loaded');
           if (data.length) {

spec/controllers/quizzes_controller_spec.rb

@@ -184,12 +184,13 @@ describe QuizzesController do
 
       user_session @teacher
       get 'managed_quiz_data', :course_id => @course.id, :quiz_id => @quiz.id
-      assigns[:submissions].sort_by(&:id).should ==[@sub1, @sub2].sort_by(&:id)
+      assigns[:submissions][@sub1.user_id].should == @sub1
+      assigns[:submissions][@sub2.user_id].should == @sub2
       assigns[:submitted_students].sort_by(&:id).should == [@user1, @user2].sort_by(&:id)
 
       user_session @ta1
       get 'managed_quiz_data', :course_id => @course.id, :quiz_id => @quiz.id
-      assigns[:submissions].should ==[@sub1]
+      assigns[:submissions][@sub1.user_id].should == @sub1
       assigns[:submitted_students].should == [@user1]
     end
   end