Fix nil school id bug

app/controllers/api/projects/remixes_controller.rb

@@ -9,8 +9,8 @@ class RemixesController < ApiController
 
       def index
         projects = Project.where(remixed_from_id: project.id).accessible_by(current_ability)
-        @projects_with_users = projects.includes(:school_project).with_users(current_user)
-        render index: @projects_with_users, formats: [:json]
+        @projects_with_students = projects.includes(:school_project).with_students(school, current_user)
+        render index: @projects_with_students, formats: [:json]
       end
 
       def show

app/controllers/api/projects_controller.rb

@@ -18,7 +18,7 @@ def index
 
     def show
       if !@project.school_id.nil? && @project.lesson_id.nil?
-        project_with_user = @project.with_user(current_user)
+        project_with_user = @project.with_student(current_user)
         @user = project_with_user[1]
       end
 

app/models/project.rb

@@ -42,21 +42,19 @@ module Types
     }
   )
 
-  def self.users(current_user)
-    school = School.find_by(id: pluck(:school_id))
+  def self.students(school, current_user)
     SchoolStudent::List.call(school:, token: current_user.token, student_ids: pluck(:user_id).uniq)[:school_students] || []
   end
 
-  def self.with_users(current_user)
-    by_id = users(current_user).index_by(&:id)
+  def self.with_students(school, current_user)
+    by_id = students(school, current_user).index_by(&:id)
     all.map { |instance| [instance, by_id[instance.user_id]] }
   end
 
-  def with_user(current_user)
+  def with_student(current_user)
     # students cannot call the Profile API so do not even try
     return [self, nil] if current_user&.student?
 
-    school = School.find_by(id: school_id)
     students = SchoolStudent::List.call(school:, token: current_user.token,
                                         student_ids: [user_id])[:school_students] || []
     [self, students.first]

app/views/api/projects/remixes/index.json.jbuilder

@@ -1,6 +1,6 @@
 # frozen_string_literal: true
 
-json.array!(@projects_with_users) do |project, user|
+json.array!(@projects_with_students) do |project, user|
   json.call(
     project,
     :identifier,

lib/concepts/school_student/list.rb

@@ -7,7 +7,7 @@ def call(school:, token:, student_ids: nil)
         response = OperationResponse.new
         response[:school_students] = list_students(school, token, student_ids)
         response
-      rescue StandardError => e
+      rescue Faraday::Error => e
         Sentry.capture_exception(e)
         response[:error] = "Error listing school students: #{e}"
         response
@@ -17,6 +17,8 @@ def call(school:, token:, student_ids: nil)
 
       def list_students(school, token, student_ids)
         student_ids ||= Role.student.where(school:).map(&:user_id)
+        return [] if student_ids.empty?
+
         students = ProfileApiClient.list_school_students(token:, school_id: school.id, student_ids:)
         students.map do |student|
           User.new(student.to_h.slice(:id, :username, :name, :email).merge(sso_providers: student.ssoProviders))

spec/concepts/school_student/list_spec.rb

@@ -84,13 +84,19 @@
         expect(response[:school_students]).to include(expected_user)
       end
     end
+
+    it 'skips the API call if student_ids is empty' do
+      response = described_class.call(school:, token:, student_ids: [])
+      expect(ProfileApiClient).not_to have_received(:list_school_students)
+      expect(response[:school_students].size).to eq(0)
+    end
   end
 
-  context 'when listing fails' do
+  context 'when listing fails due to a Faraday error' do
     let(:student_ids) { [123] }
 
     before do
-      allow(ProfileApiClient).to receive(:list_school_students).and_raise('Some API error')
+      allow(ProfileApiClient).to receive(:list_school_students).and_raise(Faraday::Error.new('Some API error'))
       allow(Sentry).to receive(:capture_exception)
     end
 
@@ -106,7 +112,7 @@
 
     it 'sent the exception to Sentry' do
       described_class.call(school:, token:, student_ids:)
-      expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError))
+      expect(Sentry).to have_received(:capture_exception).with(kind_of(Faraday::Error))
     end
   end
 end

spec/models/project_spec.rb

@@ -182,7 +182,7 @@
     end
   end
 
