show individual grade publishing status results on the course settings page

Change-Id: I81ddbc77fbc7ac5db4d1b26e88afa73477b2c26c
Reviewed-on: https://gerrit.instructure.com/5950
Tested-by: Hudson <hudson@instructure.com>
Reviewed-by: Jacob Fugal <jacob@instructure.com>

app/controllers/courses_controller.rb

@@ -973,16 +973,15 @@ class CoursesController < ApplicationController
   end
 
   def publish_to_sis
-    get_context
+    sis_publish_status(true)
-    return unless authorized_action(@context, @current_user, :manage_grades)
-    @context.publish_final_grades(@current_user)
-    render :json => {:sis_publish_status => @context.grade_publishing_status}.to_json
   end
 
-  def sis_publish_status
+  def sis_publish_status(publish_grades=false)
     get_context
     return unless authorized_action(@context, @current_user, :manage_grades)
-    render :json => {:sis_publish_status => @context.grade_publishing_status}
+    @context.publish_final_grades(@current_user) if publish_grades
+    render :json => {:sis_publish_messages => @context.grade_publishing_messages,
+                     :sis_publish_status => @context.grade_publishing_status}
   end
 
   def reset_content
@@ -991,4 +990,5 @@ class CoursesController < ApplicationController
     @new_course = @context.reset_content
     redirect_to course_settings_path(@new_course.id)
   end
+
 end

app/models/course.rb

@@ -986,19 +986,68 @@ class Course < ActiveRecord::Base
   end
 
   def grade_publishing_messages
