diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index c5b0d03f..812c5fb7 100644 --- a/app/controllers/courses_controller.rb +++ b/app/controllers/courses_controller.rb @@ -5,7 +5,7 @@ class CoursesController < ApplicationController before_action :determine_user_role def index - teacher_courses = UserToCourse.includes(:course).where(user: @user, role: %w[teacher ta]) + teacher_courses = UserToCourse.includes(:course).where(user: @user, role: UserToCourse.staff_roles) @teacher_courses_by_semester = group_by_semester(teacher_courses) # Only show courses to students if extensions are enabled at the course level @@ -44,16 +44,14 @@ def new @courses = Course.fetch_courses(token) flash[:alert] = 'No courses found.' if @courses.empty? - # Collect unique semester names from Canvas term data for the filter dropdown @semesters = @courses.filter_map { |c| c.dig('term', 'name') }.uniq.sort @selected_semester = params[:semester] - teacher_enrollment_types = %w[teacher ta] # TODO: Add spec for when a course is created, but the user is not enrolled in it. # TODO: Why do some courses have empty enrollments? existing_canvas_ids = @user.courses.pluck(:canvas_id) - @courses_teacher = filter_courses(@courses, teacher_enrollment_types, existing_canvas_ids) - @courses_student = filter_courses(@courses, [ 'student' ], existing_canvas_ids) + @courses_teacher = filter_courses(@courses, UserToCourse.staff_roles, existing_canvas_ids) + @courses_student = filter_courses(@courses, [ UserToCourse::STUDENT_ROLE ], existing_canvas_ids) if @selected_semester.present? @courses_teacher = filter_by_semester(@courses_teacher, @selected_semester) @@ -68,7 +66,7 @@ def edit def create token = @user.lms_credentials.first.token - filter_courses(Course.fetch_courses(token), %w[teacher ta]) + filter_courses(Course.fetch_courses(token), UserToCourse.staff_roles) .select { |c| params[:courses]&.include?(c['id'].to_s) } .each { |course_api| Course.create_or_update_from_canvas(course_api, token, @user) } 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 = []) courses = courses - missing_enrollments - courses.select { |course| exclude_ids.include?(course['id'].to_s) } return [] if courses.empty? - courses.select { |course| course['enrollments'].any? { |e| roles.include?(e['type']) } } + courses.select do |course| + course['enrollments'].any? { |enrollment| roles.include?(UserToCourse.role_from_canvas_enrollment(enrollment)) } + end end def course_data_for_sync diff --git a/app/facades/canvas_facade.rb b/app/facades/canvas_facade.rb index 6d437afd..19e38689 100644 --- a/app/facades/canvas_facade.rb +++ b/app/facades/canvas_facade.rb @@ -1,4 +1,5 @@ require 'date' +require 'cgi' require 'faraday' require 'json' require 'ostruct' @@ -8,6 +9,9 @@ class CanvasFacade < LmsFacade class CanvasAPIError < LmsFacade::LmsAPIError; end CANVAS_URL = ENV.fetch('CANVAS_URL', nil) + CANVAS_CUSTOM_COURSE_ROLES = { + UserToCourse::LEAD_TA_ROLE => 'Lead TA' + }.freeze # Canvas instances can scope the flextensions developer key. # There must be one scope for each endpoint we can use. @@ -161,10 +165,10 @@ def get_all_course_users(course, role = nil) # sigh, manually construct query string until we tweak Faraday middleware # to include :url_encoded, then use `'enrollment_type[]' : list_or_string` query_string = 'per_page=100' - query_string += "&enrollment_type[]=#{role}" if role.is_a?(String) && role.present? + query_string += "{role_query_param(role)}" if role.is_a?(String) && role.present? if role.is_a?(Array) && role.present? # rubocop:disable Style/IfUnlessModifier - query_string += role.map { |r| "&enrollment_type[]=#{r}" }.join + query_string += role.map { |r| "{role_query_param(r)}" }.join end depaginate_response(@canvas_conn.get("courses/#{course.canvas_id}/users?#{query_string}")) @@ -189,6 +193,17 @@ def get_instructor_courses teacher_courses + ta_courses end + def role_query_param(role) + normalized_role = UserToCourse.normalize_role(role) + canvas_course_role = CANVAS_CUSTOM_COURSE_ROLES[normalized_role] + + if canvas_course_role + "enrollment_role=#{CGI.escape(canvas_course_role)}" + else + "enrollment_type[]=#{CGI.escape(normalized_role)}" + end + end + ## # Gets a specified course that the authorized user has access to. # diff --git a/app/javascript/controllers/enrollments_controller.js b/app/javascript/controllers/enrollments_controller.js index cd03aecb..cad253ac 100644 --- a/app/javascript/controllers/enrollments_controller.js +++ b/app/javascript/controllers/enrollments_controller.js @@ -11,7 +11,7 @@ export default class extends Controller { if (!DataTable.isDataTable('#enrollments-table')) { // Define a custom sorting function for the Role column DataTable.ext.type.order['role-pre'] = function (data) { - const rolePriority = { teacher: 4, ta: 2, student: 3 }; + const rolePriority = { teacher: 4, leadta: 3, "lead ta": 3, ta: 2, student: 1 }; if (typeof data !== 'string') { data = String(data).trim(); } diff --git a/app/models/course.rb b/app/models/course.rb index 9f4ac9b7..f9ce422a 100644 --- a/app/models/course.rb +++ b/app/models/course.rb @@ -87,8 +87,8 @@ def enabled_assignments # Or is user.staff_role?(course) or user.student_role?(course) better? def user_role(user) roles = UserToCourse.where(user_id: user.id, course_id: id).pluck(:role) - return 'instructor' if roles.include?('teacher') || roles.include?('ta') - return 'student' if roles.include?('student') + return 'instructor' if roles.intersect?(UserToCourse.staff_roles) + return 'student' if roles.include?(UserToCourse::STUDENT_ROLE) nil end @@ -118,11 +118,11 @@ def assignments end def students - user_to_courses.where(role: 'student').map(&:user) + user_to_courses.where(role: UserToCourse::STUDENT_ROLE).map(&:user) end def instructors - user_to_courses.where(role: 'teacher').map(&:user) + user_to_courses.where(role: UserToCourse::TEACHER_ROLE).map(&:user) end def staff_users @@ -257,7 +257,7 @@ def sync_users_from_canvas(user, roles = [ 'student' ]) end def sync_all_enrollments_from_canvas(user) - sync_users_from_canvas(user, [ 'teacher', 'ta', 'student' ]) + sync_users_from_canvas(user, UserToCourse.roles) end def regenerate_readonly_api_token_if_blank diff --git a/app/models/user_to_course.rb b/app/models/user_to_course.rb index abb63fa2..33b983a5 100644 --- a/app/models/user_to_course.rb +++ b/app/models/user_to_course.rb @@ -23,6 +23,16 @@ # # TODO: In the future we should name this CourseEnrollment class UserToCourse < ApplicationRecord + STUDENT_ROLE = 'student'.freeze + TEACHER_ROLE = 'teacher'.freeze + TA_ROLE = 'ta'.freeze + LEAD_TA_ROLE = 'leadta'.freeze + STAFF_ROLES = [ TEACHER_ROLE, TA_ROLE, LEAD_TA_ROLE ].freeze + COURSE_ADMIN_ROLES = [ TEACHER_ROLE, LEAD_TA_ROLE ].freeze + ROLE_LABELS = { + LEAD_TA_ROLE => 'Lead TA' + }.freeze + # Associations belongs_to :user belongs_to :course @@ -39,18 +49,48 @@ def staff? end def course_admin? - role == 'teacher' || role == 'leadta' + UserToCourse.course_admin_roles.include?(role) end def student? - role == 'student' + role == STUDENT_ROLE + end + + def display_role + UserToCourse.display_role(role) end def self.roles - [ 'student' ] + UserToCourse.staff_roles + [ STUDENT_ROLE ] + UserToCourse.staff_roles end def self.staff_roles - %w[teacher ta leadta] + STAFF_ROLES + end + + def self.course_admin_roles + COURSE_ADMIN_ROLES + end + + def self.normalize_role(role) + role.to_s.downcase.gsub(/[^a-z]/, '') + end + + def self.role_from_canvas_enrollment(enrollment) + return nil unless enrollment + + normalized_role = normalize_role(enrollment['role'] || enrollment[:role]) + return LEAD_TA_ROLE if normalized_role == LEAD_TA_ROLE + + normalized_type = normalize_role(enrollment['type'] || enrollment[:type]) + roles.include?(normalized_type) ? normalized_type : nil + end + + def self.staff_enrollment?(enrollment) + staff_roles.include?(role_from_canvas_enrollment(enrollment)) + end + + def self.display_role(role) + ROLE_LABELS.fetch(role.to_s, role.to_s.capitalize) end end diff --git a/app/views/courses/enrollments.html.erb b/app/views/courses/enrollments.html.erb index 80750df6..cf2ee520 100644 --- a/app/views/courses/enrollments.html.erb +++ b/app/views/courses/enrollments.html.erb @@ -51,7 +51,7 @@