diff --git a/app/controllers/admin/school_import_results_controller.rb b/app/controllers/admin/school_import_results_controller.rb new file mode 100644 index 000000000..4970b55e2 --- /dev/null +++ b/app/controllers/admin/school_import_results_controller.rb @@ -0,0 +1,151 @@ +# frozen_string_literal: true + +require 'csv' + +module Admin + class SchoolImportResultsController < Admin::ApplicationController + def index + search_term = params[:search].to_s.strip + resources = Administrate::Search.new( + SchoolImportResult.all, + dashboard_class, + search_term + ).run + resources = apply_collection_includes(resources) + resources = order.apply(resources) + resources = resources.page(params[:_page]).per(records_per_page) + + # Batch load user info to avoid N+1 queries + user_ids = resources.filter_map(&:user_id).uniq + RequestStore.store[:user_info_cache] = fetch_users_batch(user_ids) + + page = Administrate::Page::Collection.new(dashboard, order: order) + + render locals: { + resources: resources, + search_term: search_term, + page: page, + show_search_bar: show_search_bar? + } + end + + def show + respond_to do |format| + format.html do + render locals: { + page: Administrate::Page::Show.new(dashboard, requested_resource) + } + end + format.csv do + send_data generate_csv(requested_resource), + filename: "school_import_#{requested_resource.job_id}_#{Date.current.strftime('%Y-%m-%d')}.csv", + type: 'text/csv' + end + end + end + + def new + @error_details = flash[:error_details] + render locals: { + page: Administrate::Page::Form.new(dashboard, SchoolImportResult.new) + } + end + + def create + if params[:csv_file].blank? + flash[:error] = I18n.t('errors.admin.csv_file_required') + redirect_to new_admin_school_import_result_path + return + end + + # Call the same service that the API endpoint uses, ensuring all validation is applied + result = School::ImportBatch.call( + csv_file: params[:csv_file], + current_user: current_user + ) + + if result.success? + flash[:notice] = "Import job started successfully. Job ID: #{result[:job_id]}" + redirect_to admin_school_import_results_path + else + # Display error inline on the page + flash.now[:error] = format_error_message(result[:error]) + @error_details = extract_error_details(result[:error]) + render :new, locals: { + page: Administrate::Page::Form.new(dashboard, SchoolImportResult.new) + } + end + end + + private + + def default_sorting_attribute + :created_at + end + + def default_sorting_direction + :desc + end + + def format_error_message(error) + return error.to_s unless error.is_a?(Hash) + + error[:message] || error['message'] || 'Import failed' + end + + def extract_error_details(error) + return nil unless error.is_a?(Hash) + + error[:details] || error['details'] + end + + def generate_csv(import_result) + CSV.generate(headers: true) do |csv| + # Header row + csv << ['Status', 'School Name', 'School Code', 'School ID', 'Owner Email', 'Error Code', 'Error Message'] + + results = import_result.results + successful = results['successful'] || [] + failed = results['failed'] || [] + + # Successful schools + successful.each do |school| + csv << [ + 'Success', + school['name'], + school['code'], + school['id'], + school['owner_email'], + '', + '' + ] + end + + # Failed schools + failed.each do |school| + csv << [ + 'Failed', + school['name'], + '', + '', + school['owner_email'], + school['error_code'], + school['error'] + ] + end + end + end + + def fetch_users_batch(user_ids) + return {} if user_ids.empty? + + users = UserInfoApiClient.fetch_by_ids(user_ids) + return {} if users.nil? + + users.index_by { |user| user[:id] } + rescue StandardError => e + Rails.logger.error("Failed to batch fetch user info: #{e.message}") + {} + end + end +end diff --git a/app/controllers/api/school_import_jobs_controller.rb b/app/controllers/api/school_import_jobs_controller.rb new file mode 100644 index 000000000..a8cac9d2c --- /dev/null +++ b/app/controllers/api/school_import_jobs_controller.rb @@ -0,0 +1,50 @@ +# frozen_string_literal: true + +module Api + class SchoolImportJobsController < ApiController + before_action :authorize_user + + def show + authorize! :read, :school_import_job + @job = find_job + + if @job.nil? + render json: SchoolImportError.format_error(:job_not_found, 'Job not found'), status: :not_found + return + end + + @status = job_status(@job) + @result = SchoolImportResult.find_by(job_id: @job.active_job_id) if @job.succeeded? + render :show, formats: [:json], status: :ok + end + + private + + def find_job + job = GoodJob::Job.find_by(active_job_id: params[:id]) + + # Verify this is an import job (security check) + return nil unless job && job.job_class == SchoolImportJob.name + + job + end + + def job_status(job) + return 'discarded' if job.discarded? + return 'succeeded' if job.succeeded? + return 'failed' if job_failed?(job) + return 'running' if job.running? + return 'scheduled' if job_scheduled?(job) + + 'queued' + end + + def job_failed?(job) + job.finished? && job.error.present? + end + + def job_scheduled?(job) + job.scheduled_at.present? && job.scheduled_at > Time.current + end + end +end diff --git a/app/controllers/api/schools_controller.rb b/app/controllers/api/schools_controller.rb index 6f50aed0d..422a47b70 100644 --- a/app/controllers/api/schools_controller.rb +++ b/app/controllers/api/schools_controller.rb @@ -4,6 +4,7 @@ module Api class SchoolsController < ApiController before_action :authorize_user load_and_authorize_resource + skip_load_and_authorize_resource only: :import def index @schools = School.accessible_by(current_ability) @@ -47,6 +48,29 @@ def destroy end end + def import + authorize! :import, School + + if params[:csv_file].blank? + render json: { error: SchoolImportError.format_error(:csv_file_required, 'CSV file is required') }, + status: :unprocessable_entity + return + end + + result = School::ImportBatch.call( + csv_file: params[:csv_file], + current_user: current_user + ) + + if result.success? + @job_id = result[:job_id] + @total_schools = result[:total_schools] + render :import, formats: [:json], status: :accepted + else + render json: { error: result[:error] }, status: :unprocessable_entity + end + end + private def school_params diff --git a/app/dashboards/school_import_result_dashboard.rb b/app/dashboards/school_import_result_dashboard.rb new file mode 100644 index 000000000..0f0a42c86 --- /dev/null +++ b/app/dashboards/school_import_result_dashboard.rb @@ -0,0 +1,38 @@ +# frozen_string_literal: true + +require 'administrate/base_dashboard' + +class SchoolImportResultDashboard < Administrate::BaseDashboard + ATTRIBUTE_TYPES = { + id: Field::String, + job_id: StatusField, + user_id: UserInfoField, + results: ResultsSummaryField, + created_at: Field::DateTime, + updated_at: Field::DateTime + }.freeze + + COLLECTION_ATTRIBUTES = %i[ + job_id + user_id + results + created_at + ].freeze + + SHOW_PAGE_ATTRIBUTES = %i[ + id + job_id + user_id + results + created_at + updated_at + ].freeze + + FORM_ATTRIBUTES = [].freeze + + COLLECTION_FILTERS = {}.freeze + + def display_resource(school_import_result) + "Import Job #{school_import_result.job_id}" + end +end diff --git a/app/fields/results_summary_field.rb b/app/fields/results_summary_field.rb new file mode 100644 index 000000000..d054078c9 --- /dev/null +++ b/app/fields/results_summary_field.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +require 'administrate/field/base' + +class ResultsSummaryField < Administrate::Field::Base + def to_s + "#{successful_count} successful, #{failed_count} failed" + end + + def successful_count + data['successful']&.count || 0 + end + + def failed_count + data['failed']&.count || 0 + end + + def total_count + successful_count + failed_count + end +end diff --git a/app/fields/status_field.rb b/app/fields/status_field.rb new file mode 100644 index 000000000..70ee68103 --- /dev/null +++ b/app/fields/status_field.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'administrate/field/base' + +class StatusField < Administrate::Field::Base + def to_s + job_status + end + + def job_status + return 'unknown' if data.blank? + + job = GoodJob::Job.find_by(active_job_id: data) + return 'not_found' unless job + + determine_job_status(job) + end + + def status_class + case job_status + when 'succeeded', 'completed' then 'status-completed' + when 'failed', 'discarded' then 'status-failed' + when 'running' then 'status-running' + when 'queued', 'scheduled' then 'status-queued' + else 'status-unknown' + end + end + + private + + def determine_job_status(job) + return 'discarded' if job.discarded? + return 'succeeded' if job.succeeded? + return 'failed' if job.finished? && job.error.present? + return 'running' if job.running? + return 'scheduled' if scheduled?(job) + + 'queued' + end + + def scheduled?(job) + job.scheduled_at.present? && job.scheduled_at > Time.current + end +end diff --git a/app/fields/user_info_field.rb b/app/fields/user_info_field.rb new file mode 100644 index 000000000..c329561c9 --- /dev/null +++ b/app/fields/user_info_field.rb @@ -0,0 +1,67 @@ +# frozen_string_literal: true + +require 'administrate/field/base' + +class UserInfoField < Administrate::Field::Base + def to_s + user_display + end + + def user_display + return 'Unknown User' if data.blank? + + user_info = fetch_user_info + return data if user_info.nil? + + if user_info[:name].present? && user_info[:email].present? + "#{user_info[:name]} <#{user_info[:email]}>" + elsif user_info[:name].present? + user_info[:name] + elsif user_info[:email].present? + user_info[:email] + else + data + end + end + + def user_name + return nil if data.blank? + + user_info = fetch_user_info + user_info&.dig(:name) + end + + def user_email + return nil if data.blank? + + user_info = fetch_user_info + user_info&.dig(:email) + end + + def user_id + data + end + + private + + def fetch_user_info + return @fetch_user_info if defined?(@fetch_user_info) + + @fetch_user_info = begin + # Try to get from request-level cache first (set by controller) + cache = RequestStore.store[:user_info_cache] || {} + cached = cache[data] + + if cached + cached + else + # Fallback to individual API call if not in cache + result = UserInfoApiClient.fetch_by_ids([data]) + result&.first + end + rescue StandardError => e + Rails.logger.error("Failed to fetch user info for #{data}: #{e.message}") + nil + end + end +end diff --git a/app/jobs/school_import_job.rb b/app/jobs/school_import_job.rb new file mode 100644 index 000000000..85a54823e --- /dev/null +++ b/app/jobs/school_import_job.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +class SchoolImportJob < ApplicationJob + retry_on StandardError, wait: :polynomially_longer, attempts: 3 do |_job, e| + Sentry.capture_exception(e) + raise e + end + + queue_as :import_schools_job + + def perform(schools_data:, user_id:, token:) + @token = token + @results = { + successful: [], + failed: [] + } + + schools_data.map(&:with_indifferent_access).each do |school_data| + import_school(school_data) + end + + # Store results in dedicated table + store_results(@results, user_id) + + @results + end + + private + + def store_results(results, user_id) + SchoolImportResult.create!( + job_id: job_id, + user_id: user_id, + results: results + ) + rescue StandardError => e + Sentry.capture_exception(e) + # Don't fail the job if we can't store results + end + + def import_school(school_data) + proposed_owner = find_user_account_for_proposed_owner(school_data[:owner_email]) + + unless proposed_owner + @results[:failed] << { + name: school_data[:name], + error_code: SchoolImportError::CODES[:owner_not_found], + error: "Owner not found: #{school_data[:owner_email]}", + owner_email: school_data[:owner_email] + } + return + end + + # Check if this owner already has any role in any school + existing_role = Role.find_by(user_id: proposed_owner[:id]) + if existing_role + existing_school = existing_role.school + @results[:failed] << { + name: school_data[:name], + error_code: SchoolImportError::CODES[:owner_has_existing_role], + error: "Owner #{school_data[:owner_email]} already has a role in school '#{existing_school.name}'", + owner_email: school_data[:owner_email], + existing_school_id: existing_school.id + } + return + end + + school_params = build_school_params(school_data) + + # Use transaction for atomicity + School.transaction do + result = School::Create.call( + school_params: school_params, + creator_id: proposed_owner[:id] + ) + + if result.success? + school = result[:school] + + # Auto-verify the imported school using the verification service + SchoolVerificationService.new(school).verify(token: @token) + + @results[:successful] << { + name: school.name, + id: school.id, + code: school.code, + owner_email: school_data[:owner_email] + } + else + @results[:failed] << { + name: school_data[:name], + error_code: SchoolImportError::CODES[:school_validation_failed], + error: format_errors(result[:error]), + owner_email: school_data[:owner_email] + } + end + end + rescue StandardError => e + @results[:failed] << { + name: school_data[:name], + error_code: SchoolImportError::CODES[:unknown_error], + error: e.message, + owner_email: school_data[:owner_email] + } + Sentry.capture_exception(e) + end + + def find_user_account_for_proposed_owner(email) + return nil if email.blank? + + UserInfoApiClient.find_user_by_email(email) + rescue StandardError => e + Sentry.capture_exception(e) + nil + end + + def build_school_params(school_data) + { + name: school_data[:name], + website: school_data[:website], + address_line_1: school_data[:address_line_1], + address_line_2: school_data[:address_line_2], + municipality: school_data[:municipality], + administrative_area: school_data[:administrative_area], + postal_code: school_data[:postal_code], + country_code: school_data[:country_code]&.upcase, + reference: school_data[:reference], + creator_agree_authority: true, + creator_agree_terms_and_conditions: true, + creator_agree_responsible_safeguarding: true, + user_origin: 'experience_cs' + }.compact + end + + def format_errors(errors) + return errors.to_s unless errors.respond_to?(:full_messages) + + errors.full_messages.join(', ') + end +end diff --git a/app/models/ability.rb b/app/models/ability.rb index 65bc1987a..02c844c1e 100644 --- a/app/models/ability.rb +++ b/app/models/ability.rb @@ -15,6 +15,7 @@ def initialize(user) define_school_owner_abilities(school:) if user.school_owner?(school) end + define_editor_admin_abilities(user) define_experience_cs_admin_abilities(user) end @@ -120,10 +121,24 @@ def define_school_student_abilities(user:, school:) can(%i[show_finished set_finished show_status unsubmit submit], SchoolProject, project: { user_id: user.id, lesson_id: nil }, school_id: school.id) end + def define_school_import_abilities(user) + return unless user&.admin? || user&.experience_cs_admin? + + can :import, School + can :read, :school_import_job + end + + def define_editor_admin_abilities(user) + return unless user&.admin? + + define_school_import_abilities(user) + end + def define_experience_cs_admin_abilities(user) return unless user&.experience_cs_admin? can %i[read create update destroy], Project, user_id: nil + define_school_import_abilities(user) end def school_teacher_can_manage_lesson?(user:, school:, lesson:) diff --git a/app/models/school_import_error.rb b/app/models/school_import_error.rb new file mode 100644 index 000000000..eab0680c0 --- /dev/null +++ b/app/models/school_import_error.rb @@ -0,0 +1,37 @@ +# frozen_string_literal: true + +module SchoolImportError + CODES = { + csv_invalid_format: 'CSV_INVALID_FORMAT', + csv_malformed: 'CSV_MALFORMED', + csv_validation_failed: 'CSV_VALIDATION_FAILED', + owner_not_found: 'OWNER_NOT_FOUND', + owner_already_creator: 'OWNER_ALREADY_CREATOR', + owner_has_existing_role: 'OWNER_HAS_EXISTING_ROLE', + duplicate_owner_email: 'DUPLICATE_OWNER_EMAIL', + school_validation_failed: 'SCHOOL_VALIDATION_FAILED', + job_not_found: 'JOB_NOT_FOUND', + csv_file_required: 'CSV_FILE_REQUIRED', + unknown_error: 'UNKNOWN_ERROR' + }.freeze + + class << self + def format_error(code, message, details = {}) + { + error_code: CODES[code] || CODES[:unknown_error], + message: message, + details: details + }.compact + end + + def format_row_errors(errors_array) + { + error_code: CODES[:csv_validation_failed], + message: 'CSV validation failed', + details: { + row_errors: errors_array + } + } + end + end +end diff --git a/app/models/school_import_result.rb b/app/models/school_import_result.rb new file mode 100644 index 000000000..eca70e3b5 --- /dev/null +++ b/app/models/school_import_result.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class SchoolImportResult < ApplicationRecord + validates :job_id, presence: true, uniqueness: true + validates :user_id, presence: true + validates :results, presence: true +end diff --git a/app/views/admin/school_import_results/index.html.erb b/app/views/admin/school_import_results/index.html.erb new file mode 100644 index 000000000..0a601f2f4 --- /dev/null +++ b/app/views/admin/school_import_results/index.html.erb @@ -0,0 +1,43 @@ +<%# +# Index + +This view is the template for the index page. +It renders the search bar, header and pagination. +It renders the `_table` partial to display details about the resources. + +## Local variables: + +- `resources`: + An ActiveModel::Relation collection of resources to be displayed in the table. + By default, the number of resources is limited by pagination + or by a hard limit to prevent excessive page load times +%> + +<% content_for(:title) { "School Import History" } %> + +
+