-    student_enrollments.count(:all, :group => :grade_publishing_message, :conditions => "grade_publishing_message IS NOT NULL AND grade_publishing_message != ''")
+    messages = {}
+    student_enrollments.count(:all, :group => [:grade_publishing_message, :grade_publishing_status]).each do |key, count|
+      status = key.last
+      status = "unpublished" if status.blank?
+      message = key.first
+      case status
+      when 'error'
+        if message.present?
+          message = t('grade_publishing_status.error_with_message', "Error: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.error', "Error")
+        end
+      when 'unpublished'
+        if message.present?
+          message = t('grade_publishing_status.unpublished_with_message', "Unpublished: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.unpublished', "Unpublished")
+        end
+      when 'pending'
+        if message.present?
+          message = t('grade_publishing_status.pending_with_message', "Pending: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.pending', "Pending")
+        end
+      when 'publishing'
+        if message.present?
+          message = t('grade_publishing_status.publishing_with_message', "Publishing: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.publishing', "Publishing")
+        end
+      when 'published'
+        if message.present?
+          message = t('grade_publishing_status.published_with_message', "Published: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.published', "Published")
+        end
+      when 'unpublishable'
+        if message.present?
+          message = t('grade_publishing_status.unpublishable_with_message', "Unpublishable: %{message}", :message => message)
+        else
+          message = t('grade_publishing_status.unpublishable', "Unpublishable")
+        end
+      else
+        if message.present?
+          message = t('grade_publishing_status.unknown_with_message', "Unknown status, %{status}: %{message}", :message => message, :status => status)
+        else
+          message = t('grade_publishing_status.unknown', "Unknown status, %{status}", :status => status)
+        end
+      end
+      messages[message] ||= 0
+      messages[message] += count
+    end
+    messages
   end
 
   def grade_publishing_status
+    # this will return the overall course grade publishing status
     statuses = {}
-    student_enrollments.find(:all, :select => "DISTINCT grade_publishing_status, 0 AS user_id").each do |enrollment|
+    student_enrollments.count(:all, :group => [:grade_publishing_status]).each do |key, count|
-        status = enrollment.grade_publishing_status
+      statuses[key || "unpublished"] = true
-        status ||= "unpublished"
-        statuses[status] = true
     end
     return "unpublished" unless statuses.size > 0
-    # to fake a course-level grade publishing status, we look at all possible
-    # enrollments, and return statuses if we find any, in this order.
     ["error", "unpublished", "pending", "publishing", "published", "unpublishable"].each do |status|
       return status if statuses.has_key?(status)
     end

app/views/courses/settings.html.erb

@@ -645,11 +645,9 @@ Hashtags should consist of letters, numbers, dashes and underscores (no spaces).
   <% if publishing_enabled %>
     <div id="tab-grade-publishing">
       <h2><%= t('headings.grade_publishing', %{Grade Publishing}) %></h2>
-      <% form_tag context_url(@context, :context_publish_to_sis_url), { :id => "publish_to_sis_form", :style => "display: none;" } do %>
+      <% form_tag context_url(@context, :context_publish_to_sis_url), { :id => "publish_to_sis_form", :style => "display: none;" } do %><% end %>
-      <a href="<%= context_url(@context, :context_publish_to_sis_url) %>" id="sis_publish_link" style="display: none;">&nbsp;</a>
+      <a href="#" id="publish_grades_link" class="button disabled">...</a>
-      <% end %>
+      <ul id="publish_grades_messages"></ul>
-      <a href="#" id="publish_grades_link" class="button disabled"><%= t('links.publishing_to_sis', %{Publishing grades to SIS...}) %></a>
-      <span id="publish_grades_error" style="display: none;">&nbsp;<%= t('errors.sis_publish', %{An error occurred on the last attempt to publish grades to SIS.}) %></span>
     </div>
   <% end %>
   <% if @context.root_account.settings[:enable_alerts] && can_do(@context, @current_user, :manage_interaction_alerts) %>
@@ -686,7 +684,6 @@ Hashtags should consist of letters, numbers, dashes and underscores (no spaces).
   </div>
 <% end %>
 <script>
-  var sisPublishStatus = "<%= @context.grade_publishing_status %>";
   var sisPublishEnabled = "<%= publishing_enabled %>";
 </script>
 <div id="edit_letter_grades_form" style="display: none;" data-context_code="<%= @context.asset_string %>">

public/javascripts/course_settings.js

@@ -17,41 +17,84 @@
  */
 
 I18n.scoped('course_settings', function(I18n) {
-  function checkup() {
+
-    $.ajaxJSON($("#sis_publish_link").attr('href'), 'GET', {}, function(data) {
+  var GradePublishing = {
-      if (!data.hasOwnProperty("sis_publish_status")) {
+    status: null,
+    checkup: function() {
+      $.ajaxJSON($("#publish_to_sis_form").attr('action'), 'GET', {}, function(data) {
+        if (!data.hasOwnProperty("sis_publish_status")) {
+          return;
+        }
+        GradePublishing.status = data.sis_publish_status;
+        GradePublishing.update(data.hasOwnProperty("sis_publish_messages") ? data.sis_publish_messages : {});
+      });
+    },
+    update: function(messages, requestInProgress) {
+      var $publish_grades_link = $("#publish_grades_link"),
+          $publish_grades_error = $("#publish_grades_error");
+      if (GradePublishing.status == 'published') {
+        $publish_grades_error.hide();
+        $publish_grades_link.html(I18n.t('links.republish', "Republish grades to SIS"));
+        $publish_grades_link.removeClass("disabled");
+      } else if (GradePublishing.status == 'publishing' || GradePublishing.status == 'pending') {
+        $publish_grades_error.hide();
+        $publish_grades_link.html(I18n.t('links.publishing', "Publishing grades to SIS..."));
+        if (!requestInProgress) {
+          setTimeout(GradePublishing.checkup, 5000);
+        }
+        $publish_grades_link.addClass("disabled");
+      } else if (GradePublishing.status == 'unpublished') {
+        $publish_grades_error.hide();
+        $publish_grades_link.html(I18n.t('links.publish', "Publish grades to SIS"));
+        $publish_grades_link.removeClass("disabled");
+      } else {
+        $publish_grades_error.show();
+        $publish_grades_link.html(I18n.t('links.republish', "Republish grades to SIS"));
+        $publish_grades_link.removeClass("disabled");
+      }
+      $messages = $("#publish_grades_messages");
+      $messages.empty();
+      $.each(messages, function(message, count) {
+        var $message = $("<span/>");
+        $message.text(message);
+        var $item = $("<li/>");
+        $item.append($message);
+        $item.append(" - <b>" + count + "</b>");
+        $messages.append($item);
+      });
+    },
+    publish: function() {
+      if (GradePublishing.status == 'publishing' || GradePublishing.status == 'pending' || GradePublishing.status == null) {
         return;
       }
-      sisPublishStatus = data["sis_publish_status"];
+      if (GradePublishing.status == 'published') {
-      updatePublishingStatus();
+        if (!confirm(I18n.t('confirm.re_publish_grades', "Are you sure you want to republish these grades to the student information system?")))
-    });
+          return;
-  }
+      } else {
-
+        if (!confirm(I18n.t('confirm.publish_grades', "Are you sure you want to publish these grades to the student information system? You should only do this if all your grades have been finalized.")))
-  function updatePublishingStatus(requestInProgress) {
+          return;
-    var $publish_grades_link = $("#publish_grades_link"),
-        $publish_grades_error = $("#publish_grades_error");
-    if (sisPublishStatus == 'published') {
-      $publish_grades_error.hide();
-      $publish_grades_link.html(I18n.t('links.republish', "Republish grades to SIS"));
-      $publish_grades_link.removeClass("disabled");
-    } else if (sisPublishStatus == 'publishing' || sisPublishStatus == 'pending') {
-      $publish_grades_error.hide();
-      $publish_grades_link.html(I18n.t('links.publishing', "Publishing grades to SIS..."));
-      if (!requestInProgress) {
-        setTimeout(checkup, 5000);
       }
-      $publish_grades_link.addClass("disabled");
+      var $publish_to_sis_form = $("#publish_to_sis_form");
-    } else if (sisPublishStatus == 'unpublished') {
+      GradePublishing.status = "publishing";
-      $publish_grades_error.hide();
+      GradePublishing.update({}, true);
-      $publish_grades_link.html(I18n.t('links.publish', "Publish grades to SIS"));
+      var successful_statuses = { "published": 1, "publishing": 1, "pending": 1 };
-      $publish_grades_link.removeClass("disabled");
+      var error = function(data, xhr, status, error) {
-    } else {
+        GradePublishing.status = "unknown";
-      $publish_grades_error.show();
+        $.flashError(I18n.t('errors.publish_grades', "Something went wrong when trying to publish grades to the student information system. Please try again later."));
-      $publish_grades_link.html(I18n.t('links.republish', "Republish grades to SIS"));
+        GradePublishing.update({});
-      $publish_grades_link.removeClass("disabled");
+      };
+      $.ajaxJSON($publish_to_sis_form.attr('action'), 'POST', $publish_to_sis_form.getFormData(), function(data) {
+        if (!data.hasOwnProperty("sis_publish_status") || !successful_statuses.hasOwnProperty(data["sis_publish_status"])) {
+          error(null, null, I18n.t('errors.invalid_sis_status', "Invalid SIS publish status"), null);
+          return;
+        }
+        GradePublishing.status = data.sis_publish_status;
+        GradePublishing.update(data.hasOwnProperty("sis_publish_messages") ? data.sis_publish_messages : {});
+      }, error);
     }
   }
 
+
   $(document).ready(function() {
     var $add_section_form = $("#add_section_form"),
         $edit_section_form = $("#edit_section_form"),
@@ -452,39 +495,14 @@ I18n.scoped('course_settings', function(I18n) {
       $(".open_enrollment_holder").showIf($(this).attr('checked'));
     }).change();
 
-    var $publish_to_sis_form = $("#publish_to_sis_form");
     $("#publish_grades_link").click(function(event) {
       event.preventDefault();
-      if (sisPublishStatus == 'publishing' || sisPublishStatus == 'pending') {
+      GradePublishing.publish();
-        return;
-      }
-      if (sisPublishStatus == 'published') {
-        if (!confirm(I18n.t('confirm.re_publish_grades', "Are you sure you want to republish these grades to the student information system?")))
-          return;
-      } else {
-        if (!confirm(I18n.t('confirm.publish_grades', "Are you sure you want to publish these grades to the student information system? You should only do this if all your grades have been finalized.")))
-          return;
-      }
-      sisPublishStatus = "publishing";
-      updatePublishingStatus(true);
-      var successful_statuses = { "published": 1, "publishing": 1, "pending": 1 };
-      var error = function(data, xhr, status, error) {
-        sisPublishStatus = "unknown";
-        $.flashError(I18n.t('errors.publish_grades', "Something went wrong when trying to publish grades to the student information system. Please try again later."));
-        updatePublishingStatus();
-      };
-      $.ajaxJSON($publish_to_sis_form.attr('action'), 'POST', $publish_to_sis_form.getFormData(), function(data) {
-        if (!data.hasOwnProperty("sis_publish_status") || !successful_statuses.hasOwnProperty(data["sis_publish_status"])) {
-          error(null, null, I18n.t('errors.invalid_sis_status', "Invalid SIS publish status"), null);
-          return;
-        }
-        sisPublishStatus = data["sis_publish_status"];
-        updatePublishingStatus();
-      }, error);
     });
     if (typeof(sisPublishEnabled) != 'undefined' && sisPublishEnabled) {
-      updatePublishingStatus();
+      GradePublishing.checkup();
     }
+
     $(".reset_course_content_button").click(function(event) {
       event.preventDefault();
       $("#reset_course_content_dialog").dialog('close').dialog({

public/javascripts/gradebooks.js

@@ -152,28 +152,6 @@ I18n.scoped('gradebook', function(I18n) {
           return "th_assignment_"+assignment_id+" th_student_"+student_id;
         }
       });
-    },
-    publishGradesToSis: function() {
-      if(sisPublishStatus == 'published') {
-        if(!confirm(I18n.t('confirms.republish_to_sis', "Are you sure you want to republish these grades to the student information system?")))
-          return;
-      } else {
-        if(!confirm(I18n.t('confirms.publish_to_sis', "Are you sure you want to publish these grades to the student information system? You should only do this if all your grades have been finalized.")))
-          return;
-      }
-      sisPublishStatus = "publishing";
-      var successful_statuses = { "published": 1, "publishing": 1, "pending": 1 };
-      var error = function(data, xhr, status, error) {
-        sisPublishStatus = "unknown";
-        $.flashError(I18n.t('errors.publishing_failed', "Something went wrong when trying to publish grades to the student information system. Please try again later."));
-      };
-      $.ajaxJSON($publish_to_sis_form.attr('action'), 'POST', $publish_to_sis_form.getFormData(), function(data) {
-        if(!data.hasOwnProperty("sis_publish_status") || !successful_statuses.hasOwnProperty(data["sis_publish_status"])) {
-          error(null, null, I18n.t('errors.invalid_publishing_response', "Invalid SIS publish status"), null);
-          return;
-        }
-        sisPublishStatus = data["sis_publish_status"];
-      }, error);
     }
   };
   

spec/lib/sis_csv_importer_spec.rb

@@ -2243,7 +2243,7 @@ describe SIS::CSV::Import do
         "#{@enrollment.id},published,message1")
 
       @course.grade_publishing_status.should == 'published'
-      @course.grade_publishing_messages.should == { "message1" => 1 }
+      @course.grade_publishing_messages.should == { "Published: message1" => 1 }
 
       @enrollment.reload
       @enrollment.grade_publishing_status.should == 'published'

spec/selenium/grade_passback_spec.rb

@@ -0,0 +1,158 @@
+require File.expand_path(File.dirname(__FILE__) + '/common')
+
+describe "grade exchange course settings tab" do
+  it_should_behave_like "in-process server selenium tests"
+
+  def getpseudonym(user_sis_id)
+    pseudo = Pseudonym.find_by_sis_user_id(user_sis_id)
+    pseudo.should_not be_nil
+    pseudo
+  end
+
+  def getuser(user_sis_id)
+    user = getpseudonym(user_sis_id).user
+    user.should_not be_nil
+    user
+  end
+
+  def getsection(section_sis_id)
+    section = CourseSection.find_by_sis_source_id(section_sis_id)
+    section.should_not be_nil
+    section
+  end
+
+  def getenroll(user_sis_id, section_sis_id)
+    e = Enrollment.find_by_user_id_and_course_section_id(getuser(user_sis_id).id, getsection(section_sis_id).id)
+    e.should_not be_nil
+    e
+  end
+
+  def grade_passback_setup(wait_for_success)
+    process_csv_data_cleanly(
+      "user_id,login_id,password,first_name,last_name,email,status",
+      "T1,Teacher1,,T,1,t1@example.com,active",
+      "S1,Student1,,S,1,s1@example.com,active",
+      "S2,Student2,,S,2,s2@example.com,active",
+      "S3,Student3,,S,3,s3@example.com,active",
+      "S4,Student4,,S,4,s4@example.com,active",
+      "S5,Student5,,S,5,s5@example.com,active",
+      "S6,Student6,,S,6,s6@example.com,active")
+    process_csv_data_cleanly(
+      "course_id,short_name,long_name,account_id,term_id,status",
+      "C1,C1,C1,,,active")
+    @course = Course.find_by_sis_source_id("C1")
+    @course.assignment_groups.create(:name => "Assignments")
+    @teacher = getuser("T1")
+    process_csv_data_cleanly(
+      "section_id,course_id,name,status,start_date,end_date",
+      "S1,C1,S1,active,,",
+      "S2,C1,S2,active,,",
+      "S3,C1,S3,active,,",
+      "S4,C1,S4,active,,")
+    process_csv_data_cleanly(
+      "course_id,user_id,role,section_id,status",
+      ",T1,teacher,S1,active",
+      ",S1,student,S1,active",
+      ",S2,student,S2,active",
+      ",S3,student,S2,active",
+      ",S4,student,S1,active",
+      ",S5,student,S3,active",
+      ",S6,student,S4,active")
+    a1 = @course.assignments.create!(:title => "A1", :points_possible => 10)
+    a2 = @course.assignments.create!(:title => "A2", :points_possible => 10)
+
+    a1.grade_student(getuser("S1"), { :grade => "6", :grader => @teacher })
+    a1.grade_student(getuser("S2"), { :grade => "6", :grader => @teacher })
+    a1.grade_student(getuser("S3"), { :grade => "7", :grader => @teacher })
+    a1.grade_student(getuser("S5"), { :grade => "7", :grader => @teacher })
+    a1.grade_student(getuser("S6"), { :grade => "8", :grader => @teacher })
+    a2.grade_student(getuser("S1"), { :grade => "8", :grader => @teacher })
+    a2.grade_student(getuser("S2"), { :grade => "9", :grader => @teacher })
+    a2.grade_student(getuser("S3"), { :grade => "9", :grader => @teacher })
+    a2.grade_student(getuser("S5"), { :grade => "10", :grader => @teacher })
+    a2.grade_student(getuser("S6"), { :grade => "10", :grader => @teacher })
+
+    @stud5, @stud6, @sec4 = nil, nil, nil
+    Pseudonym.find_by_sis_user_id("S5").tap do |p|
+      @stud5 = p
+      p.sis_user_id = nil
+      p.sis_source_id = nil
+      p.save
+    end
+
+    Pseudonym.find_by_sis_user_id("S6").tap do |p|
+      @stud6 = p
+      p.sis_user_id = nil
+      p.sis_source_id = nil
+      p.save
+    end
+
+    getsection("S4").tap do |s|
+      @sec4 = s
+      sec4id = s.sis_source_id
+      s.sis_source_id = nil
+      s.save
+    end
+
+    @course.grading_standard_id = 0
+    @course.save!
+    GradeCalculator.recompute_final_score(["S1", "S2", "S3", "S4"].map{|x|getuser(x).id}, @course.id)
+    @course.reload
+
+    PluginSetting.settings_for_plugin('grade_export')[:enabled] = "true"
+    PluginSetting.settings_for_plugin('grade_export')[:format_type] = "instructure_csv"
+    PluginSetting.settings_for_plugin('grade_export')[:wait_for_success] = wait_for_success ? "yes" : "no"
+    server, server_thread, post_lines = start_test_http_server
+    PluginSetting.settings_for_plugin('grade_export')[:publish_endpoint] = "http://localhost:#{server.addr[1]}/endpoint"
+
+    @course.offer!
+    user_session(@teacher)
+
+    @course.grading_standard_id = 0
+    @course.save!
+
+    PluginSetting.settings_for_plugin('grade_export')[:enabled] = "true"
+
+    get "/courses/#{@course.id}/settings"
+    driver.find_element(:css, "a#tab-grade-publishing-link").click
+    wait_for_ajaximations
+
+    driver.find_element(:css, "#publish_grades_messages").text.should == "Unpublished - 6"
+    driver.execute_script "window.confirm = function(msg) { return true; }"
+    driver.find_element(:css, "#publish_grades_link").click
+    wait_for_ajaximations
+    driver.find_element(:css, "#publish_grades_messages").text.should == (wait_for_success ? "Publishing - 6" : "Published - 6")
+
+    server_thread.join
+    post_lines.should == [
+        "POST /endpoint HTTP/1.1",
+        "Accept: */*",
+        "Content-Type: text/csv",
+        "",
+        "publisher_id,publisher_sis_id,section_id,section_sis_id,student_id," +
+        "student_sis_id,enrollment_id,enrollment_status,grade,score\n" +
+        "#{@teacher.id},T1,#{getsection("S1").id},S1,#{getpseudonym("S1").id},S1,#{getenroll("S1", "S1").id},active,C-,70\n" +
+        "#{@teacher.id},T1,#{getsection("S2").id},S2,#{getpseudonym("S2").id},S2,#{getenroll("S2", "S2").id},active,C,75\n" +
+        "#{@teacher.id},T1,#{getsection("S2").id},S2,#{getpseudonym("S3").id},S3,#{getenroll("S3", "S2").id},active,B-,80\n" +
+        "#{@teacher.id},T1,#{getsection("S1").id},S1,#{getpseudonym("S4").id},S4,#{getenroll("S4", "S1").id},active,F,0\n" +
+        "#{@teacher.id},T1,#{getsection("S3").id},S3,#{@stud5.id},,#{Enrollment.find_by_user_id_and_course_section_id(@stud5.user.id, getsection("S3").id).id},active,B,85\n" +
+        "#{@teacher.id},T1,#{@sec4.id},,#{@stud6.id},,#{Enrollment.find_by_user_id_and_course_section_id(@stud6.user.id, @sec4.id).id},active,A-,90\n"]
+  end
+
+  it "should support grade submission" do
+    grade_passback_setup(false)
+  end
+
+  it "should support grade submission and result writeback" do
+    grade_passback_setup(true)
+    process_csv_data_cleanly(
+      "enrollment_id,grade_publishing_status,message",
+      "#{getenroll("S1", "S1").id},published,",
+      "#{getenroll("S2", "S2").id},published,",
+      "#{getenroll("S3", "S2").id},published,Grade modified",
+      "#{getenroll("S4", "S1").id},error,Invalid user",
+      "#{Enrollment.find_by_user_id_and_course_section_id(@stud5.user.id, getsection("S3").id).id},error,Invalid user",
+      "#{Enrollment.find_by_user_id_and_course_section_id(@stud6.user.id, @sec4.id).id},error,")
+    keep_trying_until { driver.find_element(:css, "#publish_grades_messages").text.strip.split("\n").to_set == "Error: Invalid user - 2\nPublished - 2\nPublished: Grade modified - 1\nError - 1".split("\n").to_set }
+  end
+end