-  describe '.users' do
+  describe '.students' do
     let(:student) { create(:student, school:, name: 'School Student') }
     let(:teacher) { create(:teacher, school:) }
 
@@ -195,28 +195,28 @@
     end
 
     it 'returns User instances for the current scope' do
-      create(:project, user_id: student.id, school_id: school.id)
-      user = described_class.all.users(teacher).first
+      create(:project, school:, user_id: student.id)
+      user = described_class.all.students(school, teacher).first
       expect(user.name).to eq('School Student')
     end
 
     it 'ignores members where no profile account exists' do
-      user_id = SecureRandom.uuid
-      create(:project, user_id:)
+      stub_profile_api_list_school_students(school:, student_attributes: [])
+      create(:project, school:, user_id: student.id)
 
-      user = described_class.all.users(teacher).first
-      expect(user).to be_nil
+      student = described_class.all.students(school, teacher).first
+      expect(student).to be_nil
     end
 
     it 'ignores members not included in the current scope' do
-      create(:project)
+      create(:project, school:, user_id: student.id)
 
-      user = described_class.none.users(teacher).first
-      expect(user).to be_nil
+      student = described_class.none.students(school, teacher).first
+      expect(student).to be_nil
     end
   end
 
-  describe '.with_users' do
+  describe '.with_students' do
     let(:student) { create(:student, school:) }
     let(:teacher) { create(:teacher, school:) }
 
@@ -231,29 +231,29 @@
     it 'returns an array of class members paired with their User instance' do
       project = create(:project, user_id: student.id)
 
-      pair = described_class.all.with_users(teacher).first
-      user = described_class.all.users(teacher).first
+      pair = described_class.all.with_students(school, teacher).first
+      students = described_class.all.students(school, teacher).first
 
-      expect(pair).to eq([project, user])
+      expect(pair).to eq([project, students])
     end
 
     it 'returns nil values for members where no profile account exists' do
-      user_id = SecureRandom.uuid
-      project = create(:project, user_id:)
+      stub_profile_api_list_school_students(school:, student_attributes: [])
+      project = create(:project, school:, user_id: student.id)
 
-      pair = described_class.all.with_users(teacher).first
+      pair = described_class.all.with_students(school, teacher).first
       expect(pair).to eq([project, nil])
     end
 
     it 'ignores members not included in the current scope' do
       create(:project)
 
-      pair = described_class.none.with_users(teacher).first
+      pair = described_class.none.with_students(school, teacher).first
       expect(pair).to be_nil
     end
   end
 
-  describe '#with_user' do
+  describe '#with_student' do
     let(:student) { create(:student, school:) }
     let(:teacher) { create(:teacher, school:) }
 
@@ -266,20 +266,20 @@
     end
 
     it 'returns the class member paired with their User instance' do
-      project = create(:project, user_id: student.id)
+      project = create(:project, school:, user_id: student.id)
 
-      pair = project.with_user(teacher)
-      user = described_class.all.users(teacher).first
+      pair = project.with_student(teacher)
+      student = described_class.all.students(school, teacher).first
 
-      expect(pair).to eq([project, user])
+      expect(pair).to eq([project, student])
     end
 
     it 'calls the Profile API to fetch the user' do
-      project = create(:project, user_id: student.id, school_id: school.id)
+      project = create(:project, school:, user_id: student.id)
 
       allow(SchoolStudent::List).to receive(:call).and_call_original
 
-      project.with_user(teacher)
+      project.with_student(teacher)
 
       expect(SchoolStudent::List).to have_received(:call).with(
         school:,
@@ -289,10 +289,10 @@
     end
 
     it 'returns a nil value if the member has no profile account' do
-      user_id = SecureRandom.uuid
-      project = create(:project, user_id:)
+      stub_profile_api_list_school_students(school:, student_attributes: [])
+      project = create(:project, school:, user_id: student.id)
 
-      pair = project.with_user(teacher)
+      pair = project.with_student(teacher)
       expect(pair).to eq([project, nil])
     end
 
@@ -302,7 +302,7 @@
 
         allow(SchoolStudent::List).to receive(:call)
 
-        pair = project.with_user(student)
+        pair = project.with_student(student)
 
         expect(pair).to eq([project, nil])
         expect(SchoolStudent::List).not_to have_received(:call)