Skip to content

Commit 2f51943

Browse files
AlexAlex
authored andcommitted
Support Lead TA course role
1 parent 4d082b4 commit 2f51943

12 files changed

Lines changed: 195 additions & 26 deletions

File tree

app/controllers/courses_controller.rb

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ class CoursesController < ApplicationController
55
before_action :determine_user_role
66

77
def index
8-
teacher_courses = UserToCourse.includes(:course).where(user: @user, role: %w[teacher ta])
8+
teacher_courses = UserToCourse.includes(:course).where(user: @user, role: UserToCourse.staff_roles)
99
@teacher_courses_by_semester = group_by_semester(teacher_courses)
1010

1111
# Only show courses to students if extensions are enabled at the course level
@@ -44,16 +44,14 @@ def new
4444
@courses = Course.fetch_courses(token)
4545
flash[:alert] = 'No courses found.' if @courses.empty?
4646

47-
# Collect unique semester names from Canvas term data for the filter dropdown
4847
@semesters = @courses.filter_map { |c| c.dig('term', 'name') }.uniq.sort
4948
@selected_semester = params[:semester]
5049

51-
teacher_enrollment_types = %w[teacher ta]
5250
# TODO: Add spec for when a course is created, but the user is not enrolled in it.
5351
# TODO: Why do some courses have empty enrollments?
5452
existing_canvas_ids = @user.courses.pluck(:canvas_id)
55-
@courses_teacher = filter_courses(@courses, teacher_enrollment_types, existing_canvas_ids)
56-
@courses_student = filter_courses(@courses, [ 'student' ], existing_canvas_ids)
53+
@courses_teacher = filter_courses(@courses, UserToCourse.staff_roles, existing_canvas_ids)
54+
@courses_student = filter_courses(@courses, [ UserToCourse::STUDENT_ROLE ], existing_canvas_ids)
5755

5856
if @selected_semester.present?
5957
@courses_teacher = filter_by_semester(@courses_teacher, @selected_semester)
@@ -68,7 +66,7 @@ def edit
6866

6967
def create
7068
token = @user.lms_credentials.first.token
71-
filter_courses(Course.fetch_courses(token), %w[teacher ta])
69+
filter_courses(Course.fetch_courses(token), UserToCourse.staff_roles)
7270
.select { |c| params[:courses]&.include?(c['id'].to_s) }
7371
.each { |course_api| Course.create_or_update_from_canvas(course_api, token, @user) }
7472
redirect_to courses_path, notice: 'Selected courses and their assignments have been imported successfully.'
@@ -146,7 +144,9 @@ def filter_courses(courses, roles, exclude_ids = [])
146144
courses = courses - missing_enrollments - courses.select { |course| exclude_ids.include?(course['id'].to_s) }
147145
return [] if courses.empty?
148146

149-
courses.select { |course| course['enrollments'].any? { |e| roles.include?(e['type']) } }
147+
courses.select do |course|
148+
course['enrollments'].any? { |enrollment| roles.include?(UserToCourse.role_from_canvas_enrollment(enrollment)) }
149+
end
150150
end
151151

152152
def course_data_for_sync

app/facades/canvas_facade.rb

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
require 'date'
2+
require 'cgi'
23
require 'faraday'
34
require 'json'
45
require 'ostruct'
@@ -8,6 +9,9 @@ class CanvasFacade < LmsFacade
89
class CanvasAPIError < LmsFacade::LmsAPIError; end
910

1011
CANVAS_URL = ENV.fetch('CANVAS_URL', nil)
12+
CANVAS_CUSTOM_COURSE_ROLES = {
13+
UserToCourse::LEAD_TA_ROLE => 'Lead TA'
14+
}.freeze
1115

1216
# Canvas instances can scope the flextensions developer key.
1317
# There must be one scope for each endpoint we can use.
@@ -161,10 +165,10 @@ def get_all_course_users(course, role = nil)
161165
# sigh, manually construct query string until we tweak Faraday middleware
162166
# to include :url_encoded, then use `'enrollment_type[]' : list_or_string`
163167
query_string = 'per_page=100'
164-
query_string += "&enrollment_type[]=#{role}" if role.is_a?(String) && role.present?
168+
query_string += "&#{role_query_param(role)}" if role.is_a?(String) && role.present?
165169

