Update class members endpoint to include teachers (#426)

closes #424

---------

Co-authored-by: create-issue-branch[bot] <53036503+create-issue-branch[bot]@users.noreply.github.com>
Co-authored-by: Dan Halson <[email protected]>
Co-authored-by: Dan Halson <[email protected]>

.rubocop.yml

@@ -30,9 +30,17 @@ RSpec/DescribeClass:
 RSpec/MultipleMemoizedHelpers:
   Max: 8
 
+RSpec/ExampleLength:
+  Enabled: false
+
 Metrics/BlockLength:
-  Exclude:
-    - "config/routes.rb"
+  Enabled: false
+
+Metrics/AbcSize:
+  Enabled: false
+
+Metrics/LineLength:
+  Enabled: false
 
 Naming/VariableNumber:
   EnforcedStyle: snake_case

app/controllers/api/class_members_controller.rb

@@ -9,12 +9,10 @@ class ClassMembersController < ApiController
 
     def index
       @class_members = @school_class.members.accessible_by(current_ability)
-      student_ids = @class_members.pluck(:student_id)
-
-      result = SchoolStudent::List.call(school: @school, token: current_user.token, student_ids:)
+      result = ClassMember::List.call(school_class: @school_class, class_members: @class_members, token: current_user.token)
 
       if result.success?
-        @school_students = result[:school_students]
+        @class_members = result[:class_members]
         render :index, formats: [:json], status: :ok
       else
         render json: { error: result[:error] }, status: :unprocessable_entity

app/controllers/api/school_teachers_controller.rb

@@ -7,7 +7,8 @@ class SchoolTeachersController < ApiController
     authorize_resource :school_teacher, class: false
 
     def index
-      result = SchoolTeacher::List.call(school: @school, token: current_user.token)
+      teacher_ids = @school.roles.where(role: :teacher).pluck(:user_id)
+      result = SchoolTeacher::List.call(teacher_ids:)
 
       if result.success?
         @school_teachers = result[:school_teachers]

app/graphql/mutations/create_component.rb

@@ -26,9 +26,7 @@ def resolve(**input)
     end
 
     def ready?(**_args)
-      if context[:current_ability]&.can?(:create, Component, Project.new(user_id: context[:current_user]&.id))
-        return true
-      end
+      return true if context[:current_ability]&.can?(:create, Component, Project.new(user_id: context[:current_user]&.id))
 
       raise GraphQL::ExecutionError, 'You are not permitted to create a component'
     end

app/graphql/mutations/delete_project.rb

@@ -13,9 +13,7 @@ def resolve(**input)
 
       raise GraphQL::ExecutionError, 'Project not found' unless project
 
-      unless context[:current_ability].can?(:destroy, project)
-        raise GraphQL::ExecutionError, 'You are not permitted to delete that project'
-      end
+      raise GraphQL::ExecutionError, 'You are not permitted to delete that project' unless context[:current_ability].can?(:destroy, project)
 
       return { id: project.id } if project.destroy
 

app/graphql/mutations/remix_project.rb

@@ -7,7 +7,6 @@ class RemixProject < BaseMutation
 
     field :project, Types::ProjectType, description: 'The project that has been created'
 
-    # rubocop:disable Metrics/AbcSize
     def resolve(**input)
       original_project = GlobalID.find(input[:id])
       raise GraphQL::ExecutionError, 'Project not found' unless original_project
@@ -30,7 +29,6 @@ def resolve(**input)
 
       { project: response[:project] }
     end
-    # rubocop:enable Metrics/AbcSize
 
     def ready?(**_args)
       return true if can_create_project?

app/models/ability.rb

@@ -3,7 +3,6 @@
 class Ability
   include CanCan::Ability
 
-  # rubocop:disable Metrics/AbcSize
   def initialize(user)
     # Anyone can view projects not owner by a user or a school.
     can :show, Project, user_id: nil, school_id: nil
@@ -43,7 +42,6 @@ def initialize(user)
       define_school_owner_abilities(school:) if user.school_owner?(school)
     end
   end
-  # rubocop:enable Metrics/AbcSize
 
   private
 
@@ -60,7 +58,6 @@ def define_school_owner_abilities(school:)
     can(%i[create], Project, school_id: school.id)
   end
 
-  # rubocop:disable Metrics/AbcSize
   def define_school_teacher_abilities(user:, school:)
     can(%i[read], School, id: school.id)
     can(%i[create], SchoolClass, school: { id: school.id })
@@ -81,9 +78,7 @@ def define_school_teacher_abilities(user:, school:)
     can(%i[read], Project,
         remixed_from_id: Project.where(user_id: user.id, school_id: school.id, remixed_from_id: nil).pluck(:id))
   end
-  # rubocop:enable Metrics/AbcSize
 
-  # rubocop:disable Layout/LineLength
   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 })
