Refactor permissions logic in controllers and models for clarity, defining roles for students, instructors, TAs, and site admins.

app/controllers/application_controller.rb

@@ -79,8 +79,8 @@ def handle_lms_api_error(error)
 
   def set_pending_request_count
     return unless defined?(@course) && @course.present? && defined?(@user) && @user.present?
-    # only calculating pending requests count if the role is instructor so we don't show it to students
-    return unless @course.user_role(@user) == 'instructor'
+    # Only show pending request count to staff (instructors, lead TAs, TAs, and site admins)
+    return unless @course.staff?(@user)
 
     @pending_requests_count = @course.requests.where(status: 'pending').count
   end
@@ -118,11 +118,26 @@ def set_course
       redirect_to courses_path
       return
     end
-    @role = @course.user_role(@user) if @user
+    if @user
+      @role = @course.user_role(@user)
+      @is_course_admin = @course.course_admin?(@user)
+      @is_staff = @course.staff?(@user)
+    end
+  end
+
+  # Only course admins (instructors/lead TAs) and site admins.
+  # Use for settings, form settings, delete, etc.
+  def ensure_course_admin
+    return if @is_course_admin
+
+    flash[:alert] = 'You do not have access to this page.'
+    redirect_to courses_path
   end
 
-  def ensure_instructor_role
-    return if @role == 'instructor'
+  # All staff (instructors, lead TAs, TAs) and site admins.
+  # Use for viewing enrollments, approving/denying requests, syncing, etc.
+  def ensure_staff_role
+    return if @is_staff || @role == 'instructor'
 
     flash[:alert] = 'You do not have access to this page.'
     redirect_to courses_path

app/controllers/assignments_controller.rb

@@ -2,10 +2,10 @@ class AssignmentsController < ApplicationController
   def toggle_enabled
     @assignment = Assignment.find(params[:id])
     course = @assignment.course_to_lms.course
-    @role = params[:role] || course&.user_role(@user)
+    @role = course&.user_role(current_user)
 
-    unless @role == 'instructor'
-      Rails.logger.error "Role #{@role} does not have permission to toggle assignment enabled status"
+    unless course&.course_admin?(current_user)
+      Rails.logger.error "User #{current_user&.id} does not have permission to toggle assignment enabled status"
       flash.now[:alert] = 'You do not have permission to perform this action.'
       return render json: { redirect_to: course_path(course) }, status: :forbidden
     end

app/controllers/course_settings_controller.rb

@@ -2,7 +2,7 @@ class CourseSettingsController < ApplicationController
   before_action :authenticated!
   before_action :authenticate_user
   before_action :set_course
-  before_action :ensure_instructor_role
+  before_action :ensure_course_admin
   before_action :set_pending_request_count
 
   # Default template settings

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)
 
     # Only show courses to students if extensions are enabled at the course level
     student_courses = UserToCourse.includes(course: :course_settings).where(user: @user, role: 'student')
@@ -57,7 +57,7 @@ def new
 
   def edit
     @side_nav = 'edit'
-    redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @role == 'instructor'
+    redirect_to course_path(@course.id), alert: 'You do not have access to this page.' unless @is_course_admin
   end
 
   def create
@@ -77,22 +77,21 @@ def sync_assignments
 
   def sync_enrollments
     return render json: { error: 'Course not found.' }, status: :not_found unless @course
-    return render json: { error: 'You do not have permission.' }, status: :forbidden unless @is_course_admin
+    return render json: { error: 'You do not have permission.' }, status: :forbidden unless @is_staff
 
     @course.sync_all_enrollments_from_canvas(@user.id)
     render json: { message: 'Users synced successfully.' }, status: :ok
   end
 
   def enrollments
     @side_nav = 'enrollments'
-    return redirect_to courses_path, alert: 'You do not have access to this page.' unless @role == 'instructor'
+    return redirect_to courses_path, alert: 'You do not have access to this page.' unless @is_staff
 
     @enrollments = @course.user_to_courses.includes(:user)
-    @is_course_admin = @course.course_admin?(@user)
   end
 
   def delete