166170
if role.is_a?(Array) && role.present? # rubocop:disable Style/IfUnlessModifier
167-
query_string += role.map { |r| "&enrollment_type[]=#{r}" }.join
171+
query_string += role.map { |r| "&#{role_query_param(r)}" }.join
168172
end
169173

170174
depaginate_response(@canvas_conn.get("courses/#{course.canvas_id}/users?#{query_string}"))
@@ -189,6 +193,17 @@ def get_instructor_courses
189193
teacher_courses + ta_courses
190194
end
191195

196+
def role_query_param(role)
197+
normalized_role = UserToCourse.normalize_role(role)
198+
canvas_course_role = CANVAS_CUSTOM_COURSE_ROLES[normalized_role]
199+
200+
if canvas_course_role
201+
"enrollment_role=#{CGI.escape(canvas_course_role)}"
202+
else
203+
"enrollment_type[]=#{CGI.escape(normalized_role)}"
204+
end
205+
end
206+
192207
##
193208
# Gets a specified course that the authorized user has access to.
194209
#

app/javascript/controllers/enrollments_controller.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ export default class extends Controller {
1111
if (!DataTable.isDataTable('#enrollments-table')) {
1212
// Define a custom sorting function for the Role column
1313
DataTable.ext.type.order['role-pre'] = function (data) {
14-
const rolePriority = { teacher: 4, ta: 2, student: 3 };
14+
const rolePriority = { teacher: 4, leadta: 3, "lead ta": 3, ta: 2, student: 1 };
1515
if (typeof data !== 'string') {
1616
data = String(data).trim();
1717
}

app/models/course.rb

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -87,8 +87,8 @@ def enabled_assignments
8787
# Or is user.staff_role?(course) or user.student_role?(course) better?
8888
def user_role(user)
8989
roles = UserToCourse.where(user_id: user.id, course_id: id).pluck(:role)
90-
return 'instructor' if roles.include?('teacher') || roles.include?('ta')
91-
return 'student' if roles.include?('student')
90+
return 'instructor' if roles.intersect?(UserToCourse.staff_roles)
91+
return 'student' if roles.include?(UserToCourse::STUDENT_ROLE)
9292

9393
nil
9494
end
@@ -118,11 +118,11 @@ def assignments
118118
end
119119

120120
def students
121-
user_to_courses.where(role: 'student').map(&:user)
121+
user_to_courses.where(role: UserToCourse::STUDENT_ROLE).map(&:user)
122122
end
123123

124124
def instructors
125-
user_to_courses.where(role: 'teacher').map(&:user)
125+
user_to_courses.where(role: UserToCourse::TEACHER_ROLE).map(&:user)
126126
end
127127

128128
def staff_users
@@ -257,7 +257,7 @@ def sync_users_from_canvas(user, roles = [ 'student' ])
257257
end
258258

259259
def sync_all_enrollments_from_canvas(user)
260-
sync_users_from_canvas(user, [ 'teacher', 'ta', 'student' ])
260+
sync_users_from_canvas(user, UserToCourse.roles)
261261
end
262262

263263
def regenerate_readonly_api_token_if_blank

app/models/user_to_course.rb

Lines changed: 44 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,16 @@
2323
#
2424
# TODO: In the future we should name this CourseEnrollment
2525
class UserToCourse < ApplicationRecord
26+
STUDENT_ROLE = 'student'.freeze
27+
TEACHER_ROLE = 'teacher'.freeze
28+
TA_ROLE = 'ta'.freeze
29+
LEAD_TA_ROLE = 'leadta'.freeze
30+
STAFF_ROLES = [ TEACHER_ROLE, TA_ROLE, LEAD_TA_ROLE ].freeze
31+
COURSE_ADMIN_ROLES = [ TEACHER_ROLE, LEAD_TA_ROLE ].freeze
32+
ROLE_LABELS = {
33+
LEAD_TA_ROLE => 'Lead TA'
34+
}.freeze
35+
2636
# Associations
2737
belongs_to :user
2838
belongs_to :course
@@ -39,18 +49,48 @@ def staff?
3949
end
4050

4151
def course_admin?
42-
role == 'teacher' || role == 'leadta'
52+
UserToCourse.course_admin_roles.include?(role)
4353
end
4454

4555
def student?
46-
role == 'student'
56+
role == STUDENT_ROLE
57+
end
58+
59+
def display_role
60+
UserToCourse.display_role(role)
4761
end
4862

4963
def self.roles
50-
[ 'student' ] + UserToCourse.staff_roles
64+
[ STUDENT_ROLE ] + UserToCourse.staff_roles
5165
end
5266

5367
def self.staff_roles
54-
%w[teacher ta leadta]
68+
STAFF_ROLES
69+
end
70+
71+
def self.course_admin_roles
72+
COURSE_ADMIN_ROLES
73+
end
74+
75+
def self.normalize_role(role)
76+
role.to_s.downcase.gsub(/[^a-z]/, '')
77+
end
78+
79+
def self.role_from_canvas_enrollment(enrollment)
80+
return nil unless enrollment
81+
82+
normalized_role = normalize_role(enrollment['role'] || enrollment[:role])
83+
return LEAD_TA_ROLE if normalized_role == LEAD_TA_ROLE
84+
85+
normalized_type = normalize_role(enrollment['type'] || enrollment[:type])
86+
roles.include?(normalized_type) ? normalized_type : nil
87+
end
88+
89+
def self.staff_enrollment?(enrollment)
90+
staff_roles.include?(role_from_canvas_enrollment(enrollment))
91+
end
92+
93+
def self.display_role(role)
94+
ROLE_LABELS.fetch(role.to_s, role.to_s.capitalize)
5595
end
5696
end

app/views/courses/enrollments.html.erb

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@
5151
</td>
5252
<td class="text-center"><%= enrollment.user.student_id %></td>
5353
<td class="text-center"><%= enrollment.user.email %></td>
54-
<td class="text-center"><%= enrollment.role.downcase.capitalize %></td>
54+
<td class="text-center"><%= enrollment.display_role %></td>
5555
<% if @is_course_admin %>
5656
<td class="justify-content-center align-content-center"
5757
data-order="<%= enrollment.allow_extended_requests ? 1 : 0 %>">
@@ -89,4 +89,4 @@
8989
<% end %>
9090
</div>
9191
</div>
92-
</div>
92+
</div>

app/views/courses/index.html.erb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@
3333
<tr>
3434
<td><a href="/courses/<%= user_to_course.course.id %>"> <%= user_to_course.course.course_name %> </a></td>
3535
<td><%= user_to_course.course.course_code %></td>
36-
<td><%= user_to_course.role.capitalize %></td>
36+
<td><%= user_to_course.display_role %></td>
3737
</tr>
3838
<% end %>
3939
<% end %>

app/views/courses/new.html.erb

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,10 @@
4646
<td><%= course.dig("term", "name") %></td>
4747
<td><%= course["course_code"] %></td>
4848
<td><%= course["name"] %></td>
49-
<td><%= course['enrollments'].find { |enrollment| %w[teacher ta].include?(enrollment['type']) }['type'].capitalize %></td>
49+
<td>
50+
<% staff_enrollment = course['enrollments'].find { |enrollment| UserToCourse.staff_enrollment?(enrollment) } %>
51+
<%= UserToCourse.display_role(UserToCourse.role_from_canvas_enrollment(staff_enrollment)) %>
52+
</td>
5053
</tr>
5154
<% end %>
5255
</tbody>

spec/controllers/courses_controller_spec.rb

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,14 @@
2929
expect(response).to render_template(:index)
3030
end
3131

32+
it 'includes Lead TA enrollments in staff courses' do
33+
UserToCourse.create!(user: user, course: student_course, role: 'leadta')
34+
35+
get :index
36+
37+
expect(assigns(:teacher_courses).map(&:role)).to include('leadta')
38+
end
39+
3240
context 'semester grouping' do
3341
let(:spring_course) { Course.create!(course_name: 'Spring Course', canvas_id: 'sp1', course_code: 'SP101', semester: 'Spring 2026') }
3442
let(:fall_course) { Course.create!(course_name: 'Fall Course', canvas_id: 'fa1', course_code: 'FA101', semester: 'Fall 2025') }
@@ -112,6 +120,21 @@
112120
expect(response).to redirect_to(courses_path)
113121
expect(flash[:notice]).to eq('Selected courses and their assignments have been imported successfully.')
114122
end
123+
124+
it 'imports courses where the user is enrolled with the Canvas Lead TA role' do
125+
lead_ta_course = {
126+
'id' => '999',
127+
'name' => 'Lead TA Canvas Course',
128+
'course_code' => 'LTA101',
129+
'enrollments' => [ { 'type' => 'ta', 'role' => 'Lead TA' } ]
130+
}
131+
allow(Course).to receive(:fetch_courses).and_return([ lead_ta_course ])
132+
allow(Course).to receive(:create_or_update_from_canvas)
133+
134+
post :create, params: { courses: [ '999' ] }
135+
136+
expect(Course).to have_received(:create_or_update_from_canvas).with(lead_ta_course, 'fake_token', user)
137+
end
115138
end
116139

117140
describe 'POST #sync_assignments' do
@@ -139,6 +162,14 @@
139162
headers: { 'Authorization' => 'Bearer fake_token' }
140163
).to_return(status: 200, body: '[]', headers: {})
141164
end
165+
stub_request(:get, "#{ENV.fetch('CANVAS_URL', nil)}/api/v1/courses/456/users")
166+
.with(
167+
query: {
168+
'enrollment_role' => 'Lead TA',
169+
'per_page' => '100'
170+
},
171+
headers: { 'Authorization' => 'Bearer fake_token' }
172+
).to_return(status: 200, body: '[]', headers: {})
142173
end
143174

144175
context 'when user is a teacher (course admin)' do
@@ -215,7 +246,7 @@
215246
'id' => '103',
216247
'name' => 'Test Course 103',
217248
'course_code' => 'TC103',
218-
'enrollments' => [ { 'type' => 'teacher' } ],
249+
'enrollments' => [ { 'type' => 'ta', 'role' => 'Lead TA' } ],
219250
'term' => { 'name' => 'Fall 2025' }
220251
},
221252
{
@@ -246,8 +277,9 @@
246277
expect(assigns(:courses_student)).not_to be_empty
247278

248279
# Teacher course should be categorized correctly
249-
teacher_course = assigns(:courses_teacher).first
250-
expect(teacher_course['enrollments'].first['type']).to eq('teacher')
280+
teacher_course_roles = assigns(:courses_teacher).map { |canvas_course| canvas_course['enrollments'].first }
281+
expect(teacher_course_roles).to include(hash_including('type' => 'teacher'))
282+
expect(teacher_course_roles).to include(hash_including('role' => 'Lead TA'))
251283

252284
# Student course should be categorized correctly
253285
student_course = assigns(:courses_student).first

spec/facades/canvas_facade_spec.rb

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,24 @@
169169
end
170170
end
171171

172+
describe '#get_all_course_users' do
173+
let(:course) { instance_double(Course, canvas_id: course_id) }
174+
175+
it 'uses enrollment_type for built-in Canvas roles' do
176+
stubs.get("courses/#{course_id}/users?per_page=100&enrollment_type[]=ta") { [ 200, {}, '[]' ] }
177+
178+
expect(facade.get_all_course_users(course, 'ta')).to eq([])
179+
stubs.verify_stubbed_calls
180+
end
181+
182+
it 'uses enrollment_role for the custom Lead TA Canvas role' do
183+
stubs.get("courses/#{course_id}/users?per_page=100&enrollment_role=Lead+TA") { [ 200, {}, '[]' ] }
184+
185+
expect(facade.get_all_course_users(course, 'leadta')).to eq([])
186+
stubs.verify_stubbed_calls
187+
end
188+
end
189+
172190
describe 'get_course' do
173191
before do
174192
stubs.get("courses/#{course_id}") { [ 200, {}, '{}' ] }

0 commit comments

Comments
 (0)