Support multiple teachers per class

app/controllers/api/class_members_controller.rb

@@ -5,10 +5,10 @@ class ClassMembersController < ApiController
     before_action :authorize_user
     load_and_authorize_resource :school
     load_and_authorize_resource :school_class, through: :school, through_association: :classes, id_param: :class_id
-    load_and_authorize_resource :class_member, through: :school_class, through_association: :members
+    load_and_authorize_resource :class_student, through: :school_class, through_association: :students
 
     def index
-      @class_members = @school_class.members.accessible_by(current_ability)
+      @class_members = @school_class.students.accessible_by(current_ability)
       result = ClassMember::List.call(school_class: @school_class, class_members: @class_members, token: current_user.token)
 
       if result.success?

app/controllers/api/school_classes_controller.rb

@@ -12,15 +12,15 @@ def index
     end
 
     def show
-      @school_class_with_teacher = @school_class.with_teacher
+      @school_class_with_teachers = @school_class.with_teachers
       render :show, formats: [:json], status: :ok
     end
 
     def create
       result = SchoolClass::Create.call(school: @school, school_class_params:)
 
       if result.success?
-        @school_class_with_teacher = result[:school_class].with_teacher
+        @school_class_with_teachers = result[:school_class].with_teachers
         render :show, formats: [:json], status: :created
       else
         render json: { error: result[:error] }, status: :unprocessable_entity
@@ -32,7 +32,7 @@ def update
       result = SchoolClass::Update.call(school_class:, school_class_params:)
 
       if result.success?
-        @school_class_with_teacher = result[:school_class].with_teacher
+        @school_class_with_teachers = result[:school_class].with_teachers
         render :show, formats: [:json], status: :ok
       else
         render json: { error: result[:error] }, status: :unprocessable_entity

app/models/ability.rb

@@ -62,7 +62,8 @@ def define_school_owner_abilities(school:)
     can(%i[read], :school_member)
     can(%i[read create update destroy], SchoolClass, school: { id: school.id })
     can(%i[read], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
-    can(%i[read create create_batch destroy], ClassMember, school_class: { school: { id: school.id } })
+    can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id } })
+    can(%i[read create create_batch destroy], ClassTeacher, school_class: { school: { id: school.id } })
     can(%i[read create destroy], :school_owner)
     can(%i[read create destroy], :school_teacher)
     can(%i[read create create_batch update destroy], :school_student)
@@ -74,9 +75,9 @@ def define_school_teacher_abilities(user:, school:)
     can(%i[read], School, id: school.id)
     can(%i[read], :school_member)
     can(%i[create], SchoolClass, school: { id: school.id })
-    can(%i[read update destroy], SchoolClass, school: { id: school.id }, teacher_id: user.id)
-    can(%i[read create create_batch destroy], ClassMember,
-        school_class: { school: { id: school.id }, teacher_id: user.id })
+    can(%i[read update destroy], SchoolClass, school: { id: school.id }, class_teachers: { teacher_id: user.id })
+    can(%i[read create create_batch destroy], ClassStudent, school_class: { school: { id: school.id }, class_teachers: { teacher_id: user.id } })
+    can(%i[read create create_batch destroy], ClassTeacher, school_class: { school: { id: school.id }, class_teachers: { teacher_id: user.id } })
     can(%i[read], :school_owner)
     can(%i[read], :school_teacher)
     can(%i[read create create_batch update], :school_student)
@@ -89,16 +90,16 @@ def define_school_teacher_abilities(user:, school:)
     end
     can(%i[read update], Project, school_id: school.id, lesson: { visibility: %w[teachers students] })
     can(%i[read], Project,
-        remixed_from_id: Project.where(user_id: user.id, school_id: school.id, remixed_from_id: nil).pluck(:id))
+        remixed_from_id: Project.where(school_id: school.id, remixed_from_id: nil, lesson_id: Lesson.where(school_class_id: ClassTeacher.where(teacher_id: user.id).select(:school_class_id))).pluck(:id))
   end
 
   def define_school_student_abilities(user:, school:)
     can(%i[read], School, id: school.id)
-    can(%i[read], SchoolClass, school: { id: school.id }, members: { student_id: user.id })
+    can(%i[read], SchoolClass, school: { id: school.id }, students: { student_id: user.id })
     # Ensure no access to ClassMember resources, relationships otherwise allow access in some circumstances.