-    return redirect_to courses_path, alert: 'You do not have access to this page.' unless @role == 'instructor'
+    return redirect_to courses_path, alert: 'You do not have access to this page.' unless @is_course_admin
     return redirect_to courses_path, alert: 'Extensions are enabled for this course.' if @course.course_settings&.enable_extensions
 
     assignments = Assignment.where(course_to_lms_id: CourseToLms.where(course_id: @course.id).select(:id))
@@ -117,6 +116,7 @@ def set_course
   def determine_user_role
     @role = @course&.user_role(@user)
     @is_course_admin = @course&.course_admin?(@user) || false
+    @is_staff = @course&.staff?(@user) || false
   end
 
   # Filters Canvas API course hashes by their term name

app/controllers/form_settings_controller.rb

@@ -2,7 +2,7 @@ class FormSettingsController < ApplicationController
   before_action :authenticated!
   before_action :authenticate_user
   before_action :set_course
-  before_action :ensure_instructor_role
+  before_action :ensure_course_admin
   before_action :set_pending_request_count
 
   def edit

app/controllers/requests_controller.rb

@@ -11,7 +11,7 @@ class RequestsController < ApplicationController
   before_action :check_extensions_enabled_for_students, except: [ :export ]
   before_action :ensure_request_is_pending, only: %i[update approve reject]
   before_action :set_request, only: %i[show edit cancel]
-  before_action :check_instructor_permission, only: %i[approve reject mass_approve mass_reject]
+  before_action :check_staff_permission, only: %i[approve reject mass_approve mass_reject]
 
   def index
     @side_nav = 'requests'
@@ -56,7 +56,7 @@ def new
 
   def new_for_student
     @side_nav = 'form'
-    return redirect_to course_requests_path(@course), alert: 'You do not have permission to access this page.' unless @role == 'instructor'
+    return redirect_to course_requests_path(@course), alert: 'You do not have permission to access this page.' unless @course.staff?(@user)
 
     course_to_lmss = @course.all_linked_lmss.pluck(:id)
     return redirect_to courses_path, alert: 'No Canvas LMS data found for this course.' unless course_to_lmss.any?
@@ -91,7 +91,7 @@ def create
   end
 
   def create_for_student
-    return redirect_to course_requests_path(@course), alert: 'You do not have permission to perform this action.' unless @role == 'instructor'
+    return redirect_to course_requests_path(@course), alert: 'You do not have permission to perform this action.' unless @course.staff?(@user)
 
     student = User.find_by(id: params[:request][:user_id])
     return redirect_to new_course_request_path(@course), alert: 'Student not found.' unless student
@@ -196,8 +196,8 @@ def set_request
     redirect_to course_path(@course), alert: 'Request not found.' unless @request
   end
 
-  def check_instructor_permission
-    result = RequestService.check_instructor_permission(@role, course_path(@course))
+  def check_staff_permission
+    result = RequestService.check_staff_permission(@role, course_path(@course))
     redirect_to result[:redirect_to], alert: result[:alert] if result != true
   end
 

app/controllers/user_to_courses_controller.rb

@@ -17,8 +17,7 @@ def toggle_allow_extended_requests
   private
 
   def ensure_course_admin
-    enrollment = @course.user_to_courses.find_by(user: @user)
-    return if enrollment&.course_admin?
+    return if @course.course_admin?(@user)
 
     render json: { error: 'You must be an instructor or Lead TA.', redirect_to: course_path(@course) }, status: :forbidden
   end

app/models/course.rb

@@ -62,18 +62,30 @@ def enabled_assignments
     assignments.where(enabled: true)
   end
 
-  # TODO: Replace this with staff_role?(user) or student_role?(user)
-  # Or is user.staff_role?(course) or user.student_role?(course) better?
+  # Returns the view-level role for rendering instructor vs student templates.
+  # Site admins always see the instructor view.
   def user_role(user)
+    return 'instructor' if user.admin?
+
     roles = UserToCourse.where(user_id: user.id, course_id: id).pluck(:role)
-    return 'instructor' if roles.include?('teacher') || roles.include?('ta')
+    return 'instructor' if roles.any? { |r| UserToCourse.staff_roles.include?(r) }
     return 'student' if roles.include?('student')
 
     nil
   end
 
+  # Course admins: instructors (teachers) and lead TAs.
+  # Can manage everything in a course (settings, form, extended circumstances, delete, etc.).
+  # Site admins are always course admins.
   def course_admin?(user)