+ <%= content_for(:title) %> +

+ +
+ <%= link_to( + "New Import", + new_admin_school_import_result_path, + class: "button button--primary", + ) %> +
+
+ +
+ <%= render( + "collection", + collection_presenter: page, + collection_field_name: :resources, + page: page, + resources: resources, + table_title: "page_title" + ) %> + + <%= paginate resources, param_name: :_page %> +
diff --git a/app/views/admin/school_import_results/new.html.erb b/app/views/admin/school_import_results/new.html.erb new file mode 100644 index 000000000..b2b1810a6 --- /dev/null +++ b/app/views/admin/school_import_results/new.html.erb @@ -0,0 +1,165 @@ +<% content_for(:title) { "Upload School Import CSV" } %> + +
+

+ <%= content_for(:title) %> +

+
+ <%= link_to( + "View Import History", + admin_school_import_results_path, + class: "button" + ) %> +
+
+ +
+ <% if flash[:error] %> +
+ <%= flash[:error] %> + + <% if @error_details %> + <% if @error_details[:row_errors] %> +
+ Row Errors: + + + + + + + + + <% @error_details[:row_errors].each do |error| %> + + + + + <% end %> + +
RowErrors
<%= error[:row] || error['row'] %> + <% errors = error[:errors] || error['errors'] || [] %> + <% errors.each do |err| %> +
<%= err[:field] || err['field'] %>: <%= err[:message] || err['message'] %>
+ <% end %> +
+
+ <% elsif @error_details[:duplicate_emails] %> +
+ Duplicate Owner Emails Found: +
    + <% @error_details[:duplicate_emails].each do |email| %> +
  • <%= email %>
  • + <% end %> +