@@ -94,7 +89,6 @@ def define_school_student_abilities(user:, school:)
       school_student_can_toggle_finished?(user:, school:, project:)
     end
   end
-  # rubocop:enable Layout/LineLength
 
   def school_student_can_toggle_finished?(user:, school:, project:)
     is_my_project = project.user_id == user.id && project.school_id == school.id

app/models/filesystem_project.rb

@@ -7,7 +7,7 @@ class FilesystemProject
   IMAGE_FORMATS = ['.png', '.jpg', '.jpeg', '.webp'].freeze
   PROJECTS_ROOT = Rails.root.join('lib/tasks/project_components')
 
-  def self.import_all! # rubocop:disable Metrics/AbcSize
+  def self.import_all!
     PROJECTS_ROOT.each_child do |dir|
       proj_config = YAML.safe_load_file(dir.join('project_config.yml').to_s)
       files = dir.children

app/services/school_verification_service.rb

@@ -7,7 +7,6 @@ def initialize(school)
     @school = school
   end
 
-  # rubocop:disable Metrics/AbcSize
   def verify(token:)
     School.transaction do
       school.verify!
@@ -22,7 +21,6 @@ def verify(token:)
   else
     true
   end
-  # rubocop:enable Metrics/AbcSize
 
   delegate :reject, to: :school
 end

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

@@ -1,24 +1,16 @@
 # frozen_string_literal: true
 
-json.array!(@class_members, @school_students) do |class_member|
-  json.call(
-    class_member,
-    :id,
-    :school_class_id,
-    :student_id,
-    :created_at,
-    :updated_at
-  )
-
-  school_student = @school_students.find { |student| student.id == class_member.student_id }
-
-  if school_student.present?
-    json.set! :student do
+json.array!(@class_members) do |class_member|
+  if class_member.respond_to?(:student_id)
+    json.partial! 'class_member', class_member:
+  else
+    # Teachers are not modelled as ClassMembers
+    json.set! :teacher do
       json.call(
-        school_student,
+        class_member,
         :id,
-        :username,
-        :name
+        :name,
+        :email
       )
     end
   end

config/routes.rb

@@ -1,6 +1,5 @@
 # frozen_string_literal: true
 
-# rubocop:disable Metrics/BlockLength
 Rails.application.routes.draw do
   namespace :admin do
     mount GoodJob::Engine => 'good_job'
@@ -73,4 +72,3 @@
   get '/auth/callback', to: 'auth#callback', as: 'callback'
   get '/logout', to: 'auth#destroy', as: 'logout'
 end
-# rubocop:enable Metrics/BlockLength

lib/concepts/class_member/operations/list.rb

