diff --git a/.tool-versions b/.tool-versions index 057186bf..053cba7f 100644 --- a/.tool-versions +++ b/.tool-versions @@ -1 +1 @@ -ruby 3.3.8 +ruby 3.3 diff --git a/Gemfile b/Gemfile index d0c7ea6e..9f7eed60 100644 --- a/Gemfile +++ b/Gemfile @@ -60,6 +60,9 @@ gem 'strong_migrations' # Logging Customization gem 'lograge' +# Environment variable management +gem 'dotenv-rails', require: 'dotenv/load' + # Use Active Storage for file uploads [https://guides.rubyonrails.org/active_storage_overview.html] # gem "activestorage", "~> 7.0.0" diff --git a/Gemfile.lock b/Gemfile.lock index 92b2be3c..bb2c8d7d 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -185,6 +185,10 @@ GEM diff-lcs (1.6.2) docile (1.4.1) domain_name (0.6.20240107) + dotenv (3.2.0) + dotenv-rails (3.2.0) + dotenv (= 3.2.0) + railties (>= 6.1) drb (2.2.3) dumb_delegator (1.1.0) erb (6.0.2) @@ -624,6 +628,7 @@ DEPENDENCIES cucumber-rails database_cleaner-active_record debug + dotenv-rails factory_bot_rails faraday faraday-cookie_jar diff --git a/app/controllers/course_settings_controller.rb b/app/controllers/course_settings_controller.rb index 4a2cfe07..10116d88 100644 --- a/app/controllers/course_settings_controller.rb +++ b/app/controllers/course_settings_controller.rb @@ -72,7 +72,9 @@ def course_settings_params :email_subject, :email_template, :enable_slack_webhook_url, - :slack_webhook_url + :slack_webhook_url, + :pending_notification_frequency, + :pending_notification_email ) end diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb index c5b0d03f..e0ae9377 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.' @@ -133,7 +131,6 @@ def group_by_semester(user_to_courses) sorted_semesters.map { |semester| [ semester, grouped[semester] ] } end - # Filters Canvas API course hashes by their term name def filter_by_semester(courses, semester) courses.select { |c| c.dig('term', 'name') == semester } end @@ -146,7 +143,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/controllers/session_controller.rb b/app/controllers/session_controller.rb index 81fd512c..1cd5317c 100644 --- a/app/controllers/session_controller.rb +++ b/app/controllers/session_controller.rb @@ -53,15 +53,25 @@ def omniauth_callback 'email' => auth.info.email } creds = auth.credentials # an OmniAuth::AuthHash + + # dev provider doesnt have real credentials so its stubbed + expires_at = creds.expires_at || 30.days.from_now.to_i + refresh_token = creds.refresh_token || 'none' + access_token = OAuth2::AccessToken.new( OAuth2::Client.new('', ''), # client never used – stub creds.token, - refresh_token: creds.refresh_token, - expires_at: creds.expires_at + refresh_token: refresh_token, + expires_at: expires_at ) # Persist / update the user just like `create` - find_or_create_user(user_data, access_token) + user = find_or_create_user(user_data, access_token) + + # Auto-enroll developer login users in test courses + if auth.provider == 'developer' + ensure_developer_test_enrollments(user) + end redirect_to courses_path, notice: "Logged in! Welcome, #{user_data['name']}!" rescue StandardError => e @@ -79,6 +89,18 @@ def destroy private + def ensure_developer_test_enrollments(user) + # Find the test course + test_course = Course.find_by(course_code: 'DEV101') + + # Ensure enrollment in the test course (as student so they can request extensions) + if test_course + UserToCourse.find_or_create_by!(user_id: user.id, course_id: test_course.id) do |utc| + utc.role = 'student' + end + end + end + # TODO: Refactor. def find_or_create_user(user_data, auth_token) auth_token.token @@ -102,6 +124,8 @@ def find_or_create_user(user_data, auth_token) # Store user ID in session for authentication session[:username] = user.name session[:user_id] = user.canvas_uid + + user end # TODO: Move this to a Canvas API libarary or user service @@ -115,7 +139,7 @@ def update_user_credential(user, token) ) else user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: Lms.CANVAS_LMS.id, token: token.token, refresh_token: token.refresh_token, expire_time: Time.zone.at(token.expires_at) diff --git a/app/controllers/user_to_courses_controller.rb b/app/controllers/user_to_courses_controller.rb index b07d7336..1702c256 100644 --- a/app/controllers/user_to_courses_controller.rb +++ b/app/controllers/user_to_courses_controller.rb @@ -1,25 +1,49 @@ class UserToCoursesController < ApplicationController - before_action :authenticate_user + before_action :authenticate_user! before_action :set_course - before_action :ensure_course_admin + before_action :set_enrollment + before_action :authorize_instructor! def toggle_allow_extended_requests - @enrollment = @course.user_to_courses.find(params[:id]) - if @enrollment.update(allow_extended_requests: params[:allow_extended_requests]) render json: { success: true }, status: :ok else - flash[:alert] = "Failed to update enrollment: #{@enrollment.errors.full_messages.to_sentence}" - render json: { redirect_to: course_path(@course) }, status: :unprocessable_content + render json: { + success: false, + errors: @enrollment.errors.full_messages, + redirect_to: courses_path + }, status: :unprocessable_content end end private - def ensure_course_admin - enrollment = @course.user_to_courses.find_by(user: @user) - return if enrollment&.course_admin? + def authenticate_user! + user_id = session[:user_id] + @current_user = User.find_by(canvas_uid: user_id) if user_id + redirect_to root_path unless @current_user + end + + def set_course + @course = Course.find_by(id: params[:course_id]) + unless @course + flash[:alert] = 'Course not found.' + redirect_to courses_path + end + end + + def set_enrollment + @enrollment = UserToCourse.find(params[:id]) + end - render json: { error: 'You must be an instructor or Lead TA.', redirect_to: course_path(@course) }, status: :forbidden + def authorize_instructor! + user_to_course = UserToCourse.find_by(user: @current_user, course: @course) + unless user_to_course&.course_admin? + render json: { + success: false, + error: 'Forbidden', + redirect_to: courses_path + }, status: :forbidden + end end end 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/course_settings_controller.js b/app/javascript/controllers/course_settings_controller.js index 19a4ff03..7ada1020 100644 --- a/app/javascript/controllers/course_settings_controller.js +++ b/app/javascript/controllers/course_settings_controller.js @@ -1,12 +1,13 @@ import { Controller } from "@hotwired/stimulus" export default class extends Controller { - static targets = ["emailField", "tab", "gradescopeField", "slackWebhookField"]; + static targets = ["emailField", "tab", "gradescopeField", "slackWebhookField", "pendingNotificationEmail"]; connect() { this.toggleEmailFields(); this.toggleSlackWebhookField(); this.toggleGradescopeFields(); + this.togglePendingNotificationEmail(); const gradescopeToggle = document.getElementById('enable-gradescope'); if (gradescopeToggle) { @@ -53,6 +54,15 @@ export default class extends Controller { } } + togglePendingNotificationEmail() { + const frequencySelect = document.getElementById('pending-notification-frequency'); + const emailField = document.getElementById('pending-notification-email'); + + if (frequencySelect && emailField) { + emailField.disabled = !frequencySelect.value; + } + } + updateUrlParam(event) { const tabName = event.currentTarget.dataset.tab; const url = new URL(window.location); 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/jobs/pending_requests_notification_job.rb b/app/jobs/pending_requests_notification_job.rb new file mode 100644 index 00000000..f94faa22 --- /dev/null +++ b/app/jobs/pending_requests_notification_job.rb @@ -0,0 +1,32 @@ +class PendingRequestsNotificationJob < ApplicationJob + queue_as :default + + def perform(frequency) + CourseSettings.with_pending_notifications(frequency).includes(:course).find_each do |cs| + course = cs.course + pending_count = Request.where(course_id: course.id, status: 'pending').count + next if pending_count.zero? + + requests_url = "#{ENV.fetch('APP_HOST', nil)}/courses/#{course.id}/requests" + + EmailService.send_email( + to: cs.pending_notification_email, + from: ENV.fetch('DEFAULT_FROM_EMAIL'), + reply_to: cs.reply_email.presence || ENV.fetch('DEFAULT_FROM_EMAIL'), + subject_template: '{{pending_count}} Pending Extension Request{{plural}} - {{course_code}}', + body_template: "Hello,\n\nYou have {{pending_count}} pending extension request{{plural}} " \ + "in {{course_name}} ({{course_code}}).\n\n" \ + "Please review them at: {{requests_url}}\n\n" \ + "Thank you,\nFlextensions", + mapping: { + 'pending_count' => pending_count.to_s, + 'plural' => pending_count == 1 ? '' : 's', + 'course_name' => course.course_name, + 'course_code' => course.course_code, + 'requests_url' => requests_url + }, + deliver_later: false + ) + end + end +end diff --git a/app/jobs/sync_all_course_assignments_job.rb b/app/jobs/sync_all_course_assignments_job.rb index d39f9aff..cec3ebd1 100644 --- a/app/jobs/sync_all_course_assignments_job.rb +++ b/app/jobs/sync_all_course_assignments_job.rb @@ -47,8 +47,10 @@ def sync_assignment(course_to_lms, lms_assignment, results) # Use shared LmsAssignment to populate Assignment assignment.name = lms_assignment.name - assignment.due_date = lms_assignment.due_date - assignment.late_due_date = lms_assignment.late_due_date + unless preserve_existing_dates?(assignment, lms_assignment) + assignment.due_date = lms_assignment.due_date + assignment.late_due_date = lms_assignment.late_due_date + end assignment.external_assignment_id = lms_assignment.id if assignment.new_record? @@ -60,4 +62,14 @@ def sync_assignment(course_to_lms, lms_assignment, results) end assignment.save! end + + private + + def preserve_existing_dates?(assignment, lms_assignment) + return false if assignment.new_record? + return false unless lms_assignment.is_a?(Lmss::Canvas::Assignment) + return false if lms_assignment.base_date_present? + + assignment.due_date.present? || assignment.late_due_date.present? + end end 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/course_settings.rb b/app/models/course_settings.rb index 10488a4c..0ef7cf6c 100644 --- a/app/models/course_settings.rb +++ b/app/models/course_settings.rb @@ -49,12 +49,25 @@ class CourseSettings < ApplicationRecord {{course_name}} Staff LIQUID + VALID_NOTIFICATION_FREQUENCIES = %w[daily weekly].freeze + belongs_to :course + before_validation -> { self.pending_notification_frequency = nil if pending_notification_frequency.blank? } + before_validation -> { self.pending_notification_email = nil if pending_notification_email.blank? } before_save :ensure_system_user_for_auto_approval + before_save -> { self.pending_notification_email = nil if pending_notification_frequency.nil? } validate :gradescope_url_is_valid, if: :enable_gradescope? + validates :pending_notification_frequency, inclusion: { in: VALID_NOTIFICATION_FREQUENCIES }, allow_nil: true + validates :pending_notification_email, presence: true, format: { with: /\A[^@\s]+@[^@\s]+\z/ }, + if: -> { pending_notification_frequency.present? } after_save :create_or_update_gradescope_link + scope :with_pending_notifications, ->(frequency) { + where(pending_notification_frequency: frequency) + .where.not(pending_notification_email: [ nil, '' ]) + } + def automatic_approval_enabled? return false unless enable_extensions? diff --git a/app/models/lms.rb b/app/models/lms.rb index 98ba63de..8fa66da3 100644 --- a/app/models/lms.rb +++ b/app/models/lms.rb @@ -17,23 +17,28 @@ class Lms < ApplicationRecord # Relationship with Assignment has_many :assignments + # Relationship with LmsCredential + has_many :lms_credentials, dependent: :destroy + # Singleton instances for each LMS def self.CANVAS_LMS - @canvas_lms ||= find_or_create_by( + @canvas_lms ||= find_by(id: 1) || find_or_create_by!( id: 1, - lms_name: 'Canvas', - lms_base_url: ENV.fetch('CANVAS_URL', ''), - use_auth_token: true - ) + lms_name: 'Canvas' + ) do |lms| + lms.lms_base_url = ENV.fetch('CANVAS_URL', '') + lms.use_auth_token = true + end end def self.GRADESCOPE_LMS - @gradescope_lms ||= find_or_create_by( + @gradescope_lms ||= find_by(id: 2) || find_or_create_by!( id: 2, - lms_name: 'Gradescope', - lms_base_url: 'https://www.gradescope.com', - use_auth_token: false - ) + lms_name: 'Gradescope' + ) do |lms| + lms.lms_base_url = 'https://www.gradescope.com' + lms.use_auth_token = false + end end # Map a linked LMS to the appropriate API facade which can be used to post extension requests diff --git a/app/models/lms_credential.rb b/app/models/lms_credential.rb index aa895068..7480a5e5 100644 --- a/app/models/lms_credential.rb +++ b/app/models/lms_credential.rb @@ -5,7 +5,6 @@ # # id :bigint not null, primary key # expire_time :datetime -# lms_name :string # password :string # refresh_token :string # token :string @@ -13,6 +12,7 @@ # created_at :datetime not null # updated_at :datetime not null # external_user_id :string +# lms_id :bigint # user_id :bigint # # Indexes @@ -21,15 +21,18 @@ # # Foreign Keys # +# fk_rails_... (lms_id => lmss.id) # fk_rails_... (user_id => users.id) # class LmsCredential < ApplicationRecord # Belongs to a User belongs_to :user + # Belongs to an LMS + belongs_to :lms + # Encryption for tokens encrypts :token, :refresh_token # LMS must exist - validates :lms_name, presence: true end diff --git a/app/models/user.rb b/app/models/user.rb index 9cac5f45..6695a9a7 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -38,9 +38,8 @@ class User < ApplicationRecord has_many :user_to_courses has_many :courses, through: :user_to_courses - # TODO: We should probably use lms_id over lms_name def canvas_credentials - lms_credentials.find_by(lms_name: 'canvas') + lms_credentials.find_by(lms_id: Lms.CANVAS_LMS.id) end def token_expired? diff --git a/app/models/user_to_course.rb b/app/models/user_to_course.rb index abb63fa2..9a8af215 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,52 @@ 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 teacher? + role == 'teacher' 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/edit.html.erb b/app/views/courses/edit.html.erb index 9214779e..f19b3fde 100644 --- a/app/views/courses/edit.html.erb +++ b/app/views/courses/edit.html.erb @@ -211,6 +211,34 @@ +
+ +
+ <%= select_tag 'course_settings[pending_notification_frequency]', + options_for_select( + [['No notifications', ''], ['Daily', 'daily'], ['Once weekly (Fridays)', 'weekly']], + @course.course_settings&.pending_notification_frequency + ), + class: 'form-select', + id: 'pending-notification-frequency', + data: { action: 'change->course-settings#togglePendingNotificationEmail' } %> +
+
+ +
+ +
+ <%= email_field_tag 'course_settings[pending_notification_email]', + @course.course_settings&.pending_notification_email, + class: 'form-control', + id: 'pending-notification-email', + placeholder: 'instructor@berkeley.edu', + data: { course_settings_target: 'pendingNotificationEmail' }, + disabled: @course.course_settings&.pending_notification_frequency.blank? %> +
Weekly notifications are sent on Fridays at 5:00 PM PT.
+
+
+
<% if @course.course_settings&.enable_extensions %> 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 @@ <%= enrollment.user.student_id %> <%= enrollment.user.email %> - <%= enrollment.role.downcase.capitalize %> + <%= enrollment.display_role %> <% if @is_course_admin %> @@ -89,4 +89,4 @@ <% end %>
- \ No newline at end of file + diff --git a/app/views/courses/index.html.erb b/app/views/courses/index.html.erb index b1323b6e..09a2c713 100644 --- a/app/views/courses/index.html.erb +++ b/app/views/courses/index.html.erb @@ -33,7 +33,7 @@ <%= user_to_course.course.course_name %> <%= user_to_course.course.course_code %> - <%= user_to_course.role.capitalize %> + <%= user_to_course.display_role %> <% end %> <% end %> diff --git a/app/views/courses/new.html.erb b/app/views/courses/new.html.erb index a6585801..dfd1a870 100644 --- a/app/views/courses/new.html.erb +++ b/app/views/courses/new.html.erb @@ -46,7 +46,10 @@ <%= course.dig("term", "name") %> <%= course["course_code"] %> <%= course["name"] %> - <%= course['enrollments'].find { |enrollment| %w[teacher ta].include?(enrollment['type']) }['type'].capitalize %> + + <% staff_enrollment = course['enrollments'].find { |enrollment| UserToCourse.staff_enrollment?(enrollment) } %> + <%= UserToCourse.display_role(UserToCourse.role_from_canvas_enrollment(staff_enrollment)) %> + <% end %> diff --git a/app/views/home/index.html.erb b/app/views/home/index.html.erb index d500226b..f356c5f5 100644 --- a/app/views/home/index.html.erb +++ b/app/views/home/index.html.erb @@ -23,7 +23,9 @@
<%= link_to 'Login', '/auth/canvas', class: 'btn btn-primary', id: 'login-button-index' %> - <%# Login %> + <% if Rails.env.development? %> + <%= link_to 'Developer Login', '/auth/developer', class: 'btn btn-secondary ms-2', id: 'dev-login-button' %> + <% end %>
diff --git a/app/views/requests/instructor_index.html.erb b/app/views/requests/instructor_index.html.erb index 92b8f4b8..a67986a6 100644 --- a/app/views/requests/instructor_index.html.erb +++ b/app/views/requests/instructor_index.html.erb @@ -37,15 +37,15 @@ data-action="change->requests#toggleSelectAll" aria-label="Select all pending requests"> - Actions - Name - Assignment - Student ID - Requested At - Original Due Date - Requested Due Date - # of Days - Status + Actions + Name + Assignment + Student ID + Requested At + Original Due Date + Requested Due Date + # of Days + Status diff --git a/config/initializers/omniauth.rb b/config/initializers/omniauth.rb index 29445eea..bfffa61e 100644 --- a/config/initializers/omniauth.rb +++ b/config/initializers/omniauth.rb @@ -26,7 +26,8 @@ def build_access_token client_options: { site: ENV['CANVAS_URL'], authorize_url: "/login/oauth2/auth?scope=#{encoded_scopes}" - } + }, + redirect_uri: "#{ENV['CANVAS_REDIRECT_URI']}/auth/canvas/callback" end # OmniAuth.config.before_request_phase do |env| diff --git a/db/api_spec_seeds.rb b/db/api_spec_seeds.rb index 3e99bc2b..50748302 100644 --- a/db/api_spec_seeds.rb +++ b/db/api_spec_seeds.rb @@ -50,7 +50,7 @@ test_lms_credential = LmsCredential.create!({ user_id: test_user.id, - lms_name: "canvas", + lms_id: canvas.id, token: "test token", external_user_id: "44444", }) diff --git a/db/migrate/20260304000000_refactor_lms_credentials_to_use_lms_id.rb b/db/migrate/20260304000000_refactor_lms_credentials_to_use_lms_id.rb new file mode 100644 index 00000000..d267a21a --- /dev/null +++ b/db/migrate/20260304000000_refactor_lms_credentials_to_use_lms_id.rb @@ -0,0 +1,26 @@ +class RefactorLmsCredentialsToUseLmsId < ActiveRecord::Migration[7.1] + def change + # Add lms_id column as foreign key without validation + add_column :lms_credentials, :lms_id, :bigint + add_foreign_key :lms_credentials, :lmss, column: :lms_id, validate: false + + # Migrate existing data: populate lms_id based on lms_name + reversible do |dir| + dir.up do + safety_assured do + execute <<-SQL + UPDATE lms_credentials + SET lms_id = lmss.id + FROM lmss + WHERE LOWER(lms_credentials.lms_name) = LOWER(lmss.lms_name) + SQL + end + end + end + + # Remove lms_name column (after data migration) + safety_assured do + remove_column :lms_credentials, :lms_name, :string + end + end +end diff --git a/db/migrate/20260304000001_validate_lms_credentials_lms_id_fk.rb b/db/migrate/20260304000001_validate_lms_credentials_lms_id_fk.rb new file mode 100644 index 00000000..a80bca51 --- /dev/null +++ b/db/migrate/20260304000001_validate_lms_credentials_lms_id_fk.rb @@ -0,0 +1,5 @@ +class ValidateLmsCredentialsLmsIdFk < ActiveRecord::Migration[7.1] + def change + validate_foreign_key :lms_credentials, :lmss + end +end diff --git a/db/migrate/20260406175234_add_pending_notification_to_course_settings.rb b/db/migrate/20260406175234_add_pending_notification_to_course_settings.rb new file mode 100644 index 00000000..fceca5e9 --- /dev/null +++ b/db/migrate/20260406175234_add_pending_notification_to_course_settings.rb @@ -0,0 +1,10 @@ +class AddPendingNotificationToCourseSettings < ActiveRecord::Migration[7.2] + def change + safety_assured do + change_table :course_settings, bulk: true do |t| + t.string :pending_notification_frequency, default: nil + t.string :pending_notification_email, default: nil + end + end + end +end diff --git a/db/schema.rb b/db/schema.rb index 848df19c..dd9a95bb 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,9 +10,10 @@ # # 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 +ActiveRecord::Schema[7.2].define(version: 2026_04_06_175234) do create_schema "hypershield" + # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -105,6 +106,8 @@ t.boolean "enable_gradescope", default: false t.string "gradescope_course_url" t.boolean "extend_late_due_date", default: true, null: false + t.string "pending_notification_frequency" + t.string "pending_notification_email" t.index ["course_id"], name: "index_course_settings_on_course_id" end @@ -163,7 +166,6 @@ create_table "lms_credentials", force: :cascade do |t| t.bigint "user_id" - t.string "lms_name" t.string "username" t.string "password" t.string "token" @@ -172,6 +174,7 @@ t.datetime "updated_at", null: false t.string "external_user_id" t.datetime "expire_time" + t.bigint "lms_id" t.index ["user_id"], name: "index_lms_credentials_on_user_id" end @@ -236,6 +239,7 @@ add_foreign_key "extensions", "assignments" add_foreign_key "extensions", "users", column: "last_processed_by_id" add_foreign_key "form_settings", "courses" + add_foreign_key "lms_credentials", "lmss" add_foreign_key "lms_credentials", "users" add_foreign_key "requests", "assignments" add_foreign_key "requests", "courses" diff --git a/db/seeds.rb b/db/seeds.rb index 3b17f628..6457cbea 100644 --- a/db/seeds.rb +++ b/db/seeds.rb @@ -11,3 +11,65 @@ name: SystemUserService::AUTO_APPROVAL_NAME, canvas_uid: SystemUserService::AUTO_APPROVAL_UID ) + +# Developer test users. you can use teacher or student +if Rails.env.development? + # Teacher user for managing courses + teacher_user = User.find_or_create_by!(email: 'teacher@example.com') do |u| + u.name = 'Test Teacher' + u.admin = false + u.canvas_uid = 'teacher@example.com' + end + + # Student user for requesting extensions + student_user = User.find_or_create_by!(email: 'student@example.com') do |u| + u.name = 'Test Student' + u.admin = false + u.canvas_uid = 'student@example.com' + end + + # Create a single test course + test_course = Course.find_or_create_by!(course_code: 'DEV101') do |c| + c.course_name = 'Development Test Course' + c.canvas_id = 'dev-course-001' + end + + # Link test course to Canvas LMS + CourseToLms.find_or_create_by!(course_id: test_course.id, lms_id: 1) do |ctl| + ctl.external_course_id = 'dev-course-001' + end + + # Enroll teacher as instructor + UserToCourse.find_or_create_by!(user_id: teacher_user.id, course_id: test_course.id) do |utc| + utc.role = 'teacher' + end + + # Enroll student as student + UserToCourse.find_or_create_by!(user_id: student_user.id, course_id: test_course.id) do |utc| + utc.role = 'student' + end + + # Enable extensions for test course + CourseSettings.find_or_create_by!(course_id: test_course.id) do |cs| + cs.enable_extensions = true + end + + # Create form settings for test course + FormSetting.find_or_create_by!(course_id: test_course.id) do |fs| + fs.documentation_disp = 'optional' + fs.custom_q1_disp = 'optional' + fs.custom_q2_disp = 'optional' + end + + # Create sample assignments for test course + course_lms = CourseToLms.find_by(course_id: test_course.id, lms_id: 1) + + if course_lms + Assignment.find_or_create_by!(course_to_lms_id: course_lms.id, external_assignment_id: 'dev-hw-1') do |a| + a.name = 'Homework' + a.due_date = 3.days.from_now + a.late_due_date = 10.days.from_now + a.enabled = true + end + end +end diff --git a/lib/lmss/base_assignment.rb b/lib/lmss/base_assignment.rb index 26c02376..1ecc38da 100644 --- a/lib/lmss/base_assignment.rb +++ b/lib/lmss/base_assignment.rb @@ -4,5 +4,6 @@ def id = raise(NotImplementedError) def name = raise(NotImplementedError) def due_date = raise(NotImplementedError) def late_due_date = raise(NotImplementedError) + def base_date_present? = false end end diff --git a/lib/lmss/canvas/assignment.rb b/lib/lmss/canvas/assignment.rb index 4b273238..07d284a1 100644 --- a/lib/lmss/canvas/assignment.rb +++ b/lib/lmss/canvas/assignment.rb @@ -1,15 +1,20 @@ module Lmss module Canvas class Assignment < BaseAssignment - attr_reader :id, :name, :due_date, :late_due_date + attr_reader :id, :name, :due_date, :late_due_date, :base_date def initialize(data) @id = data['id'] @name = data['name'] + @base_date = data['base_date'] @due_date = extract_date_field(data, 'due_at') @late_due_date = extract_date_field(data, 'lock_at') end + def base_date_present? + @base_date.is_a?(Hash) && @base_date.any? + end + private def extract_date_field(assignment_data, field_name) diff --git a/lib/tasks/notifications.rake b/lib/tasks/notifications.rake new file mode 100644 index 00000000..978c5291 --- /dev/null +++ b/lib/tasks/notifications.rake @@ -0,0 +1,9 @@ +namespace :notifications do + desc 'Send pending request digest emails (usage: rake notifications:send_pending_digests[daily])' + task :send_pending_digests, [ :frequency ] => :environment do |_t, args| + frequency = args[:frequency] + abort 'Usage: rake notifications:send_pending_digests[daily|weekly]' unless %w[daily weekly].include?(frequency) + + PendingRequestsNotificationJob.perform_now(frequency) + end +end diff --git a/spec/controllers/application_controller_spec.rb b/spec/controllers/application_controller_spec.rb index cb795dab..2a44ea6b 100644 --- a/spec/controllers/application_controller_spec.rb +++ b/spec/controllers/application_controller_spec.rb @@ -14,8 +14,9 @@ def test_auth let(:user) do User.create!(email: 'test@example.com', canvas_uid: '123').tap do |u| + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } u.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 1.hour.from_now diff --git a/spec/controllers/concerns/token_refreshable_spec.rb b/spec/controllers/concerns/token_refreshable_spec.rb index 599d8e23..4a7f308a 100644 --- a/spec/controllers/concerns/token_refreshable_spec.rb +++ b/spec/controllers/concerns/token_refreshable_spec.rb @@ -21,8 +21,9 @@ def current_user let(:user) do User.create!(email: 'test@example.com', canvas_uid: '123').tap do |u| + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } u.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 10.minutes.from_now diff --git a/spec/controllers/course_settings_controller_spec.rb b/spec/controllers/course_settings_controller_spec.rb index c6ac03a8..55eeb38f 100644 --- a/spec/controllers/course_settings_controller_spec.rb +++ b/spec/controllers/course_settings_controller_spec.rb @@ -6,8 +6,9 @@ let(:course) { Course.create!(course_name: 'Test Course', canvas_id: '123') } before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -118,6 +119,75 @@ end end + describe 'pending notification params' 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') + CourseSettings.create!(course: course, enable_extensions: true) + end + + it 'persists pending notification settings' do + post :update, params: { + course_id: course.id, + course_settings: { + pending_notification_frequency: 'daily', + pending_notification_email: 'prof@berkeley.edu' + }, + tab: 'general' + } + + settings = CourseSettings.find_by(course_id: course.id) + expect(settings.pending_notification_frequency).to eq('daily') + expect(settings.pending_notification_email).to eq('prof@berkeley.edu') + end + + it 'normalizes blank frequency to nil' do + post :update, params: { + course_id: course.id, + course_settings: { + pending_notification_frequency: '', + pending_notification_email: '' + }, + tab: 'general' + } + + settings = CourseSettings.find_by(course_id: course.id) + expect(settings.pending_notification_frequency).to be_nil + end + + it 'clears stored email when frequency is set to blank' do + settings = CourseSettings.find_by(course_id: course.id) + settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@berkeley.edu') + + post :update, params: { + course_id: course.id, + course_settings: { + pending_notification_frequency: '', + pending_notification_email: '' + }, + tab: 'general' + } + + settings.reload + expect(settings.pending_notification_frequency).to be_nil + expect(settings.pending_notification_email).to be_nil + end + + it 'shows validation errors for invalid email with frequency set' do + post :update, params: { + course_id: course.id, + course_settings: { + pending_notification_frequency: 'daily', + pending_notification_email: 'not-an-email' + }, + tab: 'general' + } + + expect(flash[:alert]).to include('Failed to update course settings:') + end + end + describe 'pending requests count' do let(:assignment) do # Create necessary related objects for Request @@ -190,7 +260,7 @@ before do session[:user_id] = student.canvas_uid student.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'student_token', refresh_token: 'student_refresh_token', expire_time: 1.hour.from_now diff --git a/spec/controllers/courses_controller_spec.rb b/spec/controllers/courses_controller_spec.rb index 383e1b05..7b1afc0e 100644 --- a/spec/controllers/courses_controller_spec.rb +++ b/spec/controllers/courses_controller_spec.rb @@ -8,10 +8,11 @@ let(:course_settings) { CourseSettings.create!(course: course, enable_extensions: true) } before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } session[:user_id] = user.canvas_uid UserToCourse.create!(user: user, course: course, role: 'student') user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -29,6 +30,14 @@ expect(response).to render_template(:index) end + it 'includes Lead TA enrollments in staff courses' do + UserToCourse.create!(user: user, course: student_course, role: 'leadta') + + get :index + + expect(assigns(:teacher_courses).map(&:role)).to include('leadta') + end + context 'semester grouping' do let(:spring_course) { Course.create!(course_name: 'Spring Course', canvas_id: 'sp1', course_code: 'SP101', semester: 'Spring 2026') } let(:fall_course) { Course.create!(course_name: 'Fall Course', canvas_id: 'fa1', course_code: 'FA101', semester: 'Fall 2025') } @@ -112,6 +121,21 @@ expect(response).to redirect_to(courses_path) expect(flash[:notice]).to eq('Selected courses and their assignments have been imported successfully.') end + + it 'imports courses where the user is enrolled with the Canvas Lead TA role' do + lead_ta_course = { + 'id' => '999', + 'name' => 'Lead TA Canvas Course', + 'course_code' => 'LTA101', + 'enrollments' => [ { 'type' => 'ta', 'role' => 'Lead TA' } ] + } + allow(Course).to receive(:fetch_courses).and_return([ lead_ta_course ]) + allow(Course).to receive(:create_or_update_from_canvas) + + post :create, params: { courses: [ '999' ] } + + expect(Course).to have_received(:create_or_update_from_canvas).with(lead_ta_course, 'fake_token', user) + end end describe 'POST #sync_assignments' do @@ -139,6 +163,14 @@ headers: { 'Authorization' => 'Bearer fake_token' } ).to_return(status: 200, body: '[]', headers: {}) end + stub_request(:get, "#{ENV.fetch('CANVAS_URL', nil)}/api/v1/courses/456/users") + .with( + query: { + 'enrollment_role' => 'Lead TA', + 'per_page' => '100' + }, + headers: { 'Authorization' => 'Bearer fake_token' } + ).to_return(status: 200, body: '[]', headers: {}) end context 'when user is a teacher (course admin)' do @@ -215,7 +247,7 @@ 'id' => '103', 'name' => 'Test Course 103', 'course_code' => 'TC103', - 'enrollments' => [ { 'type' => 'teacher' } ], + 'enrollments' => [ { 'type' => 'ta', 'role' => 'Lead TA' } ], 'term' => { 'name' => 'Fall 2025' } }, { @@ -230,7 +262,8 @@ before do # Create a fake LMS credential with a token - user.lms_credentials.create!(lms_name: 'canvas', token: 'fake_token', expire_time: 1.hour.from_now) + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } + user.lms_credentials.create!(lms_id: 1, token: 'fake_token', expire_time: 1.hour.from_now) allow(Course).to receive(:fetch_courses).and_return(canvas_courses) end @@ -246,8 +279,9 @@ expect(assigns(:courses_student)).not_to be_empty # Teacher course should be categorized correctly - teacher_course = assigns(:courses_teacher).first - expect(teacher_course['enrollments'].first['type']).to eq('teacher') + teacher_course_roles = assigns(:courses_teacher).map { |canvas_course| canvas_course['enrollments'].first } + expect(teacher_course_roles).to include(hash_including('type' => 'teacher')) + expect(teacher_course_roles).to include(hash_including('role' => 'Lead TA')) # Student course should be categorized correctly student_course = assigns(:courses_student).first @@ -304,7 +338,8 @@ describe 'GET #enrollments' do before do # Create LMS credentials so user has a token - user.lms_credentials.create!(lms_name: 'canvas', token: 'fake_token', expire_time: 1.hour.from_now) + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } + user.lms_credentials.create!(lms_id: 1, token: 'fake_token', expire_time: 1.hour.from_now) CourseToLms.create!(course: course, lms_id: 1) end diff --git a/spec/controllers/requests_controller_spec.rb b/spec/controllers/requests_controller_spec.rb index 723cac10..b0f7078f 100644 --- a/spec/controllers/requests_controller_spec.rb +++ b/spec/controllers/requests_controller_spec.rb @@ -29,7 +29,7 @@ CourseToLms.create!(course:, lms_id: 1) user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -314,7 +314,7 @@ session[:user_id] = instructor.canvas_uid UserToCourse.create!(user: instructor, course: course, role: 'teacher') instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'instructor_token', refresh_token: 'instructor_refresh', expire_time: 1.hour.from_now @@ -438,10 +438,11 @@ end before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } session[:user_id] = instructor.canvas_uid UserToCourse.create!(user: instructor, course: course, role: 'teacher') instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'instructor_token', refresh_token: 'instructor_refresh', expire_time: 1.hour.from_now @@ -672,7 +673,7 @@ # Create credentials for user user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now diff --git a/spec/controllers/session_controller_spec.rb b/spec/controllers/session_controller_spec.rb index 65b85f6d..6acae4d2 100644 --- a/spec/controllers/session_controller_spec.rb +++ b/spec/controllers/session_controller_spec.rb @@ -111,6 +111,61 @@ end end + describe 'GET #omniauth_callback (developer provider)' do + let(:dev_auth_hash) do + OmniAuth::AuthHash.new( + provider: 'developer', + uid: 'test@example.com', + info: OpenStruct.new(name: 'Test Developer', email: 'test@example.com'), + credentials: { + token: 'dev-token', + refresh_token: nil, + expires_at: nil + } + ) + end + + before do + request.env['omniauth.auth'] = dev_auth_hash + end + + context 'developer provider login with missing credentials' do + it 'handles nil credentials gracefully' do + get :omniauth_callback, params: { provider: 'developer' } + + user = User.find_by(canvas_uid: 'test@example.com') + expect(user).to be_present + expect(user.email).to eq('test@example.com') + + expect(session[:user_id]).to eq('test@example.com') + expect(response).to redirect_to(courses_path) + end + + # test course for dev login + it 'auto-enrolls developer login users in test course' do + test_course = Course.create!(course_code: 'DEV101', course_name: 'Test Course', canvas_id: 'dev-001') + + get :omniauth_callback, params: { provider: 'developer' } + + user = User.find_by(canvas_uid: 'test@example.com') + enrollment = UserToCourse.find_by(user_id: user.id, course_id: test_course.id) + + expect(enrollment).to be_present + expect(enrollment.role).to eq('student') + end + + it 'stores fake refresh token for developer provider' do + get :omniauth_callback, params: { provider: 'developer' } + + user = User.find_by(canvas_uid: 'test@example.com') + creds = user.lms_credentials.first + + expect(creds.refresh_token).to be_present + expect(creds.token).to be_present + end + end + end + describe 'GET #logout' do before do session[:user_id] = 'test_user_id' diff --git a/spec/controllers/user_to_courses_controller_spec.rb b/spec/controllers/user_to_courses_controller_spec.rb index eaaa61e7..90c53c2f 100644 --- a/spec/controllers/user_to_courses_controller_spec.rb +++ b/spec/controllers/user_to_courses_controller_spec.rb @@ -9,11 +9,12 @@ describe 'PATCH #toggle_allow_extended_requests' do context 'when user is an instructor' do before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } UserToCourse.create!(user: instructor, course: course, role: 'teacher') student_enrollment session[:user_id] = instructor.canvas_uid instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -65,10 +66,11 @@ context 'when user is a student' do before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } student_enrollment session[:user_id] = student_user.canvas_uid student_user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -99,10 +101,11 @@ context 'when course does not exist' do before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } student_enrollment session[:user_id] = instructor.canvas_uid instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now @@ -123,10 +126,11 @@ context 'when enrollment does not exist' do before do + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } UserToCourse.create!(user: instructor, course: course, role: 'teacher') session[:user_id] = instructor.canvas_uid instructor.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now diff --git a/spec/facades/canvas_facade_spec.rb b/spec/facades/canvas_facade_spec.rb index 94a966a3..f935f019 100644 --- a/spec/facades/canvas_facade_spec.rb +++ b/spec/facades/canvas_facade_spec.rb @@ -169,6 +169,24 @@ end end + describe '#get_all_course_users' do + let(:course) { instance_double(Course, canvas_id: course_id) } + + it 'uses enrollment_type for built-in Canvas roles' do + stubs.get("courses/#{course_id}/users?per_page=100&enrollment_type[]=ta") { [ 200, {}, '[]' ] } + + expect(facade.get_all_course_users(course, 'ta')).to eq([]) + stubs.verify_stubbed_calls + end + + it 'uses enrollment_role for the custom Lead TA Canvas role' do + stubs.get("courses/#{course_id}/users?per_page=100&enrollment_role=Lead+TA") { [ 200, {}, '[]' ] } + + expect(facade.get_all_course_users(course, 'leadta')).to eq([]) + stubs.verify_stubbed_calls + end + end + describe 'get_course' do before do stubs.get("courses/#{course_id}") { [ 200, {}, '{}' ] } diff --git a/spec/factories/course_settings.rb b/spec/factories/course_settings.rb index 2abddc97..682c96f6 100644 --- a/spec/factories/course_settings.rb +++ b/spec/factories/course_settings.rb @@ -36,5 +36,15 @@ association :course enable_extensions { true } auto_approve_days { 0 } + + trait :with_daily_notifications do + pending_notification_frequency { 'daily' } + pending_notification_email { 'instructor@example.com' } + end + + trait :with_weekly_notifications do + pending_notification_frequency { 'weekly' } + pending_notification_email { 'instructor@example.com' } + end end end diff --git a/spec/factories/lms.rb b/spec/factories/lms.rb index c5c6a006..95bd90e3 100644 --- a/spec/factories/lms.rb +++ b/spec/factories/lms.rb @@ -1,7 +1,18 @@ FactoryBot.define do factory :lms do - id { 1 } # Explicitly set id to 1 (must be hardcoded) + sequence(:id) { |n| n } lms_name { 'Canvas' } use_auth_token { true } + + trait :canvas do + id { 1 } + lms_name { 'Canvas' } + end + + trait :gradescope do + id { 2 } + lms_name { 'Gradescope' } + use_auth_token { false } + end end end diff --git a/spec/factories/lms_credential.rb b/spec/factories/lms_credential.rb index 7dff4782..4e3b097d 100644 --- a/spec/factories/lms_credential.rb +++ b/spec/factories/lms_credential.rb @@ -1,9 +1,16 @@ FactoryBot.define do factory :lms_credential do association :user - lms_name { 'canvas' } + lms_id { 1 } token { 'fake_token' } refresh_token { 'fake_refresh_token' } expire_time { 1.hour.from_now } + + before(:create) do |credential| + # Ensure the LMS with id: 1 exists + unless Lms.exists?(id: 1) + Lms.create!(id: 1, lms_name: 'Canvas', use_auth_token: true) + end + end end end diff --git a/spec/features/accessibility_spec.rb b/spec/features/accessibility_spec.rb index 106827b4..3f614ea2 100644 --- a/spec/features/accessibility_spec.rb +++ b/spec/features/accessibility_spec.rb @@ -165,8 +165,8 @@ def wait_for_page_to_load # Set a default wait time Capybara.default_max_wait_time = 3 - create(:lms_credential, user: teacher1, lms_name: 'canvas') - create(:lms_credential, user: student1, lms_name: 'canvas') + create(:lms_credential, user: teacher1, lms_id: 1) + create(:lms_credential, user: student1, lms_id: 1) stub_request(:get, %r{#{ENV.fetch('CANVAS_URL')}/api/v1/courses/.*}) .to_return(status: 200, body: [].to_json) diff --git a/spec/jobs/pending_requests_notification_job_spec.rb b/spec/jobs/pending_requests_notification_job_spec.rb new file mode 100644 index 00000000..54aa990c --- /dev/null +++ b/spec/jobs/pending_requests_notification_job_spec.rb @@ -0,0 +1,86 @@ +require 'rails_helper' + +RSpec.describe PendingRequestsNotificationJob, type: :job do + let(:course) { create(:course, canvas_id: 'notif_123', course_name: 'CS 101', course_code: 'CS101') } + let(:student) { create(:user, canvas_uid: 'stu_notif_1', email: 'student_notif@example.com', name: 'Student') } + let(:lms) { Lms.first } + let(:course_to_lms) { CourseToLms.create!(course: course, lms: lms, external_course_id: 'ext_123') } + let(:assignment) do + Assignment.create!( + name: 'HW1', + course_to_lms: course_to_lms, + due_date: 3.days.from_now, + external_assignment_id: 'asgn_notif_1', + enabled: true + ) + end + + before do + ActionMailer::Base.delivery_method = :test + ActionMailer::Base.deliveries.clear + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with('DEFAULT_FROM_EMAIL').and_return('flextensions@berkeley.edu') + allow(ENV).to receive(:fetch).with('APP_HOST', nil).and_return('http://localhost:3000') + end + + describe '#perform' do + it 'sends email when course has matching frequency and pending requests' do + course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') + Request.create!(course: course, assignment: assignment, user: student, status: 'pending', + reason: 'Need more time', requested_due_date: 5.days.from_now) + + expect { described_class.perform_now('daily') }.to change { ActionMailer::Base.deliveries.count }.by(1) + + mail = ActionMailer::Base.deliveries.last + expect(mail.to).to eq([ 'prof@example.com' ]) + expect(mail.subject).to include('1 Pending Extension Request') + expect(mail.subject).to include('CS101') + expect(mail.body.encoded).to include("http://localhost:3000/courses/#{course.id}/requests") + end + + it 'skips courses with zero pending requests' do + course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') + + expect { described_class.perform_now('daily') }.not_to(change { ActionMailer::Base.deliveries.count }) + end + + it 'only sends to courses matching the given frequency' do + course.course_settings.update!(pending_notification_frequency: 'weekly', pending_notification_email: 'prof@example.com') + Request.create!(course: course, assignment: assignment, user: student, status: 'pending', + reason: 'Need more time', requested_due_date: 5.days.from_now) + + expect { described_class.perform_now('daily') }.not_to(change { ActionMailer::Base.deliveries.count }) + end + + it 'pluralizes correctly for multiple pending requests' do + course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof@example.com') + 2.times do |i| + Request.create!(course: course, assignment: assignment, + user: create(:user, canvas_uid: "stu_multi_#{i}", email: "stu_multi_#{i}@example.com"), + status: 'pending', reason: 'Need time', requested_due_date: 5.days.from_now) + end + + described_class.perform_now('daily') + + mail = ActionMailer::Base.deliveries.last + expect(mail.subject).to include('2 Pending Extension Requests') + end + + it 'sends separate emails to multiple courses' do + course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof1@example.com') + Request.create!(course: course, assignment: assignment, user: student, status: 'pending', + reason: 'Need time', requested_due_date: 5.days.from_now) + + other_course = create(:course, canvas_id: 'notif_456', course_name: 'CS 201', course_code: 'CS201') + other_ctlms = CourseToLms.create!(course: other_course, lms: lms, external_course_id: 'ext_456') + other_assignment = Assignment.create!(name: 'HW2', course_to_lms: other_ctlms, due_date: 3.days.from_now, + external_assignment_id: 'asgn_notif_2', enabled: true) + other_course.course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'prof2@example.com') + other_student = create(:user, canvas_uid: 'stu_notif_2', email: 'stu_notif_2@example.com') + Request.create!(course: other_course, assignment: other_assignment, user: other_student, status: 'pending', + reason: 'Need time', requested_due_date: 5.days.from_now) + + expect { described_class.perform_now('daily') }.to change { ActionMailer::Base.deliveries.count }.by(2) + end + end +end diff --git a/spec/jobs/sync_all_course_assignments_job_spec.rb b/spec/jobs/sync_all_course_assignments_job_spec.rb index 079b0dd1..976e2c83 100644 --- a/spec/jobs/sync_all_course_assignments_job_spec.rb +++ b/spec/jobs/sync_all_course_assignments_job_spec.rb @@ -114,6 +114,43 @@ end end + context 'when Canvas omits base_date metadata' do + let(:canvas_assignments) do + [ + Lmss::Canvas::Assignment.new( + 'id' => '123', + 'name' => 'Assignment 1', + 'due_at' => '2025-01-30T23:59:00Z', + 'lock_at' => '2025-02-05T23:59:00Z', + 'base_date' => nil + ) + ] + end + + it 'preserves existing due dates for Canvas assignments' do + existing_assignment = create(:assignment, + course_to_lms: course_to_lms, + external_assignment_id: '123', + due_date: DateTime.parse('2025-01-15T23:59:00Z'), + late_due_date: DateTime.parse('2025-01-20T23:59:00Z') + ) + + described_class.perform_now(course_to_lms.id, sync_user.id) + + existing_assignment.reload + expect(existing_assignment.due_date).to eq(DateTime.parse('2025-01-15T23:59:00Z')) + expect(existing_assignment.late_due_date).to eq(DateTime.parse('2025-01-20T23:59:00Z')) + end + + it 'still sets dates for newly imported assignments' do + described_class.perform_now(course_to_lms.id, sync_user.id) + + assignment = Assignment.find_by(external_assignment_id: '123') + expect(assignment.due_date).to eq(DateTime.parse('2025-01-30T23:59:00Z')) + expect(assignment.late_due_date).to eq(DateTime.parse('2025-02-05T23:59:00Z')) + end + end + context 'when sync_user is not staff' do let(:student_user) { course.students.first } @@ -130,20 +167,68 @@ end end + describe '#sync_assignment' do + let(:job) { described_class.new } + let(:results) do + { + added_assignments: 0, + updated_assignments: 0, + unchanged_assignments: 0, + deleted_assignments: 0 + } + end + + it 'creates a new assignment and updates results' do + target_course_to_lms = course_to_lms + + lms_assignment = build_canvas_assignment( + 'id' => 'a123', + 'name' => 'HW1', + 'due_at' => '2025-01-15T23:59:00Z', + 'lock_at' => '2025-01-20T23:59:00Z' + ) - # THIS MUST BE REWRITTEN - # This was moved from Course.sync_assignment - # It is now a helper method within the job. - describe '.sync_assignment' do - it 'creates or updates an assignment' do - pending 'moved from course_spec and should be rewritten' - assignment_data = { 'id' => 'a123', 'name' => 'HW1', 'due_at' => 1.day.from_now.to_s } expect do - described_class.sync_assignment(course_to_lms, assignment_data) - end.to change(Assignment, :count).by(1) + job.send(:sync_assignment, target_course_to_lms, lms_assignment, results) + end.to change { Assignment.where(course_to_lms_id: target_course_to_lms.id).count }.by(1) - assignment = Assignment.last + assignment = Assignment.find_by(course_to_lms_id: target_course_to_lms.id, external_assignment_id: 'a123') expect(assignment.name).to eq('HW1') + expect(assignment.due_date).to eq(DateTime.parse('2025-01-15T23:59:00Z')) + expect(assignment.late_due_date).to eq(DateTime.parse('2025-01-20T23:59:00Z')) + expect(results[:added_assignments]).to eq(1) + expect(results[:updated_assignments]).to eq(0) + expect(results[:unchanged_assignments]).to eq(0) + end + + it 'updates an existing assignment and updates results' do + target_course_to_lms = course_to_lms + + existing_assignment = create(:assignment, + course_to_lms: target_course_to_lms, + external_assignment_id: 'a123', + name: 'Old HW Name', + due_date: DateTime.parse('2025-01-10T23:59:00Z') + ) + + lms_assignment = build_canvas_assignment( + 'id' => 'a123', + 'name' => 'HW1 Updated', + 'due_at' => '2025-01-25T23:59:00Z', + 'lock_at' => nil + ) + + expect do + job.send(:sync_assignment, target_course_to_lms, lms_assignment, results) + end.not_to change { Assignment.where(course_to_lms_id: target_course_to_lms.id).count } + + existing_assignment.reload + expect(existing_assignment.name).to eq('HW1 Updated') + expect(existing_assignment.due_date).to eq(DateTime.parse('2025-01-25T23:59:00Z')) + expect(existing_assignment.late_due_date).to be_nil + expect(results[:added_assignments]).to eq(0) + expect(results[:updated_assignments]).to eq(1) + expect(results[:unchanged_assignments]).to eq(0) end end diff --git a/spec/models/course_settings_spec.rb b/spec/models/course_settings_spec.rb index 31cd24e8..711a894a 100644 --- a/spec/models/course_settings_spec.rb +++ b/spec/models/course_settings_spec.rb @@ -177,6 +177,108 @@ end end + describe 'pending notification validations' do + context 'pending_notification_frequency' do + it 'accepts nil' do + course_settings.pending_notification_frequency = nil + expect(course_settings).to be_valid + end + + it 'accepts "daily"' do + course_settings.pending_notification_frequency = 'daily' + course_settings.pending_notification_email = 'test@example.com' + expect(course_settings).to be_valid + end + + it 'accepts "weekly"' do + course_settings.pending_notification_frequency = 'weekly' + course_settings.pending_notification_email = 'test@example.com' + expect(course_settings).to be_valid + end + + it 'rejects "monthly"' do + course_settings.pending_notification_frequency = 'monthly' + course_settings.pending_notification_email = 'test@example.com' + expect(course_settings).not_to be_valid + expect(course_settings.errors[:pending_notification_frequency]).to be_present + end + end + + context 'pending_notification_email' do + it 'is required when frequency is set' do + course_settings.pending_notification_frequency = 'daily' + course_settings.pending_notification_email = nil + expect(course_settings).not_to be_valid + expect(course_settings.errors[:pending_notification_email]).to be_present + end + + it 'validates email format when frequency is set' do + course_settings.pending_notification_frequency = 'daily' + course_settings.pending_notification_email = 'not-an-email' + expect(course_settings).not_to be_valid + expect(course_settings.errors[:pending_notification_email]).to be_present + end + + it 'accepts a valid email when frequency is set' do + course_settings.pending_notification_frequency = 'daily' + course_settings.pending_notification_email = 'instructor@berkeley.edu' + expect(course_settings).to be_valid + end + + it 'is not required when frequency is nil' do + course_settings.pending_notification_frequency = nil + course_settings.pending_notification_email = nil + expect(course_settings).to be_valid + end + end + + context 'normalization' do + it 'normalizes empty string frequency to nil' do + course_settings.pending_notification_frequency = '' + course_settings.valid? + expect(course_settings.pending_notification_frequency).to be_nil + end + + it 'normalizes empty string email to nil' do + course_settings.pending_notification_email = '' + course_settings.valid? + expect(course_settings.pending_notification_email).to be_nil + end + + it 'clears email when frequency is set to nil on save' do + course_settings.pending_notification_frequency = 'daily' + course_settings.pending_notification_email = 'test@example.com' + course_settings.save! + + course_settings.pending_notification_frequency = nil + course_settings.save! + course_settings.reload + + expect(course_settings.pending_notification_email).to be_nil + end + end + end + + describe '.with_pending_notifications' do + it 'returns records matching the given frequency with an email set' do + course_settings.update!(pending_notification_frequency: 'daily', pending_notification_email: 'a@example.com') + + other_course = create(:course, canvas_id: 'other_123', course_name: 'Other', course_code: 'OTHER101') + other_course.course_settings.update!(pending_notification_frequency: 'weekly', pending_notification_email: 'b@example.com') + + results = described_class.with_pending_notifications('daily') + expect(results).to include(course_settings) + expect(results).not_to include(other_course.course_settings) + end + + it 'excludes records with nil email' do + course_settings.update_columns(pending_notification_frequency: 'daily', pending_notification_email: nil) # rubocop:disable Rails/SkipsModelValidations + + results = described_class.with_pending_notifications('daily') + expect(results).not_to include(course_settings) + end + end + describe '#extract_gradescope_course_id' do it 'extracts course ID from valid URL' do url = 'https://www.gradescope.com/courses/123456' diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb index d85efb78..a907dc36 100644 --- a/spec/models/course_spec.rb +++ b/spec/models/course_spec.rb @@ -41,8 +41,9 @@ it 'returns the correct user for auto approval' do course = described_class.create!(canvas_id: 'canvas_123', course_name: 'Test', course_code: 'TEST101') user = User.create!(email: 'test@example.com', canvas_uid: '123') + Lms.find_or_create_by(id: 1) { |l| l.lms_name = 'Canvas'; l.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 1.hour.from_now @@ -54,6 +55,16 @@ end end + describe '#user_role' do + it 'treats leadta enrollments as instructors' do + course = described_class.create!(canvas_id: 'canvas_leadta', course_name: 'Test', course_code: 'TEST101') + user = User.create!(email: 'leadta@example.com', canvas_uid: 'leadta_123') + UserToCourse.create!(user: user, course: course, role: 'leadta') + + expect(course.user_role(user)).to eq('instructor') + end + end + describe '.fetch_courses' do it 'returns parsed JSON if response is successful' do stub_request(:get, %r{api/v1/courses}) @@ -235,6 +246,16 @@ end end + describe '#sync_all_enrollments_from_canvas' do + let!(:course) { described_class.create!(canvas_id: 'canvas_all_roles', course_name: 'User Sync', course_code: 'USYNC') } + + it 'syncs every supported internal role, including leadta' do + expect(SyncUsersFromCanvasJob).to receive(:perform_now).with(course.id, 999, %w[student teacher ta leadta]) + + course.sync_all_enrollments_from_canvas(999) + end + end + describe '.create_or_update_from_canvas' do let(:user) { create(:user, id: 999, canvas_uid: 'u1', name: 'User 1', email: 'user1@example.com') } diff --git a/spec/models/lms_credential_spec.rb b/spec/models/lms_credential_spec.rb index 3a845caf..121115dc 100644 --- a/spec/models/lms_credential_spec.rb +++ b/spec/models/lms_credential_spec.rb @@ -5,7 +5,6 @@ # # id :bigint not null, primary key # expire_time :datetime -# lms_name :string # password :string # refresh_token :string # token :string @@ -13,6 +12,7 @@ # created_at :datetime not null # updated_at :datetime not null # external_user_id :string +# lms_id :bigint # user_id :bigint # # Indexes @@ -21,6 +21,7 @@ # # Foreign Keys # +# fk_rails_... (lms_id => lmss.id) # fk_rails_... (user_id => users.id) # require 'rails_helper' @@ -40,10 +41,11 @@ def self.mock_get_service(token, refresh_token) RSpec.describe LmsCredential, type: :model do describe 'Token Encryption' do let(:user) { User.create!(email: 'test@example.com') } + let!(:lms) { Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } } let!(:credential) do described_class.create!( user: user, - lms_name: 'ExampleLMS', + lms_id: lms.id, username: 'testuser', password: 'testpassword', token: 'sensitive_token', diff --git a/spec/models/request_spec.rb b/spec/models/request_spec.rb index 24bcb0eb..a5f52c10 100644 --- a/spec/models/request_spec.rb +++ b/spec/models/request_spec.rb @@ -70,8 +70,9 @@ before do UserToCourse.create!(user: user, course: course, role: 'student') + create(:lms) user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'fake_token', refresh_token: 'fake_refresh_token', expire_time: 1.hour.from_now diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index a929e1d7..feabdbc3 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -31,8 +31,9 @@ context 'when the token is still valid' do before do + Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 1.hour.from_now @@ -46,8 +47,9 @@ context 'when the token is expired' do before do + Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'expired_token', refresh_token: 'refresh_token', expire_time: 1.hour.ago @@ -64,8 +66,9 @@ let(:user) { described_class.create!(email: 'test@example.com', canvas_uid: '123') } it 'returns the correct credentials for a user' do + Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 1.hour.from_now @@ -81,8 +84,9 @@ context 'when token does not expire soon' do before do + Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'valid_token', refresh_token: 'refresh_token', expire_time: 1.hour.from_now @@ -96,8 +100,9 @@ context 'when token expires soon and is refreshed' do let(:credential) do + Lms.find_or_create_by(id: 1) { |lms| lms.lms_name = 'Canvas'; lms.use_auth_token = true } user.lms_credentials.create!( - lms_name: 'canvas', + lms_id: 1, token: 'stale_token', refresh_token: 'refresh_token', expire_time: 5.minutes.from_now diff --git a/spec/models/user_to_course_spec.rb b/spec/models/user_to_course_spec.rb new file mode 100644 index 00000000..29aef034 --- /dev/null +++ b/spec/models/user_to_course_spec.rb @@ -0,0 +1,41 @@ +require 'rails_helper' + +RSpec.describe UserToCourse, type: :model do + describe 'Lead TA role support' do + it 'treats leadta as a supported staff role' do + user_to_course = build(:user_to_course, role: 'leadta') + + expect(described_class.staff_roles).to include('leadta') + expect(described_class.roles).to include('leadta') + expect(user_to_course).to be_staff + end + + it 'treats leadta as a course admin role' do + user_to_course = build(:user_to_course, role: 'leadta') + + expect(user_to_course).to be_course_admin + end + end + + describe '.role_from_canvas_enrollment' do + it 'normalizes Canvas Lead TA custom role enrollments' do + enrollment = { 'type' => 'ta', 'role' => 'Lead TA' } + + expect(described_class.role_from_canvas_enrollment(enrollment)).to eq('leadta') + end + + it 'falls back to the Canvas enrollment type for built-in roles' do + enrollment = { 'type' => 'teacher' } + + expect(described_class.role_from_canvas_enrollment(enrollment)).to eq('teacher') + end + end + + describe '#display_role' do + it 'formats leadta as Lead TA' do + user_to_course = build(:user_to_course, role: 'leadta') + + expect(user_to_course.display_role).to eq('Lead TA') + end + end +end diff --git a/spec/tasks/notifications_rake_spec.rb b/spec/tasks/notifications_rake_spec.rb new file mode 100644 index 00000000..638d1c4e --- /dev/null +++ b/spec/tasks/notifications_rake_spec.rb @@ -0,0 +1,21 @@ +require 'rails_helper' +require 'rake' + +RSpec.describe 'notifications:send_pending_digests' do # rubocop:disable RSpec/DescribeClass + before(:all) do + Rails.application.load_tasks + end + + it 'invokes PendingRequestsNotificationJob with valid frequency' do + expect(PendingRequestsNotificationJob).to receive(:perform_now).with('daily') + Rake::Task['notifications:send_pending_digests'].reenable + Rake::Task['notifications:send_pending_digests'].invoke('daily') + end + + it 'aborts with usage message for invalid frequency' do + Rake::Task['notifications:send_pending_digests'].reenable + expect { + Rake::Task['notifications:send_pending_digests'].invoke('monthly') + }.to raise_error(SystemExit) + end +end