-    user_to_courses.where(user_id: user.id).any?(&:course_admin?)
+    user.admin? || user_to_courses.where(user_id: user.id).any?(&:course_admin?)
+  end
+
+  # All staff: instructors, lead TAs, and TAs.
+  # Can view everything, approve/deny requests, sync assignments/enrollments.
+  # Site admins are always staff.
+  def staff?(user)
+    user.admin? || user_to_courses.where(user_id: user.id).any?(&:staff?)
   end
 
   # TODO: This doesn't make sense actually.

app/services/request_service.rb

@@ -6,8 +6,8 @@ def self.authenticate_course(course, redirect_path)
     { redirect_to: redirect_path, alert: 'Course not found.' }
   end
 
-  # Check instructor permission
-  def self.check_instructor_permission(role, course_path)
+  # Check staff permission (instructors, lead TAs, TAs, and site admins can approve/deny)
+  def self.check_staff_permission(role, course_path)
     return true if role == 'instructor'
 
     { redirect_to: course_path, alert: 'You do not have permission to perform this action.' }

app/views/courses/enrollments.html.erb

@@ -77,7 +77,7 @@
         </tbody>
       </table>
 
-      <% if @is_course_admin %>
+      <% if @role == 'instructor' %>
         <div class="text-center mt-4">
           <button class="btn btn-info"
                   data-controller="enrollments"

app/views/courses/instructor_show.html.erb

@@ -34,6 +34,7 @@
                            role="switch"
                            id="assignment-<%= assignment.id %>-enabled"
                            <%= 'checked' if assignment.enabled %>
+                           <%= 'disabled' unless @is_course_admin %>
                            data-assignment-target="checkbox"
                            data-assignment-id="<%= assignment.id %>"
                            data-url="<%= toggle_enabled_assignment_path(assignment) %>"

app/views/layouts/_sidebar.html.erb

@@ -34,17 +34,19 @@
 				text: 'Enrollments',
 				active: @side_nav == 'enrollments' %>
 
-			<%= render 'layouts/components/sidebar_menu_item',
-				path: course_settings_path(@course.id),
-				icon: 'fas fa-cog',
-				text: 'Settings',
-				active: @side_nav == 'edit' %>
+			<% if @is_course_admin %>
+				<%= render 'layouts/components/sidebar_menu_item',
+					path: course_settings_path(@course.id),
+					icon: 'fas fa-cog',
+					text: 'Settings',
+					active: @side_nav == 'edit' %>
 
-			<%= render 'layouts/components/sidebar_menu_item',
-				path: edit_course_form_setting_path(@course.id),
-				icon: 'fas fa-file-alt',
-				text: 'Form',
-				active: @side_nav == 'form_settings' %>
+				<%= render 'layouts/components/sidebar_menu_item',
+					path: edit_course_form_setting_path(@course.id),
+					icon: 'fas fa-file-alt',
+					text: 'Form',
+					active: @side_nav == 'form_settings' %>
+			<% end %>
 
 			<%= render 'layouts/components/sidebar_menu_item',
 				path: new_course_request_path(@course.id),

db/schema.rb

@@ -11,8 +11,6 @@
 # It's strongly recommended that you check this file into your version control system.
 
 ActiveRecord::Schema[7.2].define(version: 2026_03_06_000001) do
-  create_schema "hypershield"
-
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
 

spec/controllers/assignments_controller_spec.rb

@@ -7,7 +7,7 @@
   end
 
   describe 'POST #toggle_enabled' do
-    let!(:user) { User.create!(name: 'Test User', email: 'test@example.com') }
+    let!(:user) { User.create!(name: 'Test User', email: 'test@example.com', canvas_uid: '123') }
     let!(:course) { Course.create!(course_name: 'Test Course', canvas_id: '123') }
     let!(:course_to_lms) { CourseToLms.create!(course: course, lms_id: 1, external_course_id: '123') }
     let!(:course_settings) { CourseSettings.create!(course: course, enable_extensions: true) }
@@ -21,13 +21,13 @@
       )
     end
 
-    context 'when the user is an instructor' do
+    context 'when the user is a course admin (teacher)' do
       before do
