Skip to content

Commit 3fcd66d

Browse files
committed
Edit Sections: gray out sections the user can't remove
test plan: 1. enroll a user in a course via a SIS import (or just set a sis_source_id on the enrollment via console) 2. as a teacher who is not an account admin, go to the People page and click the gear menu and Edit Sections 3. the SIS enrollment should be grayed out, with no X button, and hover text stating "You cannot remove this enrollment." 4. you should be able to add/remove other non-SIS enrollments (with and without refreshing the people page) 5. "Remove From Course" should not appear in the gear menu if any non-deleteable enrollments exist 6. a teacher who *is* an account admin should be able to delete the SIS enrollment fixes CNVS-23009 Change-Id: I203286c537f7a2aede8d12b039ee47ec86a36a9b Reviewed-on: https://gerrit.instructure.com/64771 Tested-by: Jenkins Reviewed-by: Dan Minkevitch <dan@instructure.com> QA-Review: Clare Strong <clare@instructure.com> Product-Review: Cosme Salazar <cosme@instructure.com>
1 parent 2a39e94 commit 3fcd66d

5 files changed

Lines changed: 50 additions & 27 deletions

File tree

app/coffeescripts/views/courses/roster/EditSectionsView.coffee

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ define [
5656
$sections = @$('#user_sections')
5757
for e in @model.sectionEditableEnrollments()
5858
if section = ENV.CONTEXTS['sections'][e.course_section_id]
59-
$sections.append sectionTemplate(id: section.id, name: section.name, role: e.role)
59+
$sections.append sectionTemplate(id: section.id, name: section.name, role: e.role, can_be_removed: e.can_be_removed)
6060

6161
onNewToken: ($token) =>
6262
$link = $token.find('a')
@@ -86,6 +86,7 @@ define [
8686
if enrollment.role != enrollment.type
8787
data.enrollment.role = enrollment.role
8888
deferreds.push $.ajaxJSON url, 'POST', data, (newEnrollment) =>
89+
_.extend newEnrollment, { can_be_removed: true }
8990
newEnrollments.push newEnrollment
9091

9192
# delete old section enrollments

app/coffeescripts/views/courses/roster/RosterUserView.coffee

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,7 @@ define [
5050
json.url = "#{ENV.COURSE_ROOT_URL}/users/#{@model.get('id')}"
5151
json.isObserver = @model.hasEnrollmentType('ObserverEnrollment')
5252
json.isPending = @model.pending(@model.currentRole)
53-
json.canRemoveUsers = ENV.permissions.manage_admin_users or (ENV.permissions.manage_students and _.any @model.get('enrollments'), (e) -> e.can_be_removed)
53+
json.canRemoveUsers = _.all @model.get('enrollments'), (e) -> e.can_be_removed
5454
json.canEditSections = not _.isEmpty @model.sectionEditableEnrollments()
5555
json.canLinkStudents = json.isObserver && !ENV.course.concluded
5656
json.canViewLoginIdColumn = ENV.permissions.manage_admin_users or ENV.permissions.manage_students

app/stylesheets/base/mixins/_bubbles.scss

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,14 @@
3232
border-radius: 10px;
3333
cursor: default;
3434
position: relative;
35+
&.cannot_remove, &.cannot_remove:hover {
36+
background-color: $gray-light;
37+
border-color: $gray-lighter;
38+
div {
39+
background-color: $gray-lighter;
40+
color: $gray;
41+
}
42+
}
3543
&:hover {
3644
div {
3745
background-color: #bccef4;
Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,13 @@
1-
<li>
2-
<div class="ellipsis" title="{{name}} - {{enrollmentName role}}">{{name}} - {{enrollmentName role}}</div>
3-
<a href="#" title="{{#t "remove_user_from_course_section"}}Remove user from {{name}}{{/t}}">
4-
<span class="screenreader-only">
5-
{{#t "remove_user_from_course_section"}}Remove user from {{name}}{{/t}}
6-
</span>
7-
</a>
1+
<li class="{{#unless can_be_removed}}cannot_remove{{/unless}}">
2+
{{#if can_be_removed}}
3+
<div class="ellipsis" title="{{name}} - {{enrollmentName role}}">{{name}} - {{enrollmentName role}}</div>
4+
<a href="#" title="{{#t "remove_user_from_course_section"}}Remove user from {{name}}{{/t}}">
5+
<span class="screenreader-only">
6+
{{#t "remove_user_from_course_section"}}Remove user from {{name}}{{/t}}
7+
</span>
8+
</a>
9+
{{else}}
10+
<div class="ellipsis" title="{{#t}}You cannot remove this enrollment.{{/t}}">{{name}} - {{enrollmentName role}}</div>
11+
{{/if}}
812
<input type="hidden" name="sections[]" value="section_{{id}}">
913
</li>

spec/selenium/people_spec.rb

Lines changed: 28 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -347,22 +347,10 @@ def link_to_student(enrollment, student)
347347
end
348348
end
349349

350-
def add_a_section
351-
section_name = 'section2'
352-
get "/courses/#{@course.id}/settings#tab-sections"
353-
354-
section_input = f('#course_section_name')
355-
keep_trying_until { expect(section_input).to be_displayed }
356-
replace_content(section_input, section_name)
357-
submit_form('#add_section_form')
358-
wait_for_ajaximations
359-
expect(ff('#sections > .section')[1]).to include_text(section_name)
360-
end
361-
362350
context "course with multiple sections", priority: "2" do
363351
before (:each) do
364-
course_with_admin_logged_in
365-
add_a_section
352+
course_with_teacher_logged_in
353+
@section2 = @course.course_sections.create!(name: 'section2')
366354
end
367355

368356
it "should save add people form data" do
@@ -408,11 +396,9 @@ def add_a_section
408396
end
409397

410398
it "should remove a student from a section", priority: "1", test_id: 296461 do
411-
sec1 = @course.course_sections.create!(name: "section1")
412-
sec2 = @course.course_sections.create!(name: "section2")
413399
@student = user
414-
@course.enroll_student(@student, section: sec1, allow_multiple_enrollments: true)
415-
@course.enroll_student(@student, section: sec2, allow_multiple_enrollments: true)
400+
@course.enroll_student(@student, allow_multiple_enrollments: true)
401+
@course.enroll_student(@student, section: @section2, allow_multiple_enrollments: true)
416402
get "/courses/#{@course.id}/users"
417403
ff(".icon-settings")[1].click
418404
fln("Edit Sections").click
@@ -421,6 +407,30 @@ def add_a_section
421407
wait_for_ajaximations
422408
expect(ff(".StudentEnrollment")[0].text).not_to include_text("section2")
423409
end
410+
411+
it "should gray out sections the user doesn't have permission to remove" do
412+
@student = user_with_pseudonym
413+
@course.enroll_student(@student, allow_multiple_enrollments: true).update_attribute(:sis_source_id, 'E001')
414+
get "/courses/#{@course.id}/users"
415+
ff(".icon-settings")[1].click
416+
fln("Edit Sections").click
417+
expect(f('#user_sections li.cannot_remove').text).to include @course.default_section.name
418+
419+
# add another section (not via SIS) and ensure it remains editable
420+
f(".token_input.browsable").click
421+
section_input_element = driver.find_element(:name, "token_capture")
422+
section_input_element.send_keys("section2")
423+
f('.last.context').click
424+
wait_for_ajaximations
425+
expect(f("a[title='Remove user from section2']")).not_to be_nil
426+
f('.ui-dialog-buttonset .btn-primary').click
427+
wait_for_ajaximations
428+
429+
ff(".icon-settings")[1].click
430+
fln("Edit Sections").click
431+
expect(f('#user_sections li.cannot_remove').text).to include @course.default_section.name
432+
expect(f("a[title='Remove user from section2']")).not_to be_nil
433+
end
424434
end
425435

426436
it "should get the max total activity time" do

0 commit comments

Comments
 (0)