+
+ <% end %> + <% end %> +
+ <% end %> + +
+

+ Use this form to import multiple schools from a CSV file. + The import will run in the background and you can track its progress on the Import History page. +

+
+ +
+ + <%= form_with url: admin_school_import_results_path, method: :post, multipart: true, class: "form" do |f| %> +
+ <%= f.label :csv_file, "CSV File", class: "field-unit__label" %> + <%= f.file_field :csv_file, accept: ".csv", required: true, class: "field-unit__field" %> +

+ Select a CSV file containing school data to import. + Download the <%= link_to "CSV template", "/docs/import/school_import_template.csv" %> to see the required format. +

+
+ +
+ <%= f.submit "Upload and Start Import", class: "button button--primary" %> +
+ <% end %> + +
+ +
+

CSV Format Requirements

+ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + +
ColumnRequiredDescription
nameYesSchool name
websiteYesSchool website URL (must include https://)
address_line_1YesStreet address
municipalityYesCity/town
country_codeYes2-letter ISO country code (e.g., US, GB, CA)
owner_emailYesEmail of school owner (must have existing RPF account)
address_line_2NoAdditional address information
administrative_areaNoState/province
postal_codeNoZIP/postal code
referenceNoSchool reference number (must be unique if provided)
+
+ +
+

Important Notes

+
+
+ +
+
diff --git a/app/views/admin/school_import_results/show.html.erb b/app/views/admin/school_import_results/show.html.erb new file mode 100644 index 000000000..cfe872a17 --- /dev/null +++ b/app/views/admin/school_import_results/show.html.erb @@ -0,0 +1,166 @@ +<% content_for(:title) { "School Import Job #{page.resource.job_id}" } %> + +
+

+ <%= content_for(:title) %> +

+
+ <%= link_to( + "Download CSV", + admin_school_import_result_path(page.resource, format: :csv), + class: "button button--primary", + style: "margin-right: 10px;" + ) %> + <%= link_to( + "Back to Import History", + admin_school_import_results_path, + class: "button" + ) %> +
+
+ +
+
+
Job ID
+
<%= page.resource.job_id %>
+ +
User
+
+ <% user_field = UserInfoField.new(:user_id, page.resource.user_id, "show") %> + <%= user_field.user_display %> +
+ +
Status
+
+ <% status_field = StatusField.new(:job_id, page.resource.job_id, "show") %> + + <%= status_field.job_status.upcase %> + +
+ +
Created At
+
<%= page.resource.created_at.strftime("%B %d, %Y at %I:%M %p") %>
+ + <% if page.resource.updated_at != page.resource.created_at %> +
Updated At
+
<%= page.resource.updated_at.strftime("%B %d, %Y at %I:%M %p") %>
+ <% end %> +
+ + <% results = page.resource.results %> + <% if results.present? %> +
+
Results
+
+ <% results_field = ResultsSummaryField.new(:results, page.resource.results, "show") %> + <%= render "fields/results_summary_field/show", field: results_field %> +
+
+ + <% successful = results['successful'] || [] %> + <% failed = results['failed'] || [] %> + + <% if successful.any? %> +

Successful Imports (<%= successful.count %>)

+ + + + + + + + + + + + <% successful.each do |school| %> + + + + + + + + <% end %> + +
School NameSchool CodeSchool IDOwner EmailActions
<%= school['name'] %><%= school['code'] %><%= school['id'] %><%= school['owner_email'] %> + <%= link_to "View School", admin_school_path(school['id']), class: "button button--small" if school['id'] %> +
+ <% end %> + + <% if failed.any? %> +

Failed Imports (<%= failed.count %>)

+ + + + + + + + + + + <% failed.each do |school| %> + + + + + + + <% end %> + +
School NameOwner EmailError CodeError Message
<%= school['name'] %><%= school['owner_email'] %><%= school['error_code'] %><%= school['error'] %>
+ +
+

Common Error Codes

+
+
OWNER_NOT_FOUND
+
The owner email doesn't correspond to an existing account. The owner needs to create a Code Editor for Education account first.
+ +
OWNER_ALREADY_CREATOR
+
This owner has already created a school. Each person can only create one school.
+ +
SCHOOL_VALIDATION_FAILED
+
The school data failed validation. Check the error message for details about which fields are invalid.
+
+
+ <% end %> + <% else %> +
+

No results available yet. The job may still be running or may have failed to store results.

+
+ <% end %> +
+ + diff --git a/app/views/api/school_import_jobs/show.json.jbuilder b/app/views/api/school_import_jobs/show.json.jbuilder new file mode 100644 index 000000000..b675ed52b --- /dev/null +++ b/app/views/api/school_import_jobs/show.json.jbuilder @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +json.id @job.active_job_id +json.status @status +json.created_at @job.created_at +json.finished_at @job.finished_at +json.job_class @job.job_class + +if @result.present? + json.results @result.results +elsif @status == 'succeeded' + json.message 'Job completed successfully' +end + +json.error @job.error if @job.error.present? diff --git a/app/views/api/schools/import.json.jbuilder b/app/views/api/schools/import.json.jbuilder new file mode 100644 index 000000000..b56dc59d5 --- /dev/null +++ b/app/views/api/schools/import.json.jbuilder @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +json.job_id @job_id +json.total_schools @total_schools +json.message 'Import job started successfully' diff --git a/app/views/fields/results_summary_field/_index.html.erb b/app/views/fields/results_summary_field/_index.html.erb new file mode 100644 index 000000000..563ba43e6 --- /dev/null +++ b/app/views/fields/results_summary_field/_index.html.erb @@ -0,0 +1,6 @@ +<%# +Administrate field partial for ResultsSummaryField in index view +%> +<%= field.successful_count %> ✓ +/ +<%= field.failed_count %> ✗ diff --git a/app/views/fields/results_summary_field/_show.html.erb b/app/views/fields/results_summary_field/_show.html.erb new file mode 100644 index 000000000..fa41d2d45 --- /dev/null +++ b/app/views/fields/results_summary_field/_show.html.erb @@ -0,0 +1,6 @@ +<%# +Administrate field partial for ResultsSummaryField in show view +%> +<%= field.successful_count %> ✓ +/ +<%= field.failed_count %> ✗ diff --git a/app/views/fields/status_field/_index.html.erb b/app/views/fields/status_field/_index.html.erb new file mode 100644 index 000000000..f4490227c --- /dev/null +++ b/app/views/fields/status_field/_index.html.erb @@ -0,0 +1,6 @@ +<%# +Administrate field partial for StatusField in index view +%> + + <%= field.job_status.upcase %> + diff --git a/app/views/fields/status_field/_show.html.erb b/app/views/fields/status_field/_show.html.erb new file mode 100644 index 000000000..5cb80b6dc --- /dev/null +++ b/app/views/fields/status_field/_show.html.erb @@ -0,0 +1,6 @@ +<%# +Administrate field partial for StatusField in show view +%> + + <%= field.job_status.upcase %> + diff --git a/app/views/fields/user_info_field/_index.html.erb b/app/views/fields/user_info_field/_index.html.erb new file mode 100644 index 000000000..f0a6e8a2c --- /dev/null +++ b/app/views/fields/user_info_field/_index.html.erb @@ -0,0 +1,4 @@ +<%# +Administrate field partial for UserInfoField in index view +%> +<%= field.user_display %> diff --git a/app/views/fields/user_info_field/_show.html.erb b/app/views/fields/user_info_field/_show.html.erb new file mode 100644 index 000000000..83e538df3 --- /dev/null +++ b/app/views/fields/user_info_field/_show.html.erb @@ -0,0 +1,4 @@ +<%# +Administrate field partial for UserInfoField in show view +%> +<%= field.user_display %> diff --git a/config/initializers/good_job.rb b/config/initializers/good_job.rb index 65d1b301e..600f199eb 100644 --- a/config/initializers/good_job.rb +++ b/config/initializers/good_job.rb @@ -18,5 +18,5 @@ def authenticate_admin # The create_students_job queue is a serial queue that allows only one job at a time. # DO NOT change the value of create_students_job:1 without understanding the implications # of processing more than one user creation job at once. - config.good_job.queues = 'create_students_job:1;default:5' + config.good_job.queues = 'create_students_job:1;import_schools_job:1;default:5' end diff --git a/config/locales/en.yml b/config/locales/en.yml index 674aeb89e..df0bccc02 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2,6 +2,7 @@ en: errors: admin: unauthorized: "Not authorized." + csv_file_required: "A CSV file is required." project: editing: delete_default_component: "Cannot delete default file" diff --git a/config/routes.rb b/config/routes.rb index 813ad8737..daa54ef0b 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -19,6 +19,7 @@ resources :school_classes, only: %i[show] resources :lessons, only: %i[show] + resources :school_import_results, only: %i[index show new create] root to: 'projects#index' end @@ -57,6 +58,7 @@ resource :school, only: [:show], controller: 'my_school' resources :schools, only: %i[index show create update destroy] do + post :import, on: :collection resources :members, only: %i[index], controller: 'school_members' resources :classes, only: %i[index show create update destroy], controller: 'school_classes' do post :import, on: :collection @@ -81,6 +83,7 @@ end resources :user_jobs, only: %i[index show] + resources :school_import_jobs, only: %i[show] post '/google/auth/exchange-code', to: 'google_auth#exchange_code', defaults: { format: :json } end diff --git a/db/migrate/20251120092548_create_school_import_results.rb b/db/migrate/20251120092548_create_school_import_results.rb new file mode 100644 index 000000000..84f826a7f --- /dev/null +++ b/db/migrate/20251120092548_create_school_import_results.rb @@ -0,0 +1,15 @@ +# frozen_string_literal: true + +class CreateSchoolImportResults < ActiveRecord::Migration[7.1] + def change + create_table :school_import_results, id: :uuid do |t| + t.uuid :job_id, null: false + t.uuid :user_id, null: false + t.jsonb :results, null: false, default: {} + t.timestamps + end + + add_index :school_import_results, :job_id, unique: true + add_index :school_import_results, :user_id + end +end diff --git a/db/schema.rb b/db/schema.rb index a7141a0bc..188db48cc 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -250,6 +250,16 @@ t.index ["school_id"], name: "index_school_classes_on_school_id" end + create_table "school_import_results", id: :uuid, default: -> { "gen_random_uuid()" }, force: :cascade do |t| + t.uuid "job_id", null: false + t.uuid "user_id", null: false + t.jsonb "results", default: {}, null: false + t.datetime "created_at", null: false + t.datetime "updated_at", null: false + t.index ["job_id"], name: "index_school_import_results_on_job_id", unique: true + t.index ["user_id"], name: "index_school_import_results_on_user_id" + end + create_table "school_project_transitions", force: :cascade do |t| t.string "from_state", null: false t.string "to_state", null: false diff --git a/docs/import/school_import_csm_guide.md b/docs/import/school_import_csm_guide.md new file mode 100644 index 000000000..7e516f1a4 --- /dev/null +++ b/docs/import/school_import_csm_guide.md @@ -0,0 +1,128 @@ +# School Import Quick Reference for Customer Success Managers + +## Prerequisites + +1. School owners must have existing Code Editor for Education accounts. +3. School owners must be unique within the CSV, and in the existing database. +4. CSV file must be properly formatted (see template) + +## Step-by-Step Process + +### 1. Prepare the CSV File + +Download the template from the admin interface. + +Required columns: +- `name` - School name +- `website` - School website URL (e.g., https://example.edu) +- `address_line_1` - Street address +- `municipality` - City/town +- `country_code` - 2-letter code (US, GB, CA, etc.) +- `owner_email` - Email of school owner (must exist in Profile) + +Optional columns: +- `address_line_2` - Additional address info +- `administrative_area` - State/province +- `postal_code` - ZIP/postal code +- `reference` - School reference number (must be unique if provided) + +### 2. Validate Owner Emails + +Before importing, verify that all owner emails in your CSV correspond to existing accounts in Code Editor for Education. Owners without accounts will cause those schools to fail during import, but schools with correct owners will succeed. + +### 3. Upload the CSV + +Visit the `/admin` page and select **School Import Results**. On this page, click the **New Import** button. + +On the following page, you can click **Choose file** to select a CSV, then choose **Upload and Start Import** to import the CSV. + +### 4. Track Progress + +You can refresh the **School Import Results** page to see the status of your upload. It should not take very long to complete. + +### 5. Review Results + +On the **School Import Results** page, any administrator can see the history of successful uploads and inspect any particular upload. A brief summary of the number of schools that succeeded and failed is shown in the table. + +Click on a row in the table to inspect an upload futher. In the following page, you can also download a CSV of the results which will include the system-generated school code. + +### 6. Handle Failures + +Common failure reasons and solutions: + +| Error Code | Error Message | Solution | +|------------|---------------|----------| +| `OWNER_NOT_FOUND` | Owner not found: email@example.com | Verify owner has CEfE account, create if needed | +| `OWNER_ALREADY_CREATOR` | Owner is already the creator of school 'X' | Each person can only create one school; use different owner | +| `SCHOOL_VALIDATION_FAILED` | Validation errors | Check country code, website format, required fields | +| `CSV_VALIDATION_FAILED` | CSV validation failed | Check error details for specific row and field errors | +| `DUPLICATE_OWNER_EMAIL` | Duplicate owner emails found in CSV | Same owner email appears multiple times - only one school per owner allowed | + +For failed schools, you can either: +1. Fix the issues and re-import (only failed schools) +2. Create them manually through the standard process + +## Important Notes + +- **Automatic Verification**: Imported schools are automatically verified and receive school codes +- **Owner Roles**: Owner roles are automatically created +- **One Owner Per School**: Each owner can only create one school (enforced at database level) +- **No Duplicate Owners in CSV**: The CSV cannot contain the same owner email multiple times +- **No Teacher Creation**: Teachers are NOT created during import - they must be invited separately +- **Country Codes**: Use ISO 3166-1 alpha-2 codes (find at https://en.wikipedia.org/wiki/ISO_3166-1_alpha-2) +- **Structured Errors**: All errors include error codes for programmatic handling + +## Common Country Codes + +| Country | Code | +|---------|------| +| United States | US | +| United Kingdom | GB | +| Canada | CA | +| Mexico | MX | + +## Troubleshooting + +### CSV Validation Fails Immediately + +The system validates the entire CSV before starting the import. If validation fails, you'll receive an error immediately on the upload page. + +To fix: +1. Check that all required headers are present (case-sensitive) +2. Verify all required fields have values for every row +3. Check country codes are valid 2-letter codes (US not USA) +4. Verify website URLs are properly formatted (must include https://) +5. Look for the row number in the error details + +### Import Succeeds But Some Schools Failed + +This is normal - the system processes all schools it can. Review the failed list and address issues individually. + +### Duplicate Owner Emails in CSV + +If your CSV contains the same owner email more than once, the import will be rejected before processing. + +**Solution**: Each owner can only create one school. Either: +- Use different owner emails for each school +- Have one owner create their school first, then import the rest +- Remove duplicate entries and import in batches + +### All Schools Failed + +Common causes: +- Owner emails don't match any accounts +- CSV formatting issues +- Network/system errors (check with technical team) + +## Getting Help + +If you encounter issues: + +1. Check the error message and error_code for specific details +2. Verify your CSV matches the template format +3. Confirm all owner emails are valid accounts +4. Ensure no duplicate owner emails in your CSV +5. Contact the technical team with: + - The job_id + - The CSV file (or sample rows) + - Error codes and messages received diff --git a/docs/import/school_import_template.csv b/docs/import/school_import_template.csv new file mode 100644 index 000000000..c5412ead6 --- /dev/null +++ b/docs/import/school_import_template.csv @@ -0,0 +1,4 @@ +name,website,address_line_1,address_line_2,municipality,administrative_area,postal_code,country_code,reference,owner_email +Springfield Elementary School,https://springfield-elem.edu,123 Main Street,,Springfield,IL,62701,US,SPE-001,principal@springfield-elem.edu +Shelbyville High School,https://shelbyville-high.edu,456 Oak Avenue,Suite 100,Shelbyville,IL,62565,US,SHS-002,admin@shelbyville-high.edu +Capital City Academy,https://capital-city-academy.edu,789 State Road,,Capital City,IL,62702,US,CCA-003,headteacher@capital-city-academy.edu diff --git a/lib/concepts/school/operations/import_batch.rb b/lib/concepts/school/operations/import_batch.rb new file mode 100644 index 000000000..282f3c50b --- /dev/null +++ b/lib/concepts/school/operations/import_batch.rb @@ -0,0 +1,163 @@ +# frozen_string_literal: true + +class School + class ImportBatch + REQUIRED_FIELDS = %i[name website address_line_1 municipality country_code owner_email].freeze + + class << self + def call(csv_file:, current_user:) + response = OperationResponse.new + + parsed_schools = parse_csv(csv_file) + + if parsed_schools[:error] + response[:error] = parsed_schools[:error] + return response + end + + # Check for duplicate owner emails in the CSV + duplicate_check = check_duplicate_owners(parsed_schools[:schools]) + if duplicate_check[:error] + response[:error] = duplicate_check[:error] + return response + end + + job = enqueue_import_job( + schools_data: parsed_schools[:schools], + current_user: current_user + ) + + response[:job_id] = job.job_id + response[:total_schools] = parsed_schools[:schools].length + response + rescue StandardError => e + Sentry.capture_exception(e) + response[:error] = SchoolImportError.format_error(:unknown_error, e.message) + response + end + + private + + def check_duplicate_owners(schools) + owner_emails = schools.filter_map { |s| s[:owner_email]&.strip&.downcase } + duplicates = owner_emails.select { |e| owner_emails.count(e) > 1 }.uniq + + if duplicates.any? + error = SchoolImportError.format_error( + :duplicate_owner_email, + 'Duplicate owner emails found in CSV', + { duplicate_emails: duplicates } + ) + return { error: error } + end + + {} + end + + def parse_csv(csv_file) + require 'csv' + + csv_content = csv_file.read + return { error: SchoolImportError.format_error(:csv_invalid_format, 'CSV file is empty') } if csv_content.blank? + + csv_data = CSV.parse(csv_content, headers: true, header_converters: :symbol) + + if csv_data.headers.nil? || !valid_headers?(csv_data.headers) + return { + error: SchoolImportError.format_error( + :csv_invalid_format, + 'Invalid CSV format. Required headers: name, website, address_line_1, municipality, country_code, owner_email' + ) + } + end + + process_csv_rows(csv_data) + rescue CSV::MalformedCSVError => e + { error: SchoolImportError.format_error(:csv_malformed, "Invalid CSV file format: #{e.message}") } + rescue StandardError => e + Sentry.capture_exception(e) + { error: SchoolImportError.format_error(:unknown_error, "Failed to parse CSV: #{e.message}") } + end + + def process_csv_rows(csv_data) + schools = [] + errors = [] + + csv_data.each_with_index do |row, index| + row_number = index + 2 # +2 because index starts at 0 and we skip header row + school_data = row.to_h + + validation_errors = validate_school_data(school_data, row_number) + if validation_errors + errors << validation_errors + next + end + + schools << school_data + end + + if errors.any? + { error: SchoolImportError.format_row_errors(errors) } + else + { schools: schools } + end + end + + def valid_headers?(headers) + REQUIRED_FIELDS.all? { |h| headers.include?(h) } + end + + def validate_school_data(data, row_number) + errors = [] + + # Strip whitespace from all string fields + data.each do |key, value| + data[key] = value.strip if value.is_a?(String) + end + + # Validate required fields + REQUIRED_FIELDS.each do |field| + errors << { field: field.to_s, message: 'is required' } if data[field].blank? + end + + # Validate field formats + validate_country_code(data, errors) + validate_website_format(data, errors) + validate_email_format(data, errors) + + return nil if errors.empty? + + { row: row_number, errors: errors } + end + + def validate_country_code(data, errors) + return if data[:country_code].blank? + return if ISO3166::Country.codes.include?(data[:country_code].upcase) + + errors << { field: 'country_code', message: "invalid code: #{data[:country_code]}" } + end + + def validate_website_format(data, errors) + return if data[:website].blank? + return if data[:website].match?(School::VALID_URL_REGEX) + + errors << { field: 'website', message: 'invalid format' } + end + + def validate_email_format(data, errors) + return if data[:owner_email].blank? + return if data[:owner_email].match?(URI::MailTo::EMAIL_REGEXP) + + errors << { field: 'owner_email', message: 'invalid email format' } + end + + def enqueue_import_job(schools_data:, current_user:) + SchoolImportJob.perform_later( + schools_data: schools_data, + user_id: current_user.id, + token: current_user.token + ) + end + end + end +end diff --git a/lib/user_info_api_client.rb b/lib/user_info_api_client.rb index 25a84c826..74f96370b 100644 --- a/lib/user_info_api_client.rb +++ b/lib/user_info_api_client.rb @@ -18,14 +18,34 @@ def fetch_by_ids(user_ids) transform_result(response.body.fetch('users', [])) end + def find_user_by_email(email) + return nil if email.blank? + return stubbed_user_by_email(email) if bypass_oauth? + + response = conn.get do |r| + r.url "/users/#{CGI.escape(email)}" + end + return nil if response.body.blank? + + # Single user response has 'user' key, not 'users' + user = response.body.fetch('user', nil) + user.present? ? transform_user(user) : nil + rescue Faraday::ResourceNotFound + nil + end + private def bypass_oauth? ENV.fetch('BYPASS_OAUTH', nil) == 'true' end + def transform_user(user) + user.transform_keys { |k| k.to_s.underscore.to_sym } + end + def transform_result(result) - { result: }.transform_keys { |k| k.to_s.underscore.to_sym }.fetch(:result) + result.map { |user| transform_user(user) } end def conn @@ -40,27 +60,16 @@ def conn end end + # Development/test stubbing methods - only active when BYPASS_OAUTH=true + # Delegates to UserInfoApiMock to avoid code duplication def stubbed_users(user_ids) - user_ids.map do |user_id| - { - id: user_id, - email: "user-#{user_id}@example.com", - username: nil, - parentalEmail: nil, - name: 'School Owner', - nickname: 'Owner', - country: 'United Kingdom', - country_code: 'GB', - postcode: nil, - dateOfBirth: nil, - verifiedAt: '2024-01-01T12:00:00.000Z', - createdAt: '2024-01-01T12:00:00.000Z', - updatedAt: '2024-01-01T12:00:00.000Z', - discardedAt: nil, - lastLoggedInAt: '2024-01-01T12:00:00.000Z', - roles: '' - } - end + require_relative '../spec/support/user_info_api_mock' unless defined?(UserInfoApiMock) + UserInfoApiMock.default_stubbed_users(user_ids) + end + + def stubbed_user_by_email(email) + require_relative '../spec/support/user_info_api_mock' unless defined?(UserInfoApiMock) + UserInfoApiMock.default_stubbed_user_by_email(email) end end end diff --git a/public/docs/import/school_import_template.csv b/public/docs/import/school_import_template.csv new file mode 100644 index 000000000..c5412ead6 --- /dev/null +++ b/public/docs/import/school_import_template.csv @@ -0,0 +1,4 @@ +name,website,address_line_1,address_line_2,municipality,administrative_area,postal_code,country_code,reference,owner_email +Springfield Elementary School,https://springfield-elem.edu,123 Main Street,,Springfield,IL,62701,US,SPE-001,principal@springfield-elem.edu +Shelbyville High School,https://shelbyville-high.edu,456 Oak Avenue,Suite 100,Shelbyville,IL,62565,US,SHS-002,admin@shelbyville-high.edu +Capital City Academy,https://capital-city-academy.edu,789 State Road,,Capital City,IL,62702,US,CCA-003,headteacher@capital-city-academy.edu diff --git a/spec/concepts/school/import_batch_spec.rb b/spec/concepts/school/import_batch_spec.rb new file mode 100644 index 000000000..759c4d18f --- /dev/null +++ b/spec/concepts/school/import_batch_spec.rb @@ -0,0 +1,93 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe School::ImportBatch do + describe '.call' do + let(:current_user) { instance_double(User, id: SecureRandom.uuid, token: 'test-token') } + let(:csv_content) do + <<~CSV + name,website,address_line_1,municipality,country_code,owner_email + Test School 1,https://test1.example.com,123 Main St,Springfield,US,owner1@example.com + Test School 2,https://test2.example.com,456 Oak Ave,Boston,US,owner2@example.com + CSV + end + let(:csv_file) { StringIO.new(csv_content) } + + before do + allow(SchoolImportJob).to receive(:perform_later).and_return( + instance_double(SchoolImportJob, job_id: 'test-job-id') + ) + end + + context 'with valid CSV' do + it 'returns success response with job_id' do + result = described_class.call(csv_file: csv_file, current_user: current_user) + + expect(result.success?).to be true + expect(result[:job_id]).to eq('test-job-id') + expect(result[:total_schools]).to eq(2) + end + + it 'enqueues SchoolImportJob' do + described_class.call(csv_file: csv_file, current_user: current_user) + + expect(SchoolImportJob).to have_received(:perform_later).with( + hash_including( + schools_data: array_including( + hash_including(name: 'Test School 1'), + hash_including(name: 'Test School 2') + ), + user_id: current_user.id, + token: current_user.token + ) + ) + end + end + + context 'with missing required headers' do + let(:csv_content) do + <<~CSV + name,website + Test School,https://test.example.com + CSV + end + + it 'returns error response' do + result = described_class.call(csv_file: csv_file, current_user: current_user) + + expect(result.failure?).to be true + expect(result[:error][:error_code]).to eq('CSV_INVALID_FORMAT') + expect(result[:error][:message]).to include('Invalid CSV format') + end + end + + context 'with invalid country code' do + let(:csv_content) do + <<~CSV + name,website,address_line_1,municipality,country_code,owner_email + Test School,https://test.example.com,123 Main St,Springfield,INVALID,owner@example.com + CSV + end + + it 'returns validation error' do + result = described_class.call(csv_file: csv_file, current_user: current_user) + + expect(result.failure?).to be true + expect(result[:error][:error_code]).to eq('CSV_VALIDATION_FAILED') + expect(result[:error][:details][:row_errors]).to be_present + end + end + + context 'with malformed CSV' do + let(:csv_file) { StringIO.new('this is not csv,,"') } + + it 'returns error response' do + result = described_class.call(csv_file: csv_file, current_user: current_user) + + expect(result.failure?).to be true + expect(result[:error][:error_code]).to eq('CSV_MALFORMED') + end + end + end +end diff --git a/spec/features/school/importing_schools_spec.rb b/spec/features/school/importing_schools_spec.rb new file mode 100644 index 000000000..9a3949d2f --- /dev/null +++ b/spec/features/school/importing_schools_spec.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Importing schools', type: :request do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + + let(:csv_content) do + <<~CSV + name,website,address_line_1,municipality,country_code,owner_email + Test School 1,https://test1.example.com,123 Main St,Springfield,US,owner1@example.com + Test School 2,https://test2.example.com,456 Oak Ave,Boston,US,owner2@example.com + CSV + end + + describe 'POST /api/schools/import' do + let(:csv_file) do + tempfile = Tempfile.new(['schools', '.csv']) + tempfile.write(csv_content) + tempfile.rewind + Rack::Test::UploadedFile.new(tempfile.path, 'text/csv') + end + + context 'when user is an experience_cs_admin' do + let(:admin_user) { create(:user, roles: 'experience-cs-admin') } + + before do + authenticated_in_hydra_as(admin_user) + stub_user_info_api_find_by_email( + email: 'owner@example.com', + user: { id: SecureRandom.uuid, email: 'owner@example.com' } + ) + allow(SchoolImportJob).to receive(:perform_later).and_return(instance_double(SchoolImportJob, job_id: 'test-job-id')) + end + + it 'accepts CSV file and returns 202 Accepted' do + post('/api/schools/import', headers:, params: { csv_file: csv_file }) + + expect(response).to have_http_status(:accepted) + data = JSON.parse(response.body, symbolize_names: true) + expect(data[:job_id]).to eq('test-job-id') + expect(data[:total_schools]).to eq(2) + end + end + + context 'when CSV file is missing' do + let(:admin_user) { create(:user, roles: 'experience-cs-admin') } + + before do + authenticated_in_hydra_as(admin_user) + end + + it 'responds 422 Unprocessable Entity' do + post('/api/schools/import', headers:) + + expect(response).to have_http_status(:unprocessable_entity) + data = JSON.parse(response.body, symbolize_names: true) + expect(data[:error][:error_code]).to eq('CSV_FILE_REQUIRED') + end + end + + context 'when user is not an admin' do + let(:regular_user) { create(:user, roles: '') } + + before do + authenticated_in_hydra_as(regular_user) + end + + it 'responds 403 Forbidden' do + post('/api/schools/import', headers:, params: { csv_file: csv_file }) + + expect(response).to have_http_status(:forbidden) + end + end + end + + describe 'GET /api/school_import_jobs/:id' do + let(:admin_user) { create(:user, roles: 'experience-cs-admin') } + let(:job_id) { SecureRandom.uuid } + + before do + authenticated_in_hydra_as(admin_user) + end + + context 'when job exists and is completed' do + before do + GoodJob::Job.create!( + id: SecureRandom.uuid, + active_job_id: job_id, + queue_name: 'import_schools_job', + job_class: 'SchoolImportJob', + serialized_params: {}, + created_at: 1.hour.ago, + finished_at: Time.current + ) + + SchoolImportResult.create!( + job_id: job_id, + user_id: admin_user.id, + results: { + successful: [{ name: 'Test School', id: SecureRandom.uuid, code: '12-34-56' }], + failed: [] + } + ) + end + + it 'returns job status with results' do + get("/api/school_import_jobs/#{job_id}", headers:) + + expect(response).to have_http_status(:ok) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data[:status]).to eq('succeeded') + expect(data[:results][:successful].count).to eq(1) + end + end + + context 'when job does not exist' do + it 'returns 404' do + get("/api/school_import_jobs/#{SecureRandom.uuid}", headers:) + + expect(response).to have_http_status(:not_found) + end + end + + context 'when user is not an admin' do + let(:regular_user) { create(:user) } + + before do + authenticated_in_hydra_as(regular_user) + end + + it 'responds 403 Forbidden' do + get("/api/school_import_jobs/#{job_id}", headers:) + + expect(response).to have_http_status(:forbidden) + end + end + end +end diff --git a/spec/jobs/school_import_job_spec.rb b/spec/jobs/school_import_job_spec.rb new file mode 100644 index 000000000..a66e073af --- /dev/null +++ b/spec/jobs/school_import_job_spec.rb @@ -0,0 +1,155 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe SchoolImportJob do + describe '#perform' do + let(:token) { 'test-token' } + let(:user_id) { SecureRandom.uuid } + let(:owner1_id) { SecureRandom.uuid } + let(:owner2_id) { SecureRandom.uuid } + + let(:schools_data) do + [ + { + name: 'Test School 1', + website: 'https://test1.example.com', + address_line_1: '123 Main St', + municipality: 'Springfield', + country_code: 'US', + owner_email: 'owner1@example.com' + }, + { + name: 'Test School 2', + website: 'https://test2.example.com', + address_line_1: '456 Oak Ave', + municipality: 'Boston', + country_code: 'US', + owner_email: 'owner2@example.com' + } + ] + end + + before do + allow(ProfileApiClient).to receive(:create_school).and_return(true) + end + + context 'when all schools can be created successfully' do + before do + stub_user_info_api_find_by_email( + email: 'owner1@example.com', + user: { id: owner1_id, email: 'owner1@example.com' } + ) + + stub_user_info_api_find_by_email( + email: 'owner2@example.com', + user: { id: owner2_id, email: 'owner2@example.com' } + ) + end + + it 'creates schools and returns successful results' do + results = described_class.new.perform( + schools_data: schools_data, + user_id: user_id, + token: token + ) + + expect(results[:successful].count).to eq(2) + expect(results[:failed].count).to eq(0) + + school_1 = School.find_by(name: 'Test School 1') + expect(school_1).to be_present + expect(school_1.verified?).to be true + expect(school_1.code).to be_present + expect(school_1.user_origin).to eq('experience_cs') + + # Check owner and teacher roles were created + expect(Role.owner.exists?(school_id: school_1.id, user_id: owner1_id)).to be true + expect(Role.teacher.exists?(school_id: school_1.id, user_id: owner1_id)).to be true + end + end + + context 'when owner email is not found' do + before do + stub_user_info_api_find_by_email(email: 'owner1@example.com', user: nil) + end + + it 'adds failed result for that school' do + results = described_class.new.perform( + schools_data: [schools_data.first], + user_id: user_id, + token: token + ) + + expect(results[:successful].count).to eq(0) + expect(results[:failed].count).to eq(1) + expect(results[:failed].first[:error_code]).to eq('OWNER_NOT_FOUND') + expect(results[:failed].first[:error]).to include('Owner not found') + end + end + + context 'when owner already has a role in another school' do + let(:existing_school) { create(:school, name: 'Existing School') } + + before do + Role.owner.create!(school_id: existing_school.id, user_id: owner1_id) + + stub_user_info_api_find_by_email( + email: 'owner1@example.com', + user: { id: owner1_id, email: 'owner1@example.com' } + ) + end + + it 'adds failed result for that school' do + results = described_class.new.perform( + schools_data: [schools_data.first], + user_id: user_id, + token: token + ) + + expect(results[:successful].count).to eq(0) + expect(results[:failed].count).to eq(1) + expect(results[:failed].first[:error_code]).to eq('OWNER_HAS_EXISTING_ROLE') + expect(results[:failed].first[:error]).to include('already has a role in school') + expect(results[:failed].first[:existing_school_id]).to eq(existing_school.id) + end + end + + context 'when schools_data has string keys (simulating ActiveJob serialization)' do + let(:schools_data_with_string_keys) do + [ + { + 'name' => 'Test School 1', + 'website' => 'https://test1.example.com', + 'address_line_1' => '123 Main St', + 'municipality' => 'Springfield', + 'country_code' => 'us', + 'owner_email' => 'owner1@example.com' + } + ] + end + + before do + stub_user_info_api_find_by_email( + email: 'owner1@example.com', + user: { id: owner1_id, email: 'owner1@example.com' } + ) + end + + it 'handles string keys correctly' do + results = described_class.new.perform( + schools_data: schools_data_with_string_keys, + user_id: user_id, + token: token + ) + + expect(results[:successful].count).to eq(1) + expect(results[:failed].count).to eq(0) + + school = School.find_by(name: 'Test School 1') + expect(school).to be_present + expect(school.country_code).to eq('US') + end + end + end +end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index e1f9e7283..c3ab74904 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -85,6 +85,7 @@ config.include PhraseIdentifierMock config.include ProfileApiMock config.include UserProfileMock + config.include UserInfoApiMock config.include SignInStubs, type: :request config.include SignInStubs, type: :system diff --git a/spec/requests/admin/school_import_results_spec.rb b/spec/requests/admin/school_import_results_spec.rb new file mode 100644 index 000000000..eb19274cb --- /dev/null +++ b/spec/requests/admin/school_import_results_spec.rb @@ -0,0 +1,194 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin::SchoolImportResults' do + let(:admin_user) { create(:admin_user) } + + before do + sign_in_as(admin_user) + + # Stub UserInfoApiClient to avoid external API calls + stub_user_info_api_fetch_by_ids( + user_ids: [admin_user.id], + users: [{ + id: admin_user.id, + name: 'Test Admin', + email: 'admin@test.com' + }] + ) + end + + describe 'GET /admin/school_import_results' do + it 'returns successfully' do + get admin_school_import_results_path + expect(response).to have_http_status(:success) + end + + context 'with existing import results' do + before do + SchoolImportResult.create!( + job_id: SecureRandom.uuid, + user_id: admin_user.id, + results: { 'successful' => [], 'failed' => [] } + ) + end + + it 'displays the import results' do + get admin_school_import_results_path + expect(response.body).to include('School Import History') + expect(response.body).to include('Test Admin <admin@test.com>') + end + end + end + + describe 'GET /admin/school_import_results/new' do + it 'returns successfully' do + get new_admin_school_import_result_path + expect(response).to have_http_status(:success) + end + + it 'displays the upload form' do + get new_admin_school_import_result_path + expect(response.body).to include('Upload School Import CSV') + expect(response.body).to include('csv_file') + end + end + + describe 'GET /admin/school_import_results/:id' do + let(:import_result) do + SchoolImportResult.create!( + job_id: SecureRandom.uuid, + user_id: admin_user.id, + results: { + 'successful' => [ + { 'name' => 'Test School', 'code' => '12-34-56', 'id' => SecureRandom.uuid, 'owner_email' => 'owner@test.com' } + ], + 'failed' => [] + } + ) + end + + it 'returns successfully' do + get admin_school_import_result_path(import_result) + expect(response).to have_http_status(:success) + end + + it 'displays the job details' do + get admin_school_import_result_path(import_result) + expect(response.body).to include(import_result.job_id) + expect(response.body).to include('Test School') + end + + it 'displays user name and email' do + get admin_school_import_result_path(import_result) + expect(response.body).to include('Test Admin <admin@test.com>') + end + + it 'allows downloading results as CSV' do + get admin_school_import_result_path(import_result, format: :csv) + expect(response).to have_http_status(:success) + expect(response.content_type).to include('text/csv') + expect(response.body).to include('Status,School Name,School Code') + expect(response.body).to include('Success,Test School,12-34-56') + end + + context 'with failed schools' do + let(:import_result_with_failures) do + SchoolImportResult.create!( + job_id: SecureRandom.uuid, + user_id: admin_user.id, + results: { + 'successful' => [ + { 'name' => 'Good School', 'code' => '11-22-33', 'id' => SecureRandom.uuid, 'owner_email' => 'good@test.com' } + ], + 'failed' => [ + { 'name' => 'Bad School', 'owner_email' => 'bad@test.com', 'error_code' => 'OWNER_NOT_FOUND', 'error' => 'Owner not found' } + ] + } + ) + end + + it 'includes both successful and failed schools in CSV' do + get admin_school_import_result_path(import_result_with_failures, format: :csv) + expect(response.body).to include('Success,Good School,11-22-33') + expect(response.body).to include('Failed,Bad School') + expect(response.body).to include('OWNER_NOT_FOUND,Owner not found') + end + end + end + + describe 'POST /admin/school_import_results' do + let(:csv_content) { "name,website,address_line_1,municipality,country_code,owner_email\nTest,https://test.edu,123 Main,City,US,test@test.com" } + let(:csv_file) { Rack::Test::UploadedFile.new(StringIO.new(csv_content), 'text/csv', original_filename: 'test.csv') } + let(:job_id) { SecureRandom.uuid } + + before do + allow(School::ImportBatch).to receive(:call).and_return( + OperationResponse.new.tap do |response| + response[:job_id] = job_id + response[:total_schools] = 3 + end + ) + end + + it 'starts an import job' do + post admin_school_import_results_path, params: { csv_file: csv_file } + expect(School::ImportBatch).to have_received(:call) + expect(response).to redirect_to(admin_school_import_results_path) + expect(flash[:notice]).to include(job_id) + end + + context 'without a CSV file' do + it 'shows an error' do + post admin_school_import_results_path, params: {} + expect(response).to redirect_to(new_admin_school_import_result_path) + expect(flash[:error]).to include('CSV file is required') + end + end + + context 'when import fails' do + before do + allow(School::ImportBatch).to receive(:call).and_return( + OperationResponse.new.tap do |response| + response[:error] = { + message: 'CSV validation failed', + details: { + row_errors: [ + { row: 2, errors: [{ field: 'country_code', message: 'invalid code' }] } + ] + } + } + end + ) + end + + it 'shows an error message with details' do + post admin_school_import_results_path, params: { csv_file: csv_file } + expect(response).to have_http_status(:success) + expect(response.body).to include('CSV validation failed') + expect(response.body).to include('Row Errors') + end + end + end + + describe 'authorization' do + let(:non_admin_user) { create(:user, roles: nil) } + + before do + sign_in_as(non_admin_user) + end + + it 'redirects non-admin users' do + get admin_school_import_results_path + expect(response).to redirect_to('/') + end + end + + private + + def sign_in_as(user) + allow(User).to receive(:from_omniauth).and_return(user) + get '/auth/callback' + end +end diff --git a/spec/support/user_info_api_mock.rb b/spec/support/user_info_api_mock.rb new file mode 100644 index 000000000..6fff96cf2 --- /dev/null +++ b/spec/support/user_info_api_mock.rb @@ -0,0 +1,56 @@ +# frozen_string_literal: true + +module UserInfoApiMock + def stub_user_info_api_fetch_by_ids(user_ids:, users: []) + users = default_stubbed_users(user_ids) if users.empty? + + allow(UserInfoApiClient).to receive(:fetch_by_ids) + .with(user_ids) + .and_return(users) + end + + def stub_user_info_api_find_by_email(email:, user: :not_provided) + user = default_stubbed_user_by_email(email) if user == :not_provided + + allow(UserInfoApiClient).to receive(:find_user_by_email) + .with(email) + .and_return(user) + end + + private + + def default_stubbed_users(user_ids) + user_ids.map { |user_id| default_stubbed_user(id: user_id, email: "user-#{user_id}@example.com") } + end + + def default_stubbed_user_by_email(email) + default_stubbed_user( + id: Digest::UUID.uuid_v5(Digest::UUID::DNS_NAMESPACE, email), + email: email + ) + end + + def default_stubbed_user(id:, email:) + { + id: id, + email: email, + username: nil, + parental_email: nil, + name: 'School Owner', + nickname: 'Owner', + country: 'United Kingdom', + country_code: 'GB', + postcode: nil, + date_of_birth: nil, + verified_at: '2024-01-01T12:00:00.000Z', + created_at: '2024-01-01T12:00:00.000Z', + updated_at: '2024-01-01T12:00:00.000Z', + discarded_at: nil, + last_logged_in_at: '2024-01-01T12:00:00.000Z', + roles: '' + } + end + + # Class methods for use by UserInfoApiClient when BYPASS_OAUTH=true + module_function :default_stubbed_users, :default_stubbed_user_by_email, :default_stubbed_user +end