@@ -0,0 +1,34 @@
+# frozen_string_literal: true
+
+class ClassMember
+  class List
+    class << self
+      def call(school_class:, class_members:, token:)
+        response = OperationResponse.new
+        response[:class_members] = []
+
+        begin
+          school = school_class.school
+          student_ids = class_members.pluck(:student_id)
+          students = SchoolStudent::List.call(school:, token:, student_ids:).fetch(:school_students, [])
+          class_members.each do |member|
+            member.student = students.find { |student| student.id == member.student_id }
+          end
+
+          teacher_ids = [school_class.teacher_id]
+          teachers = SchoolTeacher::List.call(teacher_ids:).fetch(:school_teachers, [])
+        rescue StandardError => e
+          Sentry.capture_exception(e)
+          response[:error] = "Error listing class members: #{e}"
+          return response
+        end
+
+        response[:class_members] = teachers + class_members.sort do |a, b|
+          a.student.name <=> b.student.name
+        end
+
+        response
+      end
+    end
+  end
+end

lib/concepts/school_teacher/list.rb

@@ -3,9 +3,9 @@
 module SchoolTeacher
   class List
     class << self
-      def call(school:, token:)
+      def call(teacher_ids:)
         response = OperationResponse.new
-        response[:school_teachers] = list_teachers(school, token)
+        response[:school_teachers] = list_teachers(teacher_ids)
         response
       rescue StandardError => e
         Sentry.capture_exception(e)
@@ -15,11 +15,8 @@ def call(school:, token:)
 
       private
 
-      def list_teachers(school, token)
-        response = ProfileApiClient.list_school_teachers(token:, organisation_id: school.id)
-        user_ids = response.fetch(:ids)
-
-        User.from_userinfo(ids: user_ids)
+      def list_teachers(ids)
+        User.from_userinfo(ids:)
       end
     end
   end

lib/concepts/school_teacher/remove.rb

@@ -16,6 +16,7 @@ def call(school:, teacher_id:, token:)
       private
 
       def remove_teacher(school, teacher_id, token)
+        # TODO: This has not been implemented yet
         ProfileApiClient.remove_school_teacher(token:, teacher_id:, organisation_id: school.id)
       end
     end

lib/profile_api_client.rb

@@ -71,10 +71,6 @@ def remove_school_owner(*)
       {}
     end
 
-    def list_school_teachers(*)
-      {}
-    end
-
     def remove_school_teacher(*)
       {}
     end
@@ -117,7 +113,6 @@ def create_school_student(token:, username:, password:, name:, school_id:)
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
 
-    # rubocop:disable Metrics/AbcSize
     def update_school_student(token:, school_id:, student_id:, name: nil, username: nil, password: nil) # rubocop:disable Metrics/ParameterLists
       return nil if token.blank?
 
@@ -135,7 +130,6 @@ def update_school_student(token:, school_id:, student_id:, name: nil, username:
     rescue Faraday::UnprocessableEntityError => e
       raise Student422Error, JSON.parse(e.response_body)['errors'].first
     end
-    # rubocop:enable Metrics/AbcSize
 
     def delete_school_student(token:, school_id:, student_id:)
       return nil if token.blank?

lib/tasks/classroom_management.rake

@@ -10,7 +10,6 @@ Rails.logger = Logger.new($stdout) unless Rails.env.test?
 
 # For students to match up the school needs to match with the school defined in profile (hard coded in the helper)
 
-# rubocop:disable Metrics/BlockLength
 namespace :classroom_management do
   include ClassroomManagementHelper
 
@@ -117,4 +116,3 @@ namespace :classroom_management do
     end
   end
 end
-# rubocop:enable Metrics/BlockLength

lib/tasks/classroom_management_helper.rb

@@ -72,7 +72,6 @@ def assign_students(school_class, school)
     end
   end
 
-  # rubocop:disable Metrics/AbcSize
   def create_lessons(user_id, school, school_class, visibility = 'students')
     2.times.map do |i|
       Lesson.find_or_create_by!(school:, school_class:,
@@ -87,7 +86,6 @@ def create_lessons(user_id, school, school_class, visibility = 'students')
       end
     end
   end
-  # rubocop:enable Metrics/AbcSize
 
   def create_project(user_id, school, lesson)
     Project.find_or_create_by!(user_id:, school:, lesson:) do |project|