Show teacher names and emails on admin school classes - #1001
Conversation
|
@codex review |
There was a problem hiding this comment.
🟢 Approval recommended
The change is small and consistent with existing admin patterns, and the added request specs cover the key display and lookup behaviors.
Pull request overview
This PR updates the Administrate admin SchoolClass show page (/admin/school_classes/:id) to display each teacher using the same Name (email) formatting used elsewhere in admin (e.g., school roles), while batching user-info lookups and falling back to the teacher UUID when user info is missing.
Changes:
- Replaces the default
teachersfield on the SchoolClass dashboard with a customClassTeachersFieldand show partial to render teacher rows with name/email + timestamps. - Adds an admin controller helper to batch-fetch teacher user details via
User.from_userinfoand expose them to the field partial. - Adds request specs covering multiple teachers, missing users, and empty teacher lists.
File summaries
| File | Description |
|---|---|
| spec/features/admin/school_classes_spec.rb | Adds request specs asserting batched user lookup, Name (email) rendering, UUID fallback, and no lookup when empty. |
| app/views/fields/class_teachers_field/_show.html.erb | Renders the teachers table on the SchoolClass show page with Name (email) display and created/updated timestamps. |
| app/fields/class_teachers_field.rb | Implements the custom Administrate field and user display logic with UUID fallback when user info is missing. |
| app/dashboards/school_class_dashboard.rb | Switches teachers from Field::HasMany to ClassTeachersField. |
| app/controllers/admin/school_classes_controller.rb | Adds a helper for batched user-info lookup keyed by user ID for teacher display. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Test coverage93.57% line coverage reported by SimpleCov. |
There was a problem hiding this comment.
🟢 Approval recommended
The change is localized and low-risk with targeted coverage; only minor view/test robustness nits were identified.
Review details
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
app/views/fields/class_teachers_field/_show.html.erb:17
class_teacher_users_by_idis evaluated for every row in the loop. Even though it memoizes, repeatedly calling the helper in the view is unnecessary work and makes it less obvious it should only be fetched once per render. Compute it once before iterating and pass the local map touser_display.
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Lite
Note
Co-authored with ChatGPT GPT-6 Astro Light. Reviewed by Copilot Lite, /code-review-by-matt GPT-5.6
Show teacher names and email addresses on
/admin/school_classes/:id, using the sameName (email)format as school roles. Fetch user details in one batch and retain the teacher UUID when a user is missing.Validated with 24 passing admin request specs covering schools and school classes, including multiple teachers, missing users, and empty lists. RuboCop passes for all changed Ruby files. Browser demo uses the existing seeded Nursing 524 class and the built-in development auth/user-info mock.
Local demo screenshots
Existing seeded class and school, using the built-in development auth/user-info mock.
School class — teacher name and email
School page — matching existing display