From 773cdd32712d5fd66dcdf4d24db0568a84fa9828 Mon Sep 17 00:00:00 2001 From: Adrian Lansdown Date: Mon, 7 Sep 2026 12:40:37 +0100 Subject: [PATCH 1/3] Show teacher names and emails on admin school classes --- .../admin/school_classes_controller.rb | 7 +++ app/dashboards/school_class_dashboard.rb | 2 +- app/fields/class_teachers_field.rb | 20 +++++++++ .../class_teachers_field/_show.html.erb | 22 ++++++++++ spec/features/admin/school_classes_spec.rb | 44 +++++++++++++++++++ 5 files changed, 94 insertions(+), 1 deletion(-) create mode 100644 app/fields/class_teachers_field.rb create mode 100644 app/views/fields/class_teachers_field/_show.html.erb create mode 100644 spec/features/admin/school_classes_spec.rb diff --git a/app/controllers/admin/school_classes_controller.rb b/app/controllers/admin/school_classes_controller.rb index c6ec2043b..6ddc19b96 100644 --- a/app/controllers/admin/school_classes_controller.rb +++ b/app/controllers/admin/school_classes_controller.rb @@ -2,5 +2,12 @@ module Admin class SchoolClassesController < Admin::ApplicationController + helper_method :class_teacher_users_by_id + + private + + def class_teacher_users_by_id + @class_teacher_users_by_id ||= User.from_userinfo(ids: requested_resource.teacher_ids).index_by(&:id) + end end end diff --git a/app/dashboards/school_class_dashboard.rb b/app/dashboards/school_class_dashboard.rb index 3188f5c59..c90530711 100644 --- a/app/dashboards/school_class_dashboard.rb +++ b/app/dashboards/school_class_dashboard.rb @@ -11,7 +11,7 @@ class SchoolClassDashboard < Administrate::BaseDashboard # on pages throughout the dashboard. ATTRIBUTE_TYPES = { school: Field::BelongsTo, - teachers: Field::HasMany, + teachers: ClassTeachersField, students: Field::HasMany, lessons: Field::HasMany, id: Field::String, diff --git a/app/fields/class_teachers_field.rb b/app/fields/class_teachers_field.rb new file mode 100644 index 000000000..84ab364d5 --- /dev/null +++ b/app/fields/class_teachers_field.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true + +require 'administrate/field/base' + +class ClassTeachersField < Administrate::Field::Base + def teachers + @teachers ||= data.sort_by(&:created_at) + end + + def user_display(teacher, users_by_id) + user = users_by_id[teacher.teacher_id] + user.present? ? user_dashboard.display_resource(user) : teacher.teacher_id + end + + private + + def user_dashboard + @user_dashboard ||= UserDashboard.new + end +end diff --git a/app/views/fields/class_teachers_field/_show.html.erb b/app/views/fields/class_teachers_field/_show.html.erb new file mode 100644 index 000000000..f46200839 --- /dev/null +++ b/app/views/fields/class_teachers_field/_show.html.erb @@ -0,0 +1,22 @@ +<% if field.teachers.any? %> + + + + + + + + + + <% field.teachers.each do |teacher| %> + + + + + + <% end %> + +
TeacherCreatedUpdated
<%= field.user_display(teacher, class_teacher_users_by_id) %><%= l(teacher.created_at) %><%= l(teacher.updated_at) %>
+<% else %> + <%= t('administrate.fields.has_many.none', default: '–') %> +<% end %> diff --git a/spec/features/admin/school_classes_spec.rb b/spec/features/admin/school_classes_spec.rb new file mode 100644 index 000000000..90a23aac7 --- /dev/null +++ b/spec/features/admin/school_classes_spec.rb @@ -0,0 +1,44 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe 'Admin school classes', type: :request do + let(:admin_user) { create(:admin_user) } + let(:school) { create(:school) } + let(:teacher) { create(:user, name: 'Tariq Teacher', email: 'teacher@example.com') } + let(:other_teacher) { create(:user, name: 'Olivia Teacher', email: 'olivia@example.com') } + let(:school_class) { create(:school_class, school:, teacher_ids: [teacher.id, other_teacher.id]) } + + before do + allow(User).to receive(:from_omniauth).and_return(admin_user) + get '/auth/callback' + allow(User).to receive(:from_userinfo).with(ids: contain_exactly(teacher.id, other_teacher.id)).and_return([other_teacher, teacher]) + end + + it 'displays each teacher name and email using a single batch lookup' do + get admin_school_class_path(school_class) + + expect(response).to have_http_status(:success) + expect(response.body).to include('Tariq Teacher (teacher@example.com)', 'Olivia Teacher (olivia@example.com)') + expect(response.body).not_to include(teacher.id, other_teacher.id) + expect(User).to have_received(:from_userinfo).with(ids: contain_exactly(teacher.id, other_teacher.id)).once + end + + it 'falls back to the UUID when a teacher is missing from user info' do + allow(User).to receive(:from_userinfo).and_return([teacher]) + + get admin_school_class_path(school_class) + + expect(response).to have_http_status(:success) + expect(response.body).to include('Tariq Teacher (teacher@example.com)', other_teacher.id) + end + + it 'renders an empty teacher list without looking up users' do + school_class.teachers.destroy_all + + get admin_school_class_path(school_class) + + expect(response).to have_http_status(:success) + expect(User).not_to have_received(:from_userinfo) + end +end From dc097d70310f5d7f960cce1167c4e40c65501f49 Mon Sep 17 00:00:00 2001 From: Adrian Lansdown Date: Mon, 7 Sep 2026 12:45:55 +0100 Subject: [PATCH 2/3] Align teacher lookup with school role display --- app/controllers/admin/school_classes_controller.rb | 2 +- app/fields/class_teachers_field.rb | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/app/controllers/admin/school_classes_controller.rb b/app/controllers/admin/school_classes_controller.rb index 6ddc19b96..ac9c490fe 100644 --- a/app/controllers/admin/school_classes_controller.rb +++ b/app/controllers/admin/school_classes_controller.rb @@ -7,7 +7,7 @@ class SchoolClassesController < Admin::ApplicationController private def class_teacher_users_by_id - @class_teacher_users_by_id ||= User.from_userinfo(ids: requested_resource.teacher_ids).index_by(&:id) + @class_teacher_users_by_id ||= User.from_userinfo(ids: requested_resource.teachers.map(&:user_id).uniq).index_by(&:id) end end end diff --git a/app/fields/class_teachers_field.rb b/app/fields/class_teachers_field.rb index 84ab364d5..c5f09d9e5 100644 --- a/app/fields/class_teachers_field.rb +++ b/app/fields/class_teachers_field.rb @@ -7,9 +7,9 @@ def teachers @teachers ||= data.sort_by(&:created_at) end - def user_display(teacher, users_by_id) - user = users_by_id[teacher.teacher_id] - user.present? ? user_dashboard.display_resource(user) : teacher.teacher_id + def user_display(teacher, users_by_id = {}) + user = users_by_id[teacher.user_id] + user.present? ? user_dashboard.display_resource(user) : teacher.user_id end private From 5b96a06a0b73ffb4a7b01d38d02f0fb0660fbf64 Mon Sep 17 00:00:00 2001 From: Adrian Lansdown Date: Mon, 7 Sep 2026 17:01:35 +0100 Subject: [PATCH 3/3] Clarify class teacher timestamp --- app/views/fields/class_teachers_field/_show.html.erb | 4 +--- spec/features/admin/school_classes_spec.rb | 2 ++ 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/app/views/fields/class_teachers_field/_show.html.erb b/app/views/fields/class_teachers_field/_show.html.erb index f46200839..5159ebec9 100644 --- a/app/views/fields/class_teachers_field/_show.html.erb +++ b/app/views/fields/class_teachers_field/_show.html.erb @@ -3,8 +3,7 @@ Teacher - Created - Updated + Added to class @@ -12,7 +11,6 @@ <%= field.user_display(teacher, class_teacher_users_by_id) %> <%= l(teacher.created_at) %> - <%= l(teacher.updated_at) %> <% end %> diff --git a/spec/features/admin/school_classes_spec.rb b/spec/features/admin/school_classes_spec.rb index 90a23aac7..dd58b6df7 100644 --- a/spec/features/admin/school_classes_spec.rb +++ b/spec/features/admin/school_classes_spec.rb @@ -20,6 +20,8 @@ expect(response).to have_http_status(:success) expect(response.body).to include('Tariq Teacher (teacher@example.com)', 'Olivia Teacher (olivia@example.com)') + expect(response.body).to include('Added to class') + expect(response.body).not_to include('Updated') expect(response.body).not_to include(teacher.id, other_teacher.id) expect(User).to have_received(:from_userinfo).with(ids: contain_exactly(teacher.id, other_teacher.id)).once end