-        allow(course).to receive(:user_role).with(user).and_return('instructor')
+        UserToCourse.create!(user: user, course: course, role: 'teacher')
       end
 
       it 'updates the enabled status to true' do
-        post :toggle_enabled, params: { id: assignment.id, enabled: true, role: 'instructor', user_id: user.id }
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
 
         expect(response).to have_http_status(:ok)
         expect(assignment.reload.enabled).to be true
@@ -36,20 +36,33 @@
       it 'updates the enabled status to false' do
         assignment.update!(enabled: true)
 
-        post :toggle_enabled, params: { id: assignment.id, enabled: false, role: 'instructor', user_id: user.id }
+        post :toggle_enabled, params: { id: assignment.id, enabled: false }
 
         expect(response).to have_http_status(:ok)
         expect(assignment.reload.enabled).to be false
       end
     end
 
-    context 'when the user is not an instructor' do
+    context 'when the user is a TA (not course admin)' do
       before do
-        allow(course).to receive(:user_role).with(user).and_return('student')
+        UserToCourse.create!(user: user, course: course, role: 'ta')
       end
 
       it 'returns a forbidden status' do
-        post :toggle_enabled, params: { id: assignment.id, enabled: true, role: 'student', user_id: user.id }
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
+
+        expect(response).to have_http_status(:forbidden)
+        expect(assignment.reload.enabled).to be false
+      end
+    end
+
+    context 'when the user is a student' do
+      before do
+        UserToCourse.create!(user: user, course: course, role: 'student')
+      end
+
+      it 'returns a forbidden status' do
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
 
         expect(response).to have_http_status(:forbidden)
         expect(assignment.reload.enabled).to be false
@@ -59,11 +72,11 @@
     context 'when course-level extensions are disabled' do
       before do
         course_settings.update!(enable_extensions: false)
-        allow(course).to receive(:user_role).with(user).and_return('instructor')
+        UserToCourse.create!(user: user, course: course, role: 'teacher')
       end
 
       it 'still allows enabling the assignment and returns ok status' do
-        post :toggle_enabled, params: { id: assignment.id, enabled: true, role: 'instructor', user_id: user.id }
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
 
         expect(response).to have_http_status(:ok)
         expect(assignment.reload.enabled).to be true
@@ -73,10 +86,11 @@
     context 'when there is no due_date on an Assignment' do
       before do
         assignment.update!(due_date: nil)
+        UserToCourse.create!(user: user, course: course, role: 'teacher')
       end
 
       it 'returns a bad request status' do
-        post :toggle_enabled, params: { id: assignment.id, enabled: true, role: 'instructor', user_id: user.id }
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
 
         expect(response).to have_http_status(:unprocessable_content)
         expect(flash[:alert]).to include('Due date must be present if assignment is enabled')
@@ -85,11 +99,11 @@
 
     context 'when user_id is not provided' do
       before do
-        allow(course).to receive(:user_role).and_return('instructor')
+        UserToCourse.create!(user: user, course: course, role: 'teacher')
       end
 
       it 'uses the session user and updates the assignment' do
-        post :toggle_enabled, params: { id: assignment.id, enabled: true, role: 'instructor' }
+        post :toggle_enabled, params: { id: assignment.id, enabled: true }
 
         expect(response).to have_http_status(:ok)
         expect(assignment.reload.enabled).to be true

spec/controllers/course_settings_controller_spec.rb

@@ -17,8 +17,7 @@
   describe 'instructor access' do
     before do
       session[:user_id] = instructor.canvas_uid
-      UserToCourse.create!(user: instructor, course: course, role: 'instructor')
-      allow_any_instance_of(Course).to receive(:user_role).with(instructor).and_return('instructor')
+      UserToCourse.create!(user: instructor, course: course, role: 'teacher')
     end
 
     describe 'POST #update' do
@@ -134,8 +133,7 @@
 
     before do
       session[:user_id] = instructor.canvas_uid
-      UserToCourse.create!(user: instructor, course: course, role: 'instructor')
-      allow_any_instance_of(Course).to receive(:user_role).with(instructor).and_return('instructor')
+      UserToCourse.create!(user: instructor, course: course, role: 'teacher')
 
       # Create settings to enable extensions
       CourseSettings.create!(