-    can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { members: { student_id: user.id } })
+    can(%i[read], Lesson, school_id: school.id, visibility: 'students', school_class: { students: { student_id: user.id } })
     can(%i[read create update], Project, school_id: school.id, user_id: user.id, lesson_id: nil)
-    can(%i[read], Project, lesson: { school_id: school.id, school_class: { members: { student_id: user.id } } })
+    can(%i[read], Project, lesson: { school_id: school.id, school_class: { students: { student_id: user.id } } })
     can(%i[toggle_finished], Project) do |project|
       school_student_can_toggle_finished?(user:, school:, project:)
     end

app/models/class_member.rb → app/models/class_student.rb

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-class ClassMember < ApplicationRecord
+class ClassStudent < ApplicationRecord
   belongs_to :school_class
   delegate :school, to: :school_class
   attr_accessor :student

app/models/class_teacher.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+class ClassTeacher < ApplicationRecord
+  belongs_to :school_class
+  delegate :school, to: :school_class
+  attr_accessor :teacher
+
+  validates :teacher_id, presence: true, uniqueness: {
+    scope: :school_class_id,
+    case_sensitive: false
+  }
+
+  validate :teacher_has_the_school_teacher_role_for_the_school
+
+  has_paper_trail(
+    meta: {
+      meta_school_id: ->(cm) { cm.school_class&.school_id }
+    }
+  )
+
+  private
+
+  def teacher_has_the_school_teacher_role_for_the_school
+    return unless teacher_id_changed? && errors.blank? && teacher.present?
+
+    return if teacher.school_teacher?(school)
+
+    msg = "'#{teacher.id}' does not have the 'school-teacher' role for organisation '#{school.id}'"
+    errors.add(:teacher, msg)
+  end
+end

app/models/project.rb

@@ -80,7 +80,7 @@ def user_has_a_role_within_the_school
   end
 
   def user_is_a_member_or_the_owner_of_the_lesson
-    return if !lesson || user_id == lesson.user_id || lesson.school_class.members.exists?(student_id: user_id)
+    return if !lesson || user_id == lesson.user_id || lesson.school_class.students.exists?(student_id: user_id)
 
     errors.add(:user, "'#{user_id}' is not the owner or a member of the lesson '#{lesson_id}'")
   end

app/models/school_class.rb

@@ -2,35 +2,43 @@
 
 class SchoolClass < ApplicationRecord
   belongs_to :school
-  has_many :members, class_name: :ClassMember, inverse_of: :school_class, dependent: :destroy
+  has_many :students, class_name: :ClassStudent, inverse_of: :school_class, dependent: :destroy
+  has_many :class_teachers, class_name: :ClassTeacher, inverse_of: :school_class, dependent: :destroy
   has_many :lessons, dependent: :nullify
+  accepts_nested_attributes_for :class_teachers
 
-  validates :teacher_id, presence: true
+  scope :with_class_teacher, ->(user_id) { joins(:class_teachers).where(class_teachers: { id: user_id }) }
+
+  # validates :teacher_id, presence: true
   validates :name, presence: true
-  validate :teacher_has_the_school_teacher_role_for_the_school
+  # validate :teacher_has_the_school_teacher_role_for_the_school
 
   def self.teachers
-    User.from_userinfo(ids: pluck(:teacher_id))
+    User.from_userinfo(ids: ClassTeacher.pluck(:teacher_id))
   end
 
   def self.with_teachers
     by_id = teachers.index_by(&:id)
-    all.map { |instance| [instance, by_id[instance.teacher_id]] }
+    all.map { |instance| [instance, instance.teacher_ids.map { |teacher_id| by_id[teacher_id] }] }
+  end
+
+  def teacher_ids
+    class_teachers.pluck(:teacher_id)
   end
 
-  def with_teacher
-    [self, User.from_userinfo(ids: teacher_id).first]
+  def with_teachers
+    [self, User.from_userinfo(ids: teacher_ids)]
   end
 
-  private
+  # private
 
-  def teacher_has_the_school_teacher_role_for_the_school
-    return unless teacher_id_changed? && errors.blank?
+  # def teacher_has_the_school_teacher_role_for_the_school
+  #   return unless teacher_id_changed? && errors.blank?
 
-    user = User.new(id: teacher_id)
-    return if user.school_teacher?(school)
+  #   user = User.new(id: teacher_id)
+  #   return if user.school_teacher?(school)
 
