grades page works with multiple enrollments; fixes #7614

don't list courses multiple times on the user grades page if that user has
multiple enrollments in a course.

test-plan:
- as a student with multiple enrollments in a course and enrollments in
  > 1 courses
- visit /grades, make sure courses are listed once.

Change-Id: Icd3722ba07070fc6453e7395a833c701c447860f
Reviewed-on: https://gerrit.instructure.com/9247
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Brian Palmer <brianp@instructure.com>

app/controllers/users_controller.rb

@@ -47,7 +47,9 @@ class UsersController < ApplicationController
       end
       #@prior_enrollments.concat @user.concluded_enrollments.select{|e| e.is_a?(StudentEnrollment) }
 
-      @student_enrollments = @current_enrollments.select{|e| e.is_a?(StudentEnrollment) }
+      @student_enrollments = @current_enrollments.
+        select{ |e| e.is_a?(StudentEnrollment) }.
+        inject({}){ |hash, e| hash[e.course] = e; hash }
 
       @observer_enrollments = @current_enrollments.select{|e| e.is_a?(ObserverEnrollment) && e.associated_user_id }
       @observed_enrollments = []
@@ -56,14 +58,16 @@ class UsersController < ApplicationController
       end
       @observed_enrollments = @observed_enrollments.uniq.compact
 
-      if @current_enrollments.length + @observed_enrollments.length == 1# && @prior_enrollments.empty?
+      @teacher_enrollments = @current_enrollments.select{|e| e.instructor? }
+
+      if @student_enrollments.length + @teacher_enrollments.length + @observed_enrollments.length == 1# && @prior_enrollments.empty?
         redirect_to course_grades_url(@current_enrollments.first.course_id)
         return
       end
-      Enrollment.send(:preload_associations, @observed_enrollments, :course)
 
-      @teacher_enrollments = @current_enrollments.select{|e| e.instructor? }
+      Enrollment.send(:preload_associations, @observed_enrollments, :course)
       #Enrollment.send(:preload_associations, @prior_enrollments, :course)
+
       @course_grade_summaries = {}
       @teacher_enrollments.each do |enrollment|
         @course_grade_summaries[enrollment.course_id] = Rails.cache.fetch(['computed_avg_grade_for', enrollment.course].cache_key) do

app/stylesheets/user_grades.sass

@@ -0,0 +1,22 @@
+.course_details
+  border-collapse: collapse
+  min-width: 400px
+  margin-left: 20px
+  border-spacing: 0
+  th
+    text-align: left
+  tr:hover td
+    background-color: #eee
+  td
+    border-bottom: 1px solid #ccc
+  .course
+    font-size: 1.2em
+    padding: 6px 50px 6px 5px
+    vertical-align: top
+  .percent
+    text-align: right
+    font-size: 1.2em
+    font-weight: bold
+    padding-right: 5px
+  .report
+    padding: 12px

app/views/users/grades.html.erb

@@ -1,7 +1,9 @@
-<% add_crumb(@current_user.short_name, context_url(@current_user, :context_url))
+<% 
-   add_crumb(t('crumbs.grades', 'Grades'), grades_path) %>
+  content_for :page_title, join_title(t(:page_title, "Grades"), @current_user.name)
-
+  add_crumb(@current_user.short_name, context_url(@current_user, :context_url))
-<% content_for :page_title, join_title(t(:page_title, "Grades"), @current_user.name) %>
+  add_crumb(t('crumbs.grades', 'Grades'), grades_path) 
+  jammit_css :user_grades
+%>
 
 <% if false %>
 <% #TODO: implement previous courses  %>
@@ -11,62 +13,16 @@
     </div>
 <% end %>
 <% end %>
-<% js_block do %>
-<script>
-require([
-  'jquery' /* $ */,
-  'vendor/jquery.scrollTo' /* /\.scrollTo/ */
-], function($) {
-
-$(document).ready(function() {
-  $(".previous_courses_link").click(function(event) {
-    event.preventDefault();
-    $("#previous_courses").show();
-    $("html,body").scrollTo($("#previous_courses"));
-  });
-});
-});
-</script>
-<% end %>
 <style>
