diff --git a/Gemfile b/Gemfile
index d0c7ea6e..1657433d 100644
--- a/Gemfile
+++ b/Gemfile
@@ -32,6 +32,9 @@ gem 'tzinfo-data', platforms: %i[windows jruby]
# Reduces boot times through caching; required in config/boot.rb
gem 'bootsnap', require: false
+# Loads environment variables from .env (local dev/test only, not Heroku)
+gem 'dotenv-rails', groups: [ :development, :test ]
+
# Alternative Canvas API. We probably don't need this.
# Verify instances of `LMS::Canvas`
gem 'lms-api'
@@ -70,6 +73,7 @@ gem 'lograge'
#
gem 'blazer'
gem 'hypershield'
+gem 'good_job', '~> 4.0'
#### Frontend related tools
# The original asset pipeline for Rails [https://github.com/rails/sprockets-rails]
diff --git a/Gemfile.lock b/Gemfile.lock
index 92b2be3c..d6200315 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -185,10 +185,16 @@ 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)
erubi (1.13.1)
+ et-orbi (1.4.0)
+ tzinfo
factory_bot (6.5.6)
activesupport (>= 6.1.0)
factory_bot_rails (6.5.1)
@@ -215,8 +221,18 @@ GEM
sassc (~> 2.0)
formatador (1.2.3)
reline
+ fugit (1.12.1)
+ et-orbi (~> 1.4)
+ raabro (~> 1.4)
globalid (1.3.0)
activesupport (>= 6.1)
+ good_job (4.14.2)
+ activejob (>= 6.1.0)
+ activerecord (>= 6.1.0)
+ concurrent-ruby (>= 1.3.1)
+ fugit (>= 1.11.0)
+ railties (>= 6.1.0)
+ thor (>= 1.0.0)
guard (2.20.1)
formatador (>= 0.2.4)
listen (>= 2.7, < 4.0)
@@ -382,6 +398,7 @@ GEM
public_suffix (7.0.5)
puma (7.2.0)
nio4r (~> 2.0)
+ raabro (1.4.0)
racc (1.8.1)
rack (3.2.6)
rack-protection (4.2.1)
@@ -624,10 +641,12 @@ DEPENDENCIES
cucumber-rails
database_cleaner-active_record
debug
+ dotenv-rails
factory_bot_rails
faraday
faraday-cookie_jar
font-awesome-sass
+ good_job (~> 4.0)
guard-rspec
hypershield
importmap-rails
diff --git a/Procfile b/Procfile
new file mode 100644
index 00000000..581b1f10
--- /dev/null
+++ b/Procfile
@@ -0,0 +1,2 @@
+web: bundle exec rails server -p $PORT
+worker: bundle exec good_job start
diff --git a/app/controllers/courses_controller.rb b/app/controllers/courses_controller.rb
index c5b0d03f..411b442c 100644
--- a/app/controllers/courses_controller.rb
+++ b/app/controllers/courses_controller.rb
@@ -1,6 +1,6 @@
class CoursesController < ApplicationController
before_action :authenticate_user
- before_action :set_course, only: %i[show edit sync_assignments sync_enrollments enrollments delete]
+ before_action :set_course, only: %i[show edit sync_assignments sync_enrollments sync_status enrollments delete]
before_action :set_pending_request_count
before_action :determine_user_role
@@ -89,6 +89,16 @@ def sync_enrollments
render json: { message: 'Users synced successfully.' }, status: :ok
end
+ def sync_status
+ course_to_lms = @course.course_to_lms(1)
+ return render json: { error: 'LMS connection not found.' }, status: :not_found unless course_to_lms
+
+ render json: {
+ roster_synced_at: course_to_lms.recent_roster_sync&.dig('synced_at'),
+ assignments_synced_at: course_to_lms.recent_assignment_sync&.dig('synced_at')
+ }, 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'
diff --git a/app/javascript/controllers/assignment_controller.js b/app/javascript/controllers/assignment_controller.js
index 8112cfb5..8d7cbae1 100644
--- a/app/javascript/controllers/assignment_controller.js
+++ b/app/javascript/controllers/assignment_controller.js
@@ -5,7 +5,7 @@ import "datatables.net-responsive-bs5";
// Connects to data-controller="assignment"
export default class extends Controller {
- static targets = ["checkbox"]
+ static targets = ["checkbox", "syncBtn", "syncLabel", "syncSpinner"]
static values = { courseId: Number }
connect() {
@@ -64,31 +64,47 @@ export default class extends Controller {
}
}
- sync(event) {
- const button = event.currentTarget;
- button.disabled = true;
+ async sync() {
+ const button = this.syncBtnTarget;
+ const label = this.syncLabelTarget;
+ const spinner = this.syncSpinnerTarget;
const courseId = this.courseIdValue;
const token = document.querySelector('meta[name="csrf-token"]').content;
- fetch(`/courses/${courseId}/sync_assignments`, {
- method: "POST",
- headers: {
- "Content-Type": "application/json",
- "X-CSRF-Token": token,
- },
- })
- .then((response) => {
- if (!response.ok) {
- throw new Error("Failed to sync assignments.");
- }
- return response.json();
- })
- .then((data) => {
- flash("notice", data.message || "Assignments synced successfully.");
- location.reload();
- })
- .catch((error) => {
- flash("alert", error.message || "An error occurred while syncing assignments.");
- location.reload();
+
+ button.disabled = true;
+ label.textContent = "Syncing...";
+ spinner.classList.remove("d-none");
+
+ try {
+ const statusBefore = await fetch(`/courses/${courseId}/sync_status`).then(r => r.json());
+ const beforeTs = statusBefore.assignments_synced_at;
+
+ const response = await fetch(`/courses/${courseId}/sync_assignments`, {
+ method: "POST",
+ headers: { "Content-Type": "application/json", "X-CSRF-Token": token },
});
+
+ if (!response.ok) throw new Error(`Failed to sync assignments. ${response.status}`);
+
+ await this._pollUntilDone(courseId, "assignments_synced_at", beforeTs);
+
+ flash("notice", "Assignments synced successfully.");
+ location.reload();
+ } catch (error) {
+ flash("alert", error.message || "An error occurred while syncing assignments.");
+ button.disabled = false;
+ label.textContent = "Sync Assignments";
+ spinner.classList.add("d-none");
+ }
+ }
+
+ async _pollUntilDone(courseId, key, beforeTs, intervalMs = 1000, timeoutMs = 60000) {
+ const deadline = Date.now() + timeoutMs;
+ while (Date.now() < deadline) {
+ await new Promise(resolve => setTimeout(resolve, intervalMs));
+ const status = await fetch(`/courses/${courseId}/sync_status`).then(r => r.json());
+ if (status[key] && status[key] !== beforeTs) return;
+ }
+ throw new Error("Sync timed out. Please refresh the page.");
}
}
diff --git a/app/javascript/controllers/enrollments_controller.js b/app/javascript/controllers/enrollments_controller.js
index cd03aecb..b506cae4 100644
--- a/app/javascript/controllers/enrollments_controller.js
+++ b/app/javascript/controllers/enrollments_controller.js
@@ -4,7 +4,7 @@ import "datatables.net-responsive";
import "datatables.net-responsive-bs5";
export default class extends Controller {
- static targets = ["checkbox"]
+ static targets = ["checkbox", "syncBtn", "syncLabel", "syncSpinner"]
static values = { courseId: Number }
connect() {
@@ -76,30 +76,49 @@ export default class extends Controller {
window.dispatchEvent(new CustomEvent('flash', { detail: { type: type, message: message } }));
}
- sync() {
- const button = event.currentTarget;
- button.disabled = true;
+ async sync() {
+ const button = this.syncBtnTarget;
+ const label = this.syncLabelTarget;
+ const spinner = this.syncSpinnerTarget;
const courseId = this.courseIdValue;
- const token = document.querySelector('meta[name="csrf-token"]').content; fetch(`/courses/${courseId}/sync_enrollments`, {
- method: "POST",
- headers: {
- "Content-Type": "application/json",
- "X-CSRF-Token": token,
- },
- })
- .then((response) => {
- if (!response.ok) {
- throw new Error(`Failed to sync enrollments. ${response.status} - ${response.statusText}`);
- }
- return response.json();
- })
- .then((data) => {
- flash("notice", data.message || "Enrollments synced successfully.");
+ const token = document.querySelector('meta[name="csrf-token"]')?.content || '';
+
+ button.disabled = true;
+ label.textContent = "Syncing...";
+ spinner.classList.remove("d-none");
+
+ try {
+ // Capture timestamp before sync so we can detect when job finishes
+ const statusBefore = await fetch(`/courses/${courseId}/sync_status`).then(r => r.json());
+ const beforeTs = statusBefore.roster_synced_at;
+
+ const response = await fetch(`/courses/${courseId}/sync_enrollments`, {
+ method: "POST",
+ headers: { "Content-Type": "application/json", "X-CSRF-Token": token },
+ });
+
+ if (!response.ok) throw new Error(`Failed to sync enrollments. ${response.status}`);
+
+ // Poll until synced_at changes
+ await this._pollUntilDone(courseId, "roster_synced_at", beforeTs);
+
+ flash("notice", "Enrollments synced successfully.");
location.reload();
- })
- .catch((error) => {
+ } catch (error) {
flash("alert", error.message || "An error occurred while syncing enrollments.");
- location.reload();
- });
- }
+ button.disabled = false;
+ label.textContent = "Sync Enrollments";
+ spinner.classList.add("d-none");
+ }
+ }
+
+ async _pollUntilDone(courseId, key, beforeTs, intervalMs = 1000, timeoutMs = 60000) {
+ const deadline = Date.now() + timeoutMs;
+ while (Date.now() < deadline) {
+ await new Promise(resolve => setTimeout(resolve, intervalMs));
+ const status = await fetch(`/courses/${courseId}/sync_status`).then(r => r.json());
+ if (status[key] && status[key] !== beforeTs) return;
+ }
+ throw new Error("Sync timed out. Please refresh the page.");
+ }
}
diff --git a/app/models/course.rb b/app/models/course.rb
index 9f4ac9b7..6e4aca1d 100644
--- a/app/models/course.rb
+++ b/app/models/course.rb
@@ -246,14 +246,14 @@ def sync_assignments(sync_user)
return unless lms_links.any?
lms_links.each do |course_to_lms|
- SyncAllCourseAssignmentsJob.perform_now(course_to_lms.id, sync_user.id)
+ SyncAllCourseAssignmentsJob.perform_later(course_to_lms.id, sync_user.id)
end
end
# Fetch users for a course and create/find their User and UserToCourse records
# TODO: This may need to become a background job
def sync_users_from_canvas(user, roles = [ 'student' ])
- SyncUsersFromCanvasJob.perform_now(id, user, roles)
+ SyncUsersFromCanvasJob.perform_later(id, user, roles)
end
def sync_all_enrollments_from_canvas(user)
diff --git a/app/views/courses/enrollments.html.erb b/app/views/courses/enrollments.html.erb
index 80750df6..70e22b2d 100644
--- a/app/views/courses/enrollments.html.erb
+++ b/app/views/courses/enrollments.html.erb
@@ -82,8 +82,10 @@
<% end %>
diff --git a/app/views/courses/instructor_show.html.erb b/app/views/courses/instructor_show.html.erb
index 3f0ed8ef..54d64376 100644
--- a/app/views/courses/instructor_show.html.erb
+++ b/app/views/courses/instructor_show.html.erb
@@ -53,8 +53,10 @@
diff --git a/config/application.rb b/config/application.rb
index e298613f..9c4003f8 100644
--- a/config/application.rb
+++ b/config/application.rb
@@ -40,6 +40,7 @@ class Application < Rails::Application
config.active_record.default_timezone = :utc
config.time_zone = 'Pacific Time (US & Canada)'
config.generators.system_tests = nil
+ config.active_job.queue_adapter = :good_job
# We do not require the master key and insetad use environment variables
# Review .env.example for required variables.
diff --git a/config/environments/test.rb b/config/environments/test.rb
index c9f0436f..b43794c5 100644
--- a/config/environments/test.rb
+++ b/config/environments/test.rb
@@ -66,6 +66,11 @@
# Raise error when a before_action's only/except options reference missing actions
config.action_controller.raise_on_missing_callback_actions = true
+ # Disable GoodJob's in-process worker so enqueued jobs do not execute during tests.
+ # Without this, GoodJob runs jobs in background threads, causing sync operations to
+ # complete before Capybara can observe transient UI states (spinner, disabled button).
+ config.good_job.execution_mode = :external
+
# Set up default encryption keys for the test environment
config.active_record.encryption.primary_key = 'test-primary-key-1234567890abcdef'
config.active_record.encryption.deterministic_key = 'test-deterministic-key-1234567890abcdef'
diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb
new file mode 100644
index 00000000..3c827270
--- /dev/null
+++ b/config/initializers/good_job.rb
@@ -0,0 +1,17 @@
+Rails.application.config.to_prepare do
+ GoodJob::ApplicationController.class_eval do
+ def current_user
+ @current_user ||= User.find_by(canvas_uid: session[:user_id])
+ end
+
+ before_action :require_admin
+
+ def require_admin
+ if current_user.nil?
+ redirect_to '/', alert: 'You must be logged in.'
+ elsif !current_user.admin?
+ redirect_to '/', alert: 'You are not authorized to view this page.'
+ end
+ end
+ end
+end
diff --git a/config/initializers/lograge.rb b/config/initializers/lograge.rb
index 25af3a01..1a3bc987 100644
--- a/config/initializers/lograge.rb
+++ b/config/initializers/lograge.rb
@@ -25,7 +25,7 @@
config.lograge.custom_payload do |controller|
{
request_id: controller.request.uuid,
- user_id: controller.current_user.try(:id)
+ user_id: controller.is_a?(ApplicationController) ? controller.current_user.try(:id) : nil
}
end
diff --git a/config/routes.rb b/config/routes.rb
index 2a0448d9..9886eed9 100644
--- a/config/routes.rb
+++ b/config/routes.rb
@@ -30,6 +30,7 @@
member do
post :sync_assignments
post :sync_enrollments
+ get :sync_status
get :enrollments
delete :delete
end
@@ -68,4 +69,5 @@
# This is protected by `require_admin` via blazer.yml
mount Blazer::Engine, at: "admin/blazer"
+ mount GoodJob::Engine, at: "admin/good_job"
end
diff --git a/db/migrate/20260407004259_create_good_jobs.rb b/db/migrate/20260407004259_create_good_jobs.rb
new file mode 100644
index 00000000..e2e88bfb
--- /dev/null
+++ b/db/migrate/20260407004259_create_good_jobs.rb
@@ -0,0 +1,105 @@
+# frozen_string_literal: true
+
+class CreateGoodJobs < ActiveRecord::Migration[7.2]
+ def change
+ # Uncomment for Postgres v12 or earlier to enable gen_random_uuid() support
+ # enable_extension 'pgcrypto'
+
+ create_table :good_jobs, id: :uuid do |t|
+ t.text :queue_name
+ t.integer :priority
+ t.jsonb :serialized_params
+ t.datetime :scheduled_at
+ t.datetime :performed_at
+ t.datetime :finished_at
+ t.text :error
+
+ t.timestamps
+
+ t.uuid :active_job_id
+ t.text :concurrency_key
+ t.text :cron_key
+ t.uuid :retried_good_job_id
+ t.datetime :cron_at
+
+ t.uuid :batch_id
+ t.uuid :batch_callback_id
+
+ t.boolean :is_discrete
+ t.integer :executions_count
+ t.text :job_class
+ t.integer :error_event, limit: 2
+ t.text :labels, array: true
+ t.uuid :locked_by_id
+ t.datetime :locked_at
+ end
+
+ create_table :good_job_batches, id: :uuid do |t|
+ t.timestamps
+ t.text :description
+ t.jsonb :serialized_properties
+ t.text :on_finish
+ t.text :on_success
+ t.text :on_discard
+ t.text :callback_queue_name
+ t.integer :callback_priority
+ t.datetime :enqueued_at
+ t.datetime :discarded_at
+ t.datetime :finished_at
+ t.datetime :jobs_finished_at
+ end
+
+ create_table :good_job_executions, id: :uuid do |t|
+ t.timestamps
+
+ t.uuid :active_job_id, null: false
+ t.text :job_class
+ t.text :queue_name
+ t.jsonb :serialized_params
+ t.datetime :scheduled_at
+ t.datetime :finished_at
+ t.text :error
+ t.integer :error_event, limit: 2
+ t.text :error_backtrace, array: true
+ t.uuid :process_id
+ t.interval :duration
+ end
+
+ create_table :good_job_processes, id: :uuid do |t|
+ t.timestamps
+ t.jsonb :state
+ t.integer :lock_type, limit: 2
+ end
+
+ create_table :good_job_settings, id: :uuid do |t|
+ t.timestamps
+ t.text :key
+ t.jsonb :value
+ t.index :key, unique: true
+ end
+
+ add_index :good_jobs, :scheduled_at, where: "(finished_at IS NULL)", name: :index_good_jobs_on_scheduled_at
+ add_index :good_jobs, [ :queue_name, :scheduled_at ], where: "(finished_at IS NULL)", name: :index_good_jobs_on_queue_name_and_scheduled_at
+ add_index :good_jobs, [ :active_job_id, :created_at ], name: :index_good_jobs_on_active_job_id_and_created_at
+ add_index :good_jobs, :concurrency_key, where: "(finished_at IS NULL)", name: :index_good_jobs_on_concurrency_key_when_unfinished
+ add_index :good_jobs, [ :concurrency_key, :created_at ], name: :index_good_jobs_on_concurrency_key_and_created_at
+ add_index :good_jobs, [ :cron_key, :created_at ], where: "(cron_key IS NOT NULL)", name: :index_good_jobs_on_cron_key_and_created_at_cond
+ add_index :good_jobs, [ :cron_key, :cron_at ], where: "(cron_key IS NOT NULL)", unique: true, name: :index_good_jobs_on_cron_key_and_cron_at_cond
+ add_index :good_jobs, [ :finished_at ], where: "finished_at IS NOT NULL", name: :index_good_jobs_jobs_on_finished_at_only
+ add_index :good_jobs, [ :priority, :created_at ], order: { priority: "DESC NULLS LAST", created_at: :asc },
+ where: "finished_at IS NULL", name: :index_good_jobs_jobs_on_priority_created_at_when_unfinished
+ add_index :good_jobs, [ :priority, :created_at ], order: { priority: "ASC NULLS LAST", created_at: :asc },
+ where: "finished_at IS NULL", name: :index_good_job_jobs_for_candidate_lookup
+ add_index :good_jobs, [ :batch_id ], where: "batch_id IS NOT NULL"
+ add_index :good_jobs, [ :batch_callback_id ], where: "batch_callback_id IS NOT NULL"
+ add_index :good_jobs, :job_class, name: :index_good_jobs_on_job_class
+ add_index :good_jobs, :labels, using: :gin, where: "(labels IS NOT NULL)", name: :index_good_jobs_on_labels
+
+ add_index :good_job_executions, [ :active_job_id, :created_at ], name: :index_good_job_executions_on_active_job_id_and_created_at
+ add_index :good_jobs, [ :priority, :scheduled_at ], order: { priority: "ASC NULLS LAST", scheduled_at: :asc },
+ where: "finished_at IS NULL AND locked_by_id IS NULL", name: :index_good_jobs_on_priority_scheduled_at_unfinished_unlocked
+ add_index :good_jobs, :locked_by_id,
+ where: "locked_by_id IS NOT NULL", name: "index_good_jobs_on_locked_by_id"
+ add_index :good_job_executions, [ :process_id, :created_at ], name: :index_good_job_executions_on_process_id_and_created_at
+ end
+end
diff --git a/db/schema.rb b/db/schema.rb
index 848df19c..9336c27c 100644
--- a/db/schema.rb
+++ b/db/schema.rb
@@ -10,7 +10,7 @@
#
# 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_07_004259) do
create_schema "hypershield"
# These are extensions that must be enabled in order to support this database
@@ -161,6 +161,97 @@
t.index ["course_id"], name: "index_form_settings_on_course_id"
end
+ create_table "good_job_batches", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.text "description"
+ t.jsonb "serialized_properties"
+ t.text "on_finish"
+ t.text "on_success"
+ t.text "on_discard"
+ t.text "callback_queue_name"
+ t.integer "callback_priority"
+ t.datetime "enqueued_at"
+ t.datetime "discarded_at"
+ t.datetime "finished_at"
+ t.datetime "jobs_finished_at"
+ end
+
+ create_table "good_job_executions", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.uuid "active_job_id", null: false
+ t.text "job_class"
+ t.text "queue_name"
+ t.jsonb "serialized_params"
+ t.datetime "scheduled_at"
+ t.datetime "finished_at"
+ t.text "error"
+ t.integer "error_event", limit: 2
+ t.text "error_backtrace", array: true
+ t.uuid "process_id"
+ t.interval "duration"
+ t.index ["active_job_id", "created_at"], name: "index_good_job_executions_on_active_job_id_and_created_at"
+ t.index ["process_id", "created_at"], name: "index_good_job_executions_on_process_id_and_created_at"
+ end
+
+ create_table "good_job_processes", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.jsonb "state"
+ t.integer "lock_type", limit: 2
+ end
+
+ create_table "good_job_settings", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.text "key"
+ t.jsonb "value"
+ t.index ["key"], name: "index_good_job_settings_on_key", unique: true
+ end
+
+ create_table "good_jobs", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t|
+ t.text "queue_name"
+ t.integer "priority"
+ t.jsonb "serialized_params"
+ t.datetime "scheduled_at"
+ t.datetime "performed_at"
+ t.datetime "finished_at"
+ t.text "error"
+ t.datetime "created_at", null: false
+ t.datetime "updated_at", null: false
+ t.uuid "active_job_id"
+ t.text "concurrency_key"
+ t.text "cron_key"
+ t.uuid "retried_good_job_id"
+ t.datetime "cron_at"
+ t.uuid "batch_id"
+ t.uuid "batch_callback_id"
+ t.boolean "is_discrete"
+ t.integer "executions_count"
+ t.text "job_class"
+ t.integer "error_event", limit: 2
+ t.text "labels", array: true
+ t.uuid "locked_by_id"
+ t.datetime "locked_at"
+ t.index ["active_job_id", "created_at"], name: "index_good_jobs_on_active_job_id_and_created_at"
+ t.index ["batch_callback_id"], name: "index_good_jobs_on_batch_callback_id", where: "(batch_callback_id IS NOT NULL)"
+ t.index ["batch_id"], name: "index_good_jobs_on_batch_id", where: "(batch_id IS NOT NULL)"
+ t.index ["concurrency_key", "created_at"], name: "index_good_jobs_on_concurrency_key_and_created_at"
+ t.index ["concurrency_key"], name: "index_good_jobs_on_concurrency_key_when_unfinished", where: "(finished_at IS NULL)"
+ t.index ["cron_key", "created_at"], name: "index_good_jobs_on_cron_key_and_created_at_cond", where: "(cron_key IS NOT NULL)"
+ t.index ["cron_key", "cron_at"], name: "index_good_jobs_on_cron_key_and_cron_at_cond", unique: true, where: "(cron_key IS NOT NULL)"
+ t.index ["finished_at"], name: "index_good_jobs_jobs_on_finished_at_only", where: "(finished_at IS NOT NULL)"
+ t.index ["job_class"], name: "index_good_jobs_on_job_class"
+ t.index ["labels"], name: "index_good_jobs_on_labels", where: "(labels IS NOT NULL)", using: :gin
+ t.index ["locked_by_id"], name: "index_good_jobs_on_locked_by_id", where: "(locked_by_id IS NOT NULL)"
+ t.index ["priority", "created_at"], name: "index_good_job_jobs_for_candidate_lookup", where: "(finished_at IS NULL)"
+ t.index ["priority", "created_at"], name: "index_good_jobs_jobs_on_priority_created_at_when_unfinished", order: { priority: "DESC NULLS LAST" }, where: "(finished_at IS NULL)"
+ t.index ["priority", "scheduled_at"], name: "index_good_jobs_on_priority_scheduled_at_unfinished_unlocked", where: "((finished_at IS NULL) AND (locked_by_id IS NULL))"
+ t.index ["queue_name", "scheduled_at"], name: "index_good_jobs_on_queue_name_and_scheduled_at", where: "(finished_at IS NULL)"
+ t.index ["scheduled_at"], name: "index_good_jobs_on_scheduled_at", where: "(finished_at IS NULL)"
+ end
+
create_table "lms_credentials", force: :cascade do |t|
t.bigint "user_id"
t.string "lms_name"
diff --git a/features/assignments.feature b/features/assignments.feature
index d6978f7f..61f07628 100644
--- a/features/assignments.feature
+++ b/features/assignments.feature
@@ -15,4 +15,17 @@ Feature: Course Assignments
Then I log in as a student
And I go to the Course page
Then I should not see "Homework 1"
- Then I should see "Homework 2"
\ No newline at end of file
+ Then I should see "Homework 2"
+
+ Scenario: Instructor sees the Sync Assignments button
+ Given I'm logged in as a teacher
+ When I go to the Course page
+ Then I should see a "Sync Assignments" button
+
+ @javascript
+ Scenario: Clicking Sync Assignments disables the button and shows a spinner
+ Given I'm logged in as a teacher
+ When I go to the Course page
+ And I click the "Sync Assignments" button
+ Then the "Sync Assignments" button should be disabled
+ And I should see a loading spinner
\ No newline at end of file
diff --git a/features/enrollments.feature b/features/enrollments.feature
index 8d16f577..4be7d1c0 100644
--- a/features/enrollments.feature
+++ b/features/enrollments.feature
@@ -35,3 +35,16 @@ Feature: Course Enrollments
And I click the name link for student "User 3"
Then I should be on the "Requests page" with param show_all=true
And the requests table search should be filtered
+
+ Scenario: Instructor sees the Sync Enrollments button
+ Given I'm logged in as a teacher
+ When I go to the Course Enrollments page
+ Then I should see a "Sync Enrollments" button
+
+ @javascript
+ Scenario: Clicking Sync Enrollments disables the button and shows a spinner
+ Given I'm logged in as a teacher
+ When I go to the Course Enrollments page
+ And I click the "Sync Enrollments" button
+ Then the "Sync Enrollments" button should be disabled
+ And I should see a loading spinner
diff --git a/features/step_definitions/custom_steps.rb b/features/step_definitions/custom_steps.rb
index 651f5c7a..e1b33e54 100644
--- a/features/step_definitions/custom_steps.rb
+++ b/features/step_definitions/custom_steps.rb
@@ -218,6 +218,41 @@
expect(page.current_path).to eq(expected_path)
end
+###################
+# SYNC UI #
+###################
+
+# Then I should see a "Sync Enrollments" button
+Then(/^I should see a "([^"]*)" button$/) do |label|
+ expect(page).to have_button(label)
+end
+
+# When I click the "Sync Enrollments" button
+When(/^I click the "([^"]*)" button$/) do |label|
+ # csrf_meta_tags renders nothing when allow_forgery_protection = false (test env).
+ # Inject a placeholder token so JS fetch handlers don't throw before disabling the button.
+ page.execute_script(<<~JS)
+ if (!document.querySelector('meta[name="csrf-token"]')) {
+ const m = document.createElement('meta');
+ m.name = 'csrf-token';
+ m.content = 'test-csrf-token';
+ document.head.appendChild(m);
+ }
+ JS
+ click_button(label)
+end
+
+# Then the "Sync Enrollments" button should be disabled
+# When sync starts, the label changes to "Syncing..." so we check for that text + disabled
+Then(/^the "([^"]*)" button should be disabled$/) do |_label|
+ expect(page).to have_button("Syncing...", disabled: true)
+end
+
+# Then I should see a loading spinner
+Then(/^I should see a loading spinner$/) do
+ expect(page).to have_css('.spinner-border:not(.d-none)', visible: true)
+end
+
Given(/^I deny the request for "([^"]*)"$/) do |assignment_name|
request = Request.joins(:assignment)
.find_by(assignments: { name: assignment_name }, status: 'pending')
diff --git a/spec/jobs/sync_all_course_assignments_job_spec.rb b/spec/jobs/sync_all_course_assignments_job_spec.rb
index 079b0dd1..6f195486 100644
--- a/spec/jobs/sync_all_course_assignments_job_spec.rb
+++ b/spec/jobs/sync_all_course_assignments_job_spec.rb
@@ -131,19 +131,43 @@
end
- # 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)
-
- assignment = Assignment.last
- expect(assignment.name).to eq('HW1')
+ describe '#sync_assignment' do
+ let(:job) { described_class.new }
+ let(:lms_assignment) do
+ build_canvas_assignment('id' => 'a123', 'name' => 'HW1', 'due_at' => '2025-06-01T23:59:00Z', 'lock_at' => nil)
+ end
+ let(:results) { { added_assignments: 0, updated_assignments: 0, unchanged_assignments: 0 } }
+
+ before { Assignment.where(course_to_lms: course_to_lms).destroy_all }
+
+ it 'creates a new assignment' do
+ expect {
+ job.sync_assignment(course_to_lms, lms_assignment, results)
+ }.to change(Assignment, :count).by(1)
+
+ assignment = Assignment.find_by(external_assignment_id: 'a123')
+ expect(assignment).to have_attributes(name: 'HW1', due_date: DateTime.parse('2025-06-01T23:59:00Z'))
+ expect(results[:added_assignments]).to eq(1)
+ end
+
+ it 'updates an existing assignment' do
+ create(:assignment, course_to_lms: course_to_lms, external_assignment_id: 'a123', name: 'Old Name')
+
+ expect {
+ job.sync_assignment(course_to_lms, lms_assignment, results)
+ }.not_to change(Assignment, :count)
+
+ expect(Assignment.find_by(external_assignment_id: 'a123').name).to eq('HW1')
+ expect(results[:updated_assignments]).to eq(1)
+ end
+
+ it 'increments unchanged_assignments when nothing changed' do
+ create(:assignment, course_to_lms: course_to_lms, external_assignment_id: 'a123',
+ name: 'HW1', due_date: DateTime.parse('2025-06-01T23:59:00Z'), late_due_date: nil)
+
+ job.sync_assignment(course_to_lms, lms_assignment, results)
+
+ expect(results[:unchanged_assignments]).to eq(1)
end
end
diff --git a/spec/jobs/sync_users_from_canvas_job_spec.rb b/spec/jobs/sync_users_from_canvas_job_spec.rb
new file mode 100644
index 00000000..bb75694c
--- /dev/null
+++ b/spec/jobs/sync_users_from_canvas_job_spec.rb
@@ -0,0 +1,194 @@
+require 'rails_helper'
+
+RSpec.describe SyncUsersFromCanvasJob, type: :job do
+ let(:course) { create(:course, :with_staff) }
+ let(:sync_user) { course.staff_users.first }
+ let(:canvas_facade_double) { instance_double(CanvasFacade) }
+
+ before do
+ allow(sync_user).to receive(:ensure_fresh_canvas_token!).and_return('fake_token')
+ allow(CanvasFacade).to receive(:new).with('fake_token').and_return(canvas_facade_double)
+ end
+
+ describe '#perform' do
+ context 'user upsert' do
+ let(:canvas_user) { create(:user) }
+ let(:canvas_data) do
+ [ { 'id' => canvas_user.canvas_uid, 'name' => canvas_user.name,
+ 'email' => canvas_user.email, 'sis_user_id' => canvas_user.student_id } ]
+ end
+
+ before do
+ allow(canvas_facade_double).to receive(:get_all_course_users).and_return(canvas_data)
+ end
+
+ it 'creates new users from Canvas data when they do not exist locally' do
+ new_uid = 'brand-new-canvas-uid'
+ allow(canvas_facade_double).to receive(:get_all_course_users)
+ .and_return([ { 'id' => new_uid, 'name' => 'New Person', 'email' => 'new@example.com', 'sis_user_id' => 'S99' } ])
+
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.to change(User, :count).by(1)
+
+ expect(User.find_by(canvas_uid: new_uid)).to have_attributes(name: 'New Person', email: 'new@example.com')
+ end
+
+ it 'updates an existing user without creating a duplicate' do
+ canvas_user.update!(name: 'Old Name')
+ updated_data = [ { 'id' => canvas_user.canvas_uid, 'name' => 'New Name',
+ 'email' => canvas_user.email, 'sis_user_id' => canvas_user.student_id } ]
+ allow(canvas_facade_double).to receive(:get_all_course_users).and_return(updated_data)
+
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.not_to change(User, :count)
+
+ expect(canvas_user.reload.name).to eq('New Name')
+ end
+
+ it 'skips users with a blank email' do
+ allow(canvas_facade_double).to receive(:get_all_course_users)
+ .and_return([ { 'id' => 'no-email', 'name' => 'No Email', 'email' => '', 'sis_user_id' => nil } ])
+
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.not_to change(User, :count)
+ end
+ end
+
+ context 'enrollment creation' do
+ let(:first_student) { create(:user) }
+ let(:second_student) { create(:user) }
+ let(:canvas_data) do
+ [ first_student, second_student ].map do |u|
+ { 'id' => u.canvas_uid, 'name' => u.name, 'email' => u.email, 'sis_user_id' => u.student_id }
+ end
+ end
+
+ before do
+ allow(canvas_facade_double).to receive(:get_all_course_users).and_return(canvas_data)
+ end
+
+ it 'creates UserToCourse enrollments for synced users' do
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.to change(UserToCourse, :count).by(2)
+
+ expect(UserToCourse.exists?(user: first_student, course: course, role: 'student')).to be true
+ expect(UserToCourse.exists?(user: second_student, course: course, role: 'student')).to be true
+ end
+
+ it 'assigns the correct role to enrollments' do
+ described_class.perform_now(course.id, sync_user.id, 'ta')
+
+ expect(UserToCourse.find_by(user: first_student, course: course).role).to eq('ta')
+ end
+
+ it 'does not duplicate enrollments on re-run' do
+ described_class.perform_now(course.id, sync_user.id, 'student')
+
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.not_to change(UserToCourse, :count)
+ end
+ end
+
+ context 'role-based removal' do
+ let(:remaining) { create(:user) }
+ let(:removed) { create(:user) }
+
+ before do
+ create(:user_to_course, user: removed, course: course, role: 'student')
+ allow(canvas_facade_double).to receive(:get_all_course_users)
+ .and_return([ { 'id' => remaining.canvas_uid, 'name' => remaining.name,
+ 'email' => remaining.email, 'sis_user_id' => remaining.student_id } ])
+ end
+
+ it 'removes enrollments for users no longer returned by Canvas' do
+ # Also pre-enroll `remaining` so the job's insert doesn't offset the removal
+ create(:user_to_course, user: remaining, course: course, role: 'student')
+
+ expect {
+ described_class.perform_now(course.id, sync_user.id, 'student')
+ }.to change(UserToCourse, :count).by(-1)
+
+ expect(UserToCourse.exists?(user: removed, course: course)).to be false
+ end
+
+ it 'does not remove enrollments for other roles' do
+ teacher = create(:user)
+ create(:user_to_course, user: teacher, course: course, role: 'teacher')
+
+ described_class.perform_now(course.id, sync_user.id, 'student')
+
+ expect(UserToCourse.exists?(user: teacher, course: course, role: 'teacher')).to be true
+ end
+ end
+
+ context 'multiple roles' do
+ let(:student) { create(:user) }
+ let(:ta) { create(:user) }
+
+ before do
+ allow(canvas_facade_double).to receive(:get_all_course_users).with(anything, 'student')
+ .and_return([ { 'id' => student.canvas_uid, 'name' => student.name, 'email' => student.email, 'sis_user_id' => student.student_id } ])
+ allow(canvas_facade_double).to receive(:get_all_course_users).with(anything, 'ta')
+ .and_return([ { 'id' => ta.canvas_uid, 'name' => ta.name, 'email' => ta.email, 'sis_user_id' => ta.student_id } ])
+ end
+
+ it 'syncs each role independently' do
+ described_class.perform_now(course.id, sync_user.id, %w[student ta])
+
+ expect(UserToCourse.exists?(user: student, course: course, role: 'student')).to be true
+ expect(UserToCourse.exists?(user: ta, course: course, role: 'ta')).to be true
+ end
+ end
+
+ context 'return value and persistence' do
+ # Use a canvas_uid not in the DB so the job counts this as an add, not an update
+ let(:canvas_data) do
+ [ { 'id' => 'brand-new-uid-999', 'name' => 'New Student', 'email' => 'newstudent@example.com', 'sis_user_id' => 'S999' } ]
+ end
+
+ before do
+ allow(canvas_facade_double).to receive(:get_all_course_users).and_return(canvas_data)
+ end
+
+ it 'returns results keyed by role with added/removed/updated counts' do
+ result = described_class.perform_now(course.id, sync_user.id, 'student')
+
+ # The job keys results with string role names
+ expect(result['student']).to include(added: 1, removed: 0, updated: 0)
+ end
+
+ it 'includes a synced_at timestamp' do
+ result = described_class.perform_now(course.id, sync_user.id, 'student')
+
+ expect(result[:synced_at]).to be_within(1.second).of(Time.current)
+ end
+
+ it 'persists results to course_to_lms.recent_roster_sync' do
+ described_class.perform_now(course.id, sync_user.id, 'student')
+
+ expect(course.course_to_lms(1).reload.recent_roster_sync).to include('student' => include('added' => 1))
+ end
+ end
+
+ context 'when Canvas returns a non-array response' do
+ before do
+ allow(canvas_facade_double).to receive(:get_all_course_users).and_return({ 'error' => 'unauthorized' })
+ end
+
+ it 'returns zeroed counts without raising' do
+ result = nil
+ expect {
+ result = described_class.perform_now(course.id, sync_user.id, 'student')
+ }.not_to raise_error
+
+ # The job keys results with string role names
+ expect(result['student']).to eq(added: 0, removed: 0, updated: 0)
+ end
+ end
+ end
+end
diff --git a/spec/models/course_spec.rb b/spec/models/course_spec.rb
index d85efb78..c3a9d17c 100644
--- a/spec/models/course_spec.rb
+++ b/spec/models/course_spec.rb
@@ -171,12 +171,9 @@
allow(user).to receive(:ensure_fresh_canvas_token!).and_return('fake_token')
end
- it 'creates user and user_to_course record' do
- expect do
- course.sync_users_from_canvas(user.id, 'student')
- end.to change(User, :count).by(CANVAS_USERS.size).and(
- change(UserToCourse, :count).by(CANVAS_USERS.size)
- )
+ it 'enqueues SyncUsersFromCanvasJob with the correct arguments' do
+ expect(SyncUsersFromCanvasJob).to receive(:perform_later).with(course.id, user.id, 'student')
+ course.sync_users_from_canvas(user.id, 'student')
end
end