-    msg = "'#{teacher_id}' does not have the 'school-teacher' role for organisation '#{school.id}'"
-    errors.add(:user, msg)
-  end
+  #   msg = "'#{teacher_id}' does not have the 'school-teacher' role for organisation '#{school.id}'"
+  #   errors.add(:user, msg)
+  # end
 end

app/views/api/school_classes/index.json.jbuilder

@@ -1,16 +1,21 @@
 # frozen_string_literal: true
 
-json.array!(@school_classes_with_teachers) do |school_class, teacher|
+json.array!(@school_classes_with_teachers) do |school_class, teachers|
   json.call(
     school_class,
     :id,
     :description,
     :school_id,
-    :teacher_id,
     :name,
     :created_at,
     :updated_at
   )
 
-  json.teacher_name(teacher&.name)
+  json.teachers(teachers) do |teacher|
+    json.call(
+      teacher,
+      :id,
+      :name
+    )
+  end
 end

app/views/api/school_classes/show.json.jbuilder

@@ -1,16 +1,21 @@
 # frozen_string_literal: true
 
-school_class, teacher = @school_class_with_teacher
+school_class, teachers = @school_class_with_teachers
 
 json.call(
   school_class,
   :id,
   :description,
   :school_id,
-  :teacher_id,
   :name,
   :created_at,
   :updated_at
 )
 
-json.teacher_name(teacher&.name)
+json.teachers(teachers) do |teacher|
+  json.call(
+    teacher,
+    :id,
+    :name
+  )
+end

db/migrate/20241101125219_create_class_teachers.rb

@@ -0,0 +1,12 @@
+class CreateClassTeachers < ActiveRecord::Migration[7.1]
+  def change
+    create_table :class_teachers, id: :uuid do |t|
+      t.references :school_class, type: :uuid, foreign_key: true, index: true, null: false
+      t.uuid :teacher_id, null: false
+      t.timestamps
+    end
+
+    add_index :class_teachers, :teacher_id
+    add_index :class_teachers, %i[school_class_id teacher_id], unique: true
+  end
+end

db/migrate/20241101125505_move_teacher_data_to_class_teachers_table.rb

@@ -0,0 +1,11 @@
+class MoveTeacherDataToClassTeachersTable < ActiveRecord::Migration[7.1]
+  def up
+    SchoolClass.find_each do |school_class|
+      ClassTeacher.create!(school_class: school_class, teacher_id: school_class.teacher_id)
+    end
+  end
+
+  def down
+    ClassTeacher.destroy_all
+  end
+end

db/migrate/20241101125814_remove_class_teacher_id.rb

@@ -0,0 +1,14 @@
+class RemoveClassTeacherId < ActiveRecord::Migration[7.1]
+  def up
+    remove_column :school_classes, :teacher_id
+  end
+
+  def down
+    add_column :school_classes, :teacher_id, :uuid
+    ClassTeacher.find_each do |class_teacher|
+      school_class = class_teacher.school_class
+      school_class.update!(teacher_id: class_teacher.teacher_id)
+    end
+    change_column_null :school_classes, :teacher_id, false
+  end
+end

db/migrate/20241101142545_rename_class_members_to_class_students.rb

@@ -0,0 +1,9 @@
+class RenameClassMembersToClassStudents < ActiveRecord::Migration[7.1]
+  def up
+    rename_table :class_members, :class_students
+  end
+
+  def down
+    rename_table :class_students, :class_members
+  end
+end

lib/concepts/class_member/operations/create.rb

@@ -1,9 +1,12 @@
 # frozen_string_literal: true
 
-class ClassMember
+module ClassMember
   class Create
     class << self
       def call(school_class:, students:)
+        # puts 'Creating class members...'
+        # pp students
+        # put students.first.student?
         response = OperationResponse.new
         response[:class_members] = []
         response[:errors] = {}
@@ -21,12 +24,12 @@ def call(school_class:, students:)
 
       def create_class_members(school_class:, students:, response:)
         students.each do |student|
-          class_member = school_class.members.build({ student_id: student.id })
-          class_member.student = student
-          class_member.save!
-          response[:class_members] << class_member
+          class_student = school_class.students.build({ student_id: student.id })
+          class_student.student = student
+          class_student.save!
+          response[:class_members] << class_student
         rescue StandardError => e
-          handle_class_member_error(e, class_member, student, response)
+          handle_class_member_error(e, class_student, student, response)
           response
         end
       end