-.course_details { 
-  border-collapse: collapse;
-  min-width: 400px;
-  margin-left: 20px;
-  border-spacing: 0;
-}
-.course_details th {
-  text-align: left;
-}
-.course_details tr:hover td {
-  background-color: #eee;
-}
-.course_details td {
-  border-bottom: 1px solid #ccc;
-}
-.course_details .course {
-  font-size: 1.2em;
-  padding: 6px 50px 6px 5px;
-  vertical-align: top;
-}
-.course_details .percent {
-  text-align: right;
-  font-size: 1.2em;
-  font-weight: bold;
-  padding-right: 5px;
-}
-.course_details .report {
-  padding: 12px;
-}
 </style>
 <% unless @student_enrollments.empty? %>
 <h2><%= t('titles.current_student_courses', "Courses I'm Taking") %></h2>
-<table class="course_details">
+<table class="course_details student_grades">
-<% @student_enrollments.each do |enrollment| %>
+<% @student_enrollments.each do |course, enrollment| %>
   <tr>
-    <td class="course"><a href="<%= course_student_grades_path(enrollment.course_id, enrollment.user_id) %>"><%= enrollment.course.name %></a></td>
+    <td class="course"><a href="<%= course_student_grades_path(course.id, enrollment.user_id) %>"><%= course.name %></a></td>
     <td class="percent">
-      <% if enrollment.course.settings[:hide_final_grade] %>
+      <% if course.settings[:hide_final_grade] %>
         --
       <% elsif enrollment.computed_current_score %>
         <%= enrollment.computed_current_score %>%
@@ -81,7 +37,7 @@ $(document).ready(function() {
 <% end %>
 <% unless @observed_enrollments.empty? %>
 <h2 style="margin-top: 20px;"><%= t('linked_student_accounts', 'Linked Student Accounts') %></h2>
-<table class="course_details">
+<table class="course_details observer_grades">
 <% @observed_enrollments.each do |enrollment| %>
   <tr>
     <td class="course"><a href="<%= course_student_grades_path(enrollment.course_id, enrollment.user_id) %>"><%= context_user_name(nil, enrollment.user_id) %>, <%= enrollment.course.name %></a></td>
@@ -101,7 +57,7 @@ $(document).ready(function() {
 <% end %>
 <% unless @teacher_enrollments.empty? %>
 <h2 style="margin-top: 20px;"><%= t('current_teacher_courses', "Courses I'm Teaching") %></h2>
-<table class="course_details">
+<table class="course_details teacher_grades">
 <% @teacher_enrollments.each do |enrollment| %>
   <tr>
     <td class="course" style="vertical-align: middle;"><a href="<%= course_gradebook_path(enrollment.course_id) %>"><%= enrollment.course.name %></a></td>

config/assets.yml

@@ -192,6 +192,8 @@ stylesheets:
     - public/javascripts/vendor/slickgrid/slick.grid.css
   sub_accounts:
     - public/stylesheets/compiled/sub_accounts.css
+  user_grades:
+    - public/stylesheets/compiled/user_grades.css
   user_logins:
     - public/stylesheets/compiled/user_logins.css
   account_settings:

spec/integration/users_controller_spec.rb

@@ -203,5 +203,21 @@ describe UsersController do
       end
     end
   end
+
+  describe "#grades" do
+    it "should only list courses once for multiple enrollments" do
+      course_with_student_logged_in(:active_all => true)
+      @first_course = @course
+      add_section("other section")
+      multiple_student_enrollment(@student, @course_section)
+      course_with_student(:user => @student, :active_all => true)
+
+      get grades_url
+      student_grades = Nokogiri::HTML(response.body).css('.student_grades tr')
+      student_grades.length.should == 2
+      student_grades.text.should match /#{@first_course.name}/
+      student_grades.text.should match /#{@course.name}/
+    end
+  end
 end