edit/delete mult enroll on course settings page; fixes #6996
We want to be able to change or delete enrollments on the course settings page, even if there are multiple enrollments for a single student. This commit adds this ability, and adds tests to ensure that multiple enrollments are coalesed under a single user. test-plan: - create a course with three sections - enroll a student in two of these sections (from the console) - navigate to the course settings page, users tab - make sure the student only shows up once, with both enrollments listed underneath - try changing one of these enrollments to the third section. - try deleting one, and then both of these enrollments Change-Id: I4bc35ac2f79fd6297b50e5217a567290eff59a92 Reviewed-on: https://gerrit.instructure.com/8364 Tested-by: Hudson <hudson@instructure.com> Reviewed-by: Brian Palmer <brianp@instructure.com>
This commit is contained in:
Simon Williams
parent
a30a1b8d9f
commit
e68f1beecc
|
|
@ -859,15 +859,22 @@ class CoursesController < ApplicationController
|
|||
@enrollment = @context.enrollments.find(params[:id])
|
||||
can_move = [StudentEnrollment, ObserverEnrollment].include?(@enrollment.class) && @context.grants_right?(@current_user, session, :manage_students)
|
||||
can_move ||= @context.grants_right?(@current_user, session, :manage_admin_users)
|
||||
can_move &&= @context.grants_right?(@current_user, session, :manage_account_settings) if @enrollment.defined_by_sis?
|
||||
if can_move
|
||||
respond_to do |format|
|
||||
if @enrollment.defined_by_sis? &&! @context.grants_right?(@current_user, session, :manage_account_settings)
|
||||
return format.json { render :json => @enrollment.to_json, :status => :bad_request }
|
||||
end
|
||||
@enrollment.course_section = @context.course_sections.find(params[:course_section_id])
|
||||
@enrollment.save!
|
||||
# ensure user_id,section_id,type,associated_user_id is unique (this
|
||||
# will become a DB constraint eventually)
|
||||
@possible_dup = @context.enrollments.find(:first, :conditions =>
|
||||
["id <> ? AND user_id = ? AND course_section_id = ? AND type = ? AND (associated_user_id IS NULL OR associated_user_id = ?)",
|
||||
@enrollment.id, @enrollment.user_id, params[:course_section_id], @enrollment.type, @enrollment.associated_user_id])
|
||||
if @possible_dup.present?
|
||||
format.json { render :json => @enrollment.to_json, :status => :forbidden }
|
||||
else
|
||||
@enrollment.course_section = @context.course_sections.find(params[:course_section_id])
|
||||
@enrollment.save!
|
||||
|
||||
format.json { render :json => @enrollment.to_json }
|
||||
format.json { render :json => @enrollment.to_json }
|
||||
end
|
||||
end
|
||||
else
|
||||
authorized_action(@context, @current_user, :permission_fail)
|
||||
|
|
|
|||
|
|
@ -2,7 +2,8 @@
|
|||
<% include_type ||= false; show_information_link ||= false %>
|
||||
<% enrollments = enrollment[1].sort_by(&:id) if enrollment.is_a?(Array) %>
|
||||
<% enrollment = enrollment[1][0] if enrollment.is_a?(Array) %>
|
||||
<li id="enrollment_<%= enrollment.try_rescue(:id) || "blank" %>" class="user <%= "pending" if enrollment.try_rescue(:pending?) %> <%= enrollment.class.to_s.underscore %> user_<%= enrollment.try_rescue(:user_id) %>"
|
||||
<% multiple_enrollments = enrollments && enrollments.length > 1 %>
|
||||
<li id="enrollment_<%= enrollment.try_rescue(:id) || "blank" %>" class="user <%= "pending" if enrollment.try_rescue(:pending?) %> <%= enrollment.class.to_s.underscore %> user_<%= enrollment.try_rescue(:user_id) %>"
|
||||
<% if enrollment.try_rescue(:user) %>
|
||||
title="<%= enrollment.user.name %>: <%= enrollment.user.email %>"
|
||||
<% end %>>
|
||||
|
|
@ -25,8 +26,8 @@
|
|||
<a href="#" class="edit_section_link no-hover"><%= image_tag "edit.png" %></a>
|
||||
<% end %>
|
||||
<% end %>
|
||||
<% if allowedbypermissions %>
|
||||
<a href="<%= context_url(@context, :context_unenroll_url, (enrollment.try_rescue(:id) || "{{ id }}" ) ) %>" class="unenroll_user_link <%= cant_unenroll %> no-hover" title="<%= t 'links.unenroll_user', "Remove User from Course" %>"><%= image_tag unenroll_image %></a>
|
||||
<% if allowedbypermissions && !multiple_enrollments %>
|
||||
<a href="<%= context_url(@context, :context_unenroll_url, (enrollment.try_rescue(:id) || "{{ id }}" ) ) %>" class="unenroll_user_link <%= cant_unenroll %> no-hover" title="<%= t 'links.unenroll_user_course', "Remove User from Course" %>"><%= image_tag unenroll_image %></a>
|
||||
<% end %>
|
||||
<a href="<%= re_send_confirmation_url( enrollment.try_rescue(:user_id) || "{{ user_id }}", enrollment.try_rescue(:user).try_rescue(:communication_channel).try_rescue(:id) || "{{ communication_channel_id }}", :enrollment_id => enrollment.try_rescue(:id) || "{{ id }}" ) rescue "#" %>" class="re_send_confirmation_url" style="display: none;"> </a>
|
||||
</span>
|
||||
|
|
@ -81,20 +82,27 @@
|
|||
</div>
|
||||
<% end %>
|
||||
<% if show_section && enrollments %>
|
||||
<div class="sections">
|
||||
<% enrollments.each do |enrollment| %>
|
||||
<% if enrollment && enrollment.course_section %>
|
||||
<div class="section"><%= enrollment.course_section.display_name %></div>
|
||||
<% unless lockedbysis %>
|
||||
<% form_tag course_move_enrollment_url(@context, enrollment.id), {:class => "enrollment_course_section_form", :style => "display: none;"} do %>
|
||||
<select title="<%= t 'titles.course_section', "Course Section" %>" name="course_section_id" id="course_section_id">
|
||||
<% @context.course_sections.active.each do |section| %>
|
||||
<option value="<%= section.id %>" class="option_for_section_<%= section.id %>" <%= "selected=selected" if enrollment.course_section_id==section.id %>><%= section.display_name %></option>
|
||||
<div class="section section_<%= enrollment.course_section_id %>">
|
||||
<div class="section_name"><%= enrollment.course_section.display_name %></div>
|
||||
<% unless lockedbysis %>
|
||||
<% form_tag course_move_enrollment_url(@context, enrollment.id), {:class => "enrollment_course_section_form", :style => "display: none;"} do %>
|
||||
<select title="<%= t 'titles.course_section', "Course Section" %>" name="course_section_id" class="course_section_id">
|
||||
<% @context.course_sections.active.each do |section| %>
|
||||
<option value="<%= section.id %>" class="option_for_section_<%= section.id %>" <%= "selected=selected" if enrollment.course_section_id==section.id %>><%= section.display_name %></option>
|
||||
<% end %>
|
||||
</select>
|
||||
<% if allowedbypermissions && multiple_enrollments %>
|
||||
<a href="<%= context_url(@context, :context_unenroll_url, (enrollment.try_rescue(:id) || "{{ id }}" ) ) %>" class="unenroll_user_link <%= cant_unenroll %> no-hover" title="<%= t 'links.unenroll_user_section', "Remove User from Section" %>"><%= image_tag unenroll_image %></a>
|
||||
<% end %>
|
||||
</select>
|
||||
<% end %>
|
||||
<% end %>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
<% end %>
|
||||
</div>
|
||||
<% end %>
|
||||
<span class="invitation_sent_at" style="display: none;"><%= datetime_string(enrollment.try_rescue(:updated_at)) || nbsp %></span>
|
||||
<span class="id" style="display: none;"><%= enrollment.try_rescue(:id) %></span>
|
||||
|
|
|
|||
|
|
@ -439,19 +439,24 @@ require([
|
|||
$('.user_list .edit_section_link').click(function(event) {
|
||||
event.preventDefault();
|
||||
var $this = $(this);
|
||||
$user = $this.closest('.user');
|
||||
$user.find('.section').toggle();
|
||||
$user.find('.enrollment_course_section_form').toggle();
|
||||
var $user = $this.parents('.user');
|
||||
var $sections = $user.find('.sections');
|
||||
$sections.find('.section_name').toggle();
|
||||
$sections.find('.enrollment_course_section_form').toggle();
|
||||
});
|
||||
$('.user_list .enrollment_course_section_form #course_section_id').change(function (event) {
|
||||
var form = $(this).parent('form');
|
||||
var $this = $(this)
|
||||
var section = form.prev('.section')
|
||||
$.ajaxJSON(form.attr('action'), 'POST', form.getFormData(), function(data) {
|
||||
section.html($this.find('option[value="' + $this.val() + '"]').html());
|
||||
section.toggle();
|
||||
form.toggle();
|
||||
$('.user_list .enrollment_course_section_form .course_section_id').change(function (event) {
|
||||
var $this = $(this);
|
||||
var $sections = $this.parents('.sections');
|
||||
var $form = $this.parent('form');
|
||||
var $section_name = $form.prev('.section_name');
|
||||
$.ajaxJSON($form.attr('action'), 'POST', $form.getFormData(), function(data) {
|
||||
$section_name.html($this.find('option[value="' + $this.val() + '"]').html());
|
||||
$sections.find('.section_name').toggle();
|
||||
$sections.find('.enrollment_course_section_form').toggle();
|
||||
}, function(data) {
|
||||
if (data && data.enrollment) {
|
||||
$this.val(data.enrollment.course_section_id);
|
||||
}
|
||||
$.flashError(I18n.t('errors.move_user', "Something went wrong moving the user to the new section. Please try again later."));
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -70,10 +70,16 @@ require([
|
|||
$form.find(".user_list").val("");
|
||||
UL.showTextarea();
|
||||
if (!enrollments || !enrollments.length) { return false; }
|
||||
var already_existed = 0;
|
||||
$.each( enrollments, function(){
|
||||
UL.addUserToList(this.enrollment);
|
||||
already_existed += UL.addUserToList(this.enrollment);
|
||||
});
|
||||
$.flashMessage(I18n.t("users_added", { one: "1 user added", other: "%{count} users added" }, { count: enrollments.length }));
|
||||
|
||||
var addedMsg = I18n.t("users_added", { one: "1 user added", other: "%{count} users added" }, { count: enrollments.length - already_existed });
|
||||
if (already_existed > 0) {
|
||||
addedMsg += " " + I18n.t("users_existed", { one: "(1 user already existed)", other: "(%{count} users already existed)" }, { count: already_existed });
|
||||
}
|
||||
$.flashMessage(addedMsg);
|
||||
}, function(data) {
|
||||
$.flashError(I18n.t("users_adding_failed", "Failed to enroll users"));
|
||||
});
|
||||
|
|
@ -88,7 +94,14 @@ require([
|
|||
if($(this).hasClass('cant_unenroll')) {
|
||||
alert(I18n.t("cant_unenroll", "This user was automatically enrolled using the campus enrollment system, so they can't be manually removed. Please contact your system administrator if you have questions."));
|
||||
} else {
|
||||
$(this).parents(".user").confirmDelete({
|
||||
$user = $(this).parents('.user')
|
||||
$sections = $(this).parents('.sections')
|
||||
$section = $(this).parents('.section')
|
||||
var $toDelete = $user;
|
||||
if ($sections.find('.section:visible').size() > 1) {
|
||||
$toDelete = $section;
|
||||
}
|
||||
$toDelete.confirmDelete({
|
||||
message: I18n.t("delete_confirm", "Are you sure you want to remove this user?"),
|
||||
url: $(this).attr('href'),
|
||||
success: function() {
|
||||
|
|
@ -175,7 +188,10 @@ require([
|
|||
return false;
|
||||
}
|
||||
});
|
||||
|
||||
var already_existed = true;
|
||||
if(!$("#enrollment_" + enrollment.id).length) {
|
||||
already_existed = false;
|
||||
var $enrollment = $enrollment_blank
|
||||
.clone(true)
|
||||
.fillTemplateData({
|
||||
|
|
@ -200,6 +216,7 @@ require([
|
|||
.scrollToVisible($enrollment);
|
||||
}
|
||||
UL.updateCounts();
|
||||
return already_existed ? 1 : 0;
|
||||
}
|
||||
|
||||
};
|
||||
|
|
|
|||
|
|
@ -8,23 +8,13 @@ describe "course settings tests" do
|
|||
end
|
||||
|
||||
def add_section(section_name)
|
||||
@course.course_sections.create!(:name => section_name)
|
||||
@course_section = @course.course_sections.create!(:name => section_name)
|
||||
@course.reload
|
||||
end
|
||||
|
||||
def add_user_to_section(username = 'user@example.com', accept_invitation = true)
|
||||
cs = @course.course_sections.create!
|
||||
u = User.create!(:name => username)
|
||||
u.register!
|
||||
if accept_invitation
|
||||
@course.enroll_user(u, 'StudentEnrollment', :section => cs).accept
|
||||
else
|
||||
e = @course.enroll_user(u, 'StudentEnrollment', :section => cs)
|
||||
e.workflow_state = 'active'
|
||||
e.save!
|
||||
end
|
||||
@course.reload
|
||||
username
|
||||
def multiple_student_enrollment
|
||||
@enrollment = @course.student_enrollments.build(:user => @user, :workflow_state => "active", :course_section => @course_section)
|
||||
@enrollment.save!
|
||||
end
|
||||
|
||||
describe "course items" do
|
||||
|
|
@ -137,36 +127,37 @@ describe "course settings tests" do
|
|||
end
|
||||
|
||||
it "should remove a user from a section" do
|
||||
username = add_user_to_section
|
||||
username = "user@example.com"
|
||||
student_in_course_section(:name => username)
|
||||
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
driver.execute_script("$('li.student_enrollment .unenroll_user_link').click()")
|
||||
driver.execute_script("$('#enrollment_#{@enrollment.id} .unenroll_user_link').click()")
|
||||
driver.switch_to.alert.accept
|
||||
keep_trying_until do
|
||||
driver.find_element(:id, 'tab-users').should_not include_text(username)
|
||||
true
|
||||
end
|
||||
wait_for_ajaximations
|
||||
driver.find_element(:id, 'tab-users').should_not include_text(username)
|
||||
end
|
||||
|
||||
it "should move a user to a new section" do
|
||||
section_name = 'Unnamed Course'
|
||||
add_user_to_section
|
||||
section_name = 'Move to Course Section'
|
||||
add_section(section_name)
|
||||
student_in_course_section(:course_section => @course_section)
|
||||
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
driver.execute_script("$('li.student_enrollment .edit_section_link').click()")
|
||||
click_option('#course_section_id', section_name)
|
||||
driver.execute_script("$('#enrollment_#{@enrollment.id} .edit_section_link').click()")
|
||||
click_option("#enrollment_#{@enrollment.id} .course_section_id", section_name)
|
||||
wait_for_ajaximations
|
||||
driver.find_element(:css, 'li.student_enrollment .section').should include_text(section_name)
|
||||
driver.find_element(:css, "#enrollment_#{@enrollment.id} .section").should include_text(section_name)
|
||||
end
|
||||
|
||||
it "should view the users enrollment details" do
|
||||
username = add_user_to_section('user@example.com', true)
|
||||
username = "user@example.com"
|
||||
student_in_course_section(:name => username)
|
||||
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
driver.execute_script("$('li.student_enrollment .user_information_link').click()")
|
||||
driver.execute_script("$('#enrollment_#{@enrollment.id} .user_information_link').click()")
|
||||
enrollment_dialog = driver.find_element(:id, 'enrollment_dialog')
|
||||
enrollment_dialog.should be_displayed
|
||||
enrollment_dialog.should include_text(username + ' has already received and accepted the invitation')
|
||||
|
|
@ -229,4 +220,56 @@ describe "course settings tests" do
|
|||
@obs.enrollments.map {|e| e.associated_user_id}.sort.should == [@students[0].id, @students[1].id]
|
||||
end
|
||||
end
|
||||
|
||||
describe "course users multiple enrollments" do
|
||||
before (:each) do
|
||||
@username = "multiple@example.com"
|
||||
add_section("Section 1")
|
||||
@old_section = @course_section
|
||||
student_in_course_section(:name => @username, :course_section => @course_section)
|
||||
add_section("Section 2")
|
||||
multiple_student_enrollment
|
||||
end
|
||||
|
||||
it "should coalesce multiple enrollments under a single student" do
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
|
||||
driver.find_elements(:css, ".user_#{@user.id} .section").length.should == 2
|
||||
driver.find_elements(:css, ".user_#{@user.id} .links .unenroll_user_link").length.should == 0
|
||||
end
|
||||
|
||||
it "should show individual course section remove icons" do
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
|
||||
driver.execute_script("$('.user_#{@user.id} .edit_section_link').click()")
|
||||
find_all_with_jquery(".user_#{@user.id} .sections .unenroll_user_link:visible").length.should == 2
|
||||
end
|
||||
|
||||
it "should only remove a user from a single section" do
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
|
||||
driver.execute_script("$('.user_#{@user.id} .section_#{@course_section.id} .unenroll_user_link').click()")
|
||||
driver.switch_to.alert.accept
|
||||
wait_for_ajaximations
|
||||
driver.find_element(:id, 'tab-users').should include_text(@username)
|
||||
driver.find_element(:css, ".user_#{@user.id}").should include_text(@old_section.name)
|
||||
end
|
||||
|
||||
it "should change the correct section when editing" do
|
||||
add_section("Section 3")
|
||||
|
||||
get "/courses/#{@course.id}/settings"
|
||||
driver.find_element(:link, 'Users').click
|
||||
|
||||
driver.execute_script("$('.user_#{@user.id} .edit_section_link').click()")
|
||||
click_option(".user_#{@user.id} .section_#{@old_section.id} .course_section_id", @course_section.name)
|
||||
wait_for_ajaximations
|
||||
|
||||
driver.find_element(:css, ".user_#{@user.id}").should_not include_text(@old_section.name)
|
||||
driver.find_element(:css, ".user_#{@user.id}").should include_text(@course_section.name)
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -224,4 +224,4 @@ describe "course wizard" do
|
|||
get "/"
|
||||
driver.find_elements(:link, 'Start a New Course').should be_empty
|
||||
end
|
||||
end
|
||||
end
|
||||
|
|
|
|||
|
|
@ -239,8 +239,8 @@ describe "courses" do
|
|||
|
||||
enrollment = user[:enrollment]
|
||||
enrollment_element = driver.find_element(:css, "#enrollment_#{enrollment.id}")
|
||||
section_label = enrollment_element.find_element(:css, ".section") rescue nil
|
||||
section_dropdown = enrollment_element.find_element(:css, ".enrollment_course_section_form #course_section_id") rescue nil
|
||||
section_label = enrollment_element.find_element(:css, ".section_name") rescue nil
|
||||
section_dropdown = enrollment_element.find_element(:css, ".enrollment_course_section_form .course_section_id") rescue nil
|
||||
edit_section_link = enrollment_element.find_element(:css, ".edit_section_link") rescue nil
|
||||
unenroll_user_link = enrollment_element.find_element(:css, ".unenroll_user_link") rescue nil
|
||||
|
||||
|
|
|
|||
|
|
@ -281,6 +281,17 @@ Spec::Runner.configure do |config|
|
|||
course_with_student(opts)
|
||||
end
|
||||
|
||||
def student_in_course_section(opts={})
|
||||
@course ||= opts[:course] || course(opts)
|
||||
@course_section = opts[:course_section] || @course.course_sections.create!
|
||||
@user = opts[:user] || user(opts)
|
||||
@user.register!
|
||||
@enrollment = @course.enroll_user(@user, 'StudentEnrollment', :section => @course_section)
|
||||
@enrollment.workflow_state = 'active'
|
||||
@enrollment.save!
|
||||
@course.reload
|
||||
end
|
||||
|
||||
def course_with_teacher(opts={})
|
||||
course_with_user('TeacherEnrollment', opts)
|
||||
@teacher = @user
|
||||
|
|
|
|||
Loading…
Reference in New Issue