diff --git a/.rubocop.yml b/.rubocop.yml index b8ca7ed8..ce780ec6 100644 --- a/.rubocop.yml +++ b/.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 diff --git a/app/controllers/api/class_members_controller.rb b/app/controllers/api/class_members_controller.rb index 34454b2b..1a93b9cc 100644 --- a/app/controllers/api/class_members_controller.rb +++ b/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 diff --git a/app/controllers/api/school_teachers_controller.rb b/app/controllers/api/school_teachers_controller.rb index d7994495..e9f086e0 100644 --- a/app/controllers/api/school_teachers_controller.rb +++ b/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] diff --git a/app/graphql/mutations/create_component.rb b/app/graphql/mutations/create_component.rb index 8a3ca486..e1896fd9 100644 --- a/app/graphql/mutations/create_component.rb +++ b/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 diff --git a/app/graphql/mutations/delete_project.rb b/app/graphql/mutations/delete_project.rb index abc14154..c371219f 100644 --- a/app/graphql/mutations/delete_project.rb +++ b/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 diff --git a/app/graphql/mutations/remix_project.rb b/app/graphql/mutations/remix_project.rb index 33a0ff41..e95c728f 100644 --- a/app/graphql/mutations/remix_project.rb +++ b/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? diff --git a/app/models/ability.rb b/app/models/ability.rb index 321629fb..e84ccbc6 100644 --- a/app/models/ability.rb +++ b/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 diff --git a/app/models/filesystem_project.rb b/app/models/filesystem_project.rb index 1d056ea9..c2622367 100644 --- a/app/models/filesystem_project.rb +++ b/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 diff --git a/app/services/school_verification_service.rb b/app/services/school_verification_service.rb index 38b6f4ff..f9cb5b5a 100644 --- a/app/services/school_verification_service.rb +++ b/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 diff --git a/app/views/api/class_members/index.json.jbuilder b/app/views/api/class_members/index.json.jbuilder index 9dd65498..3e04d4cc 100644 --- a/app/views/api/class_members/index.json.jbuilder +++ b/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 diff --git a/config/routes.rb b/config/routes.rb index dd3ff9d6..9b07d3db 100644 --- a/config/routes.rb +++ b/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 diff --git a/lib/concepts/class_member/operations/list.rb b/lib/concepts/class_member/operations/list.rb new file mode 100644 index 00000000..e18997e1 --- /dev/null +++ b/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 diff --git a/lib/concepts/school_teacher/list.rb b/lib/concepts/school_teacher/list.rb index dfefc553..f8cfede1 100644 --- a/lib/concepts/school_teacher/list.rb +++ b/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 diff --git a/lib/concepts/school_teacher/remove.rb b/lib/concepts/school_teacher/remove.rb index 50561b4e..505f331e 100644 --- a/lib/concepts/school_teacher/remove.rb +++ b/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 diff --git a/lib/profile_api_client.rb b/lib/profile_api_client.rb index 98344582..6bcdc466 100644 --- a/lib/profile_api_client.rb +++ b/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? diff --git a/lib/tasks/classroom_management.rake b/lib/tasks/classroom_management.rake index 1b4032f0..add61a6d 100644 --- a/lib/tasks/classroom_management.rake +++ b/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 diff --git a/lib/tasks/classroom_management_helper.rb b/lib/tasks/classroom_management_helper.rb index 3cd281fc..e296e817 100644 --- a/lib/tasks/classroom_management_helper.rb +++ b/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| diff --git a/spec/concepts/class_member/list_spec.rb b/spec/concepts/class_member/list_spec.rb new file mode 100644 index 00000000..e5b2233a --- /dev/null +++ b/spec/concepts/class_member/list_spec.rb @@ -0,0 +1,82 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe ClassMember::List, type: :unit do + let(:token) { UserProfileMock::TOKEN } + let(:school) { create(:school) } + let(:students) { create_list(:student, 3, school:) } + let(:teacher) { create(:teacher, school:) } + let(:school_class) { create(:school_class, teacher_id: teacher.id, school:) } + let(:class_members) { school_class.members } + + let(:student_ids) { students.map(&:id) } + let(:teacher_ids) { [teacher.id] } + + context 'with students and a teacher' do + before do + student_ids.each do |student_id| + create(:class_member, school_class:, student_id:) + end + + student_attributes = students.map do |student| + { id: student.id, name: student.name, username: student.username } + end + stub_profile_api_list_school_students(school:, student_attributes:) + stub_user_info_api_for(teacher) + end + + it 'returns a successful operation response' do + response = described_class.call(school_class:, class_members:, token:) + expect(response.success?).to be(true) + end + + it 'returns class members in the operation response' do + response = described_class.call(school_class:, class_members:, token:) + expect(response[:class_members].count { |member| member.is_a?(ClassMember) }).to eq(3) + end + + it 'returns the teacher in the operation response' do + response = described_class.call(school_class:, class_members:, token:) + expect(response[:class_members].count { |member| member.is_a?(User) }).to eq(1) + end + + it 'contains the expected students' do + response = described_class.call(school_class:, class_members:, token:) + class_members.each do |class_member| + expect(response[:class_members].map(&:id)).to include(class_member.id) + end + end + + it 'contains the expected teacher' do + response = described_class.call(school_class:, class_members:, token:) + expect(response[:class_members].map(&:id)).to include(teacher.id) + end + end + + context 'when errors occur' do + before do + allow(Sentry).to receive(:capture_exception) + end + + # rubocop:disable RSpec/MultipleExpectations + it 'captures and handles errors' do + allow(SchoolStudent::List).to receive(:call).and_raise(StandardError.new('forced error')) + + response = described_class.call(school_class:, class_members:, token:) + + expect(response[:error]).to eq('Error listing class members: forced error') + expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError)) + end + # rubocop:enable RSpec/MultipleExpectations + + it 'returns an empty array when no ids match' do + allow(SchoolStudent::List).to receive(:call).and_return({ school_students: [] }) + allow(SchoolTeacher::List).to receive(:call).and_return({ school_teachers: [] }) + + response = described_class.call(school_class:, class_members:, token:) + + expect(response[:class_members]).to eq([]) + end + end +end diff --git a/spec/concepts/school_teacher/list_spec.rb b/spec/concepts/school_teacher/list_spec.rb index 407df33b..4e403b56 100644 --- a/spec/concepts/school_teacher/list_spec.rb +++ b/spec/concepts/school_teacher/list_spec.rb @@ -3,51 +3,40 @@ require 'rails_helper' RSpec.describe SchoolTeacher::List, type: :unit do - let(:token) { UserProfileMock::TOKEN } let(:school) { create(:school) } - let(:teacher) { create(:teacher, school:) } + let(:teachers) { create_list(:teacher, 3, school:) } + let(:teacher_ids) { teachers.map(&:id) } + let(:response) { described_class.call(teacher_ids:) } - before do - stub_profile_api_list_school_teachers(user_id: teacher.id) - stub_user_info_api_for(teacher) - end - - it 'returns a successful operation response' do - response = described_class.call(school:, token:) - expect(response.success?).to be(true) - end - - it 'makes a profile API call' do - described_class.call(school:, token:) + context 'when successful' do + before do + allow(User).to receive(:from_userinfo).with(ids: teacher_ids).and_return(teachers) + end - # TODO: Replace with WebMock assertion once the profile API has been built. - expect(ProfileApiClient).to have_received(:list_school_teachers).with(token:, organisation_id: school.id) + # rubocop:disable RSpec/MultipleExpectations + it 'returns a successful response with school teachers' do + expect(response[:school_teachers]).to eq(teachers) + expect(response[:error]).to be_nil + end + # rubocop:enable RSpec/MultipleExpectations end - it 'returns the school teachers in the operation response' do - response = described_class.call(school:, token:) - expect(response[:school_teachers].first).to be_a(User) - end + context 'when an error occurs' do + let(:error_message) { 'Error listing school teachers: some error' } - context 'when listing fails' do before do - allow(ProfileApiClient).to receive(:list_school_teachers).and_raise('Some API error') + allow(User).to receive(:from_userinfo).with(ids: teacher_ids).and_raise(StandardError.new('some error')) allow(Sentry).to receive(:capture_exception) end - it 'returns a failed operation response' do - response = described_class.call(school:, token:) - expect(response.failure?).to be(true) - end - - it 'returns the error message in the operation response' do - response = described_class.call(school:, token:) - expect(response[:error]).to match(/Some API error/) - end - - it 'sent the exception to Sentry' do - described_class.call(school:, token:) - expect(Sentry).to have_received(:capture_exception).with(kind_of(StandardError)) + # rubocop:disable RSpec/MultipleExpectations + it 'captures the exception and returns an error response' do + # Call the method to ensure the error is raised and captured + response + expect(Sentry).to have_received(:capture_exception).with(instance_of(StandardError)) + expect(response[:school_teachers]).to be_nil + expect(response[:error]).to eq(error_message) end + # rubocop:enable RSpec/MultipleExpectations end end diff --git a/spec/features/class_member/listing_class_members_spec.rb b/spec/features/class_member/listing_class_members_spec.rb index 03701ffb..73fb0d6c 100644 --- a/spec/features/class_member/listing_class_members_spec.rb +++ b/spec/features/class_member/listing_class_members_spec.rb @@ -3,23 +3,27 @@ require 'rails_helper' RSpec.describe 'Listing class members', type: :request do + let(:headers) { { Authorization: UserProfileMock::TOKEN } } + let(:school) { create(:school) } + let(:owner) { create(:owner, school:) } + let(:teacher) { create(:teacher, school:) } + let(:students) { create_list(:student, 3, school:) } + let(:school_class) { build(:school_class, teacher_id: teacher.id, school:) } + before do authenticated_in_hydra_as(owner) + student_attributes = students.map do |student| { id: student.id, name: student.name, username: student.username } end stub_profile_api_list_school_students(school:, student_attributes:) + students.each do |student| create(:class_member, student_id: student.id, school_class:) end - end - let(:headers) { { Authorization: UserProfileMock::TOKEN } } - let(:school_class) { build(:school_class, teacher_id: teacher.id, school:) } - let(:school) { create(:school) } - let(:students) { create_list(:student, 3, school:) } - let(:teacher) { create(:teacher, school:) } - let(:owner) { create(:owner, school:) } + stub_user_info_api_for(teacher) + end it 'responds 200 OK' do get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) @@ -30,37 +34,79 @@ get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) data = JSON.parse(response.body, symbolize_names: true) - expect(data.size).to eq(3) + expect(data.size).to eq(4) end - it 'responds with the correct student_ids' do + it 'responds with the correct member ids, where applicable' do get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) data = JSON.parse(response.body, symbolize_names: true) - student_ids_from_response = data.pluck(:student_id) - expected_student_ids = students.map(&:id) + school_class.members.each do |class_member| + expect(data.pluck(:id)).to include(class_member.id) + end + end - expect(student_ids_from_response).to match_array(expected_student_ids) + it 'responds with the correct student ids' do + get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) + data = JSON.parse(response.body, symbolize_names: true) + student_ids = data.pluck(:student).compact.pluck(:id) + + school_class.members.each do |class_member| + expect(student_ids).to include(class_member.student_id) + end + end + + it 'responds with the expected student parameters' do + get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) + data = JSON.parse(response.body, symbolize_names: true) + student_data = data.pluck(:student).compact.find { |member| member[:id] == students[0].id } + + expect(student_data).to eq( + { + id: students[0].id, + username: students[0].username, + name: students[0].name + } + ) end - it 'responds with the correct nested student ids' do + it 'responds with the correct teacher id' do get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) data = JSON.parse(response.body, symbolize_names: true) + teacher_id = data.pluck(:teacher).compact.pick(:id) - student_ids_from_response = data.map { |member| member[:student][:id] } - expected_student_ids = students.map(&:id) + expect(teacher_id).to eq(teacher.id) + end + + it 'responds with the expected teacher parameters' do + get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) + data = JSON.parse(response.body, symbolize_names: true) + teacher_data = data.pluck(:teacher).compact + + expect(teacher_data.first).to eq( + { + id: teacher.id, + name: teacher.name, + email: teacher.email + } + ) + end + + it 'responds with teachers at the top' do + get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) + data = JSON.parse(response.body, symbolize_names: true) - expect(student_ids_from_response).to match_array(expected_student_ids) + expect(data[0][:teacher]).to be_truthy end - it 'responds with the students JSON' do + it 'responds with students in alphabetical order by name ascending' do get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) data = JSON.parse(response.body, symbolize_names: true) - student_names_from_response = data.map { |member| member[:student][:name] } - expected_student_names = students.map(&:name) + student_names = data.pluck(:student).compact.pluck(:name) + sorted_student_names = student_names.sort - expect(student_names_from_response).to match_array(expected_student_names) + expect(student_names).to eq(sorted_student_names) end it "responds with nil attributes for students if the user profile doesn't exist" do @@ -72,7 +118,6 @@ expect(data.first[:student_name]).to be_nil end - # rubocop:disable RSpec/ExampleLength it 'does not include class members that belong to a different class' do student = create(:student, school:) different_class = create(:school_class, school:, teacher_id: teacher.id) @@ -81,9 +126,8 @@ get("/api/schools/#{school.id}/classes/#{school_class.id}/members", headers:) data = JSON.parse(response.body, symbolize_names: true) - expect(data.size).to eq(3) + expect(data.size).to eq(4) end - # rubocop:enable RSpec/ExampleLength it 'responds 401 Unauthorized when no token is given' do get "/api/schools/#{school.id}/classes/#{school_class.id}/members" diff --git a/spec/features/lesson/listing_lessons_spec.rb b/spec/features/lesson/listing_lessons_spec.rb index 93e6c34d..df501817 100644 --- a/spec/features/lesson/listing_lessons_spec.rb +++ b/spec/features/lesson/listing_lessons_spec.rb @@ -48,7 +48,6 @@ expect(data.first[:project]).to eq(expected_project) end - # rubocop:disable RSpec/ExampleLength it "responds with nil attributes for the user if their user profile doesn't exist" do user_id = SecureRandom.uuid stub_user_info_api_for_unknown_users(user_id:) @@ -59,7 +58,6 @@ expect(data.first[:user_name]).to be_nil end - # rubocop:enable RSpec/ExampleLength it 'does not include archived lessons' do lesson.archive! @@ -190,7 +188,6 @@ let!(:lesson) { create(:lesson, school_class:, name: 'Test Lesson', visibility: 'students', user_id: teacher.id) } let(:teacher) { create(:teacher, school:) } - # rubocop:disable RSpec/ExampleLength it 'includes the lesson when the user owns the lesson' do another_teacher = create(:teacher, school:) authenticated_in_hydra_as(another_teacher) @@ -201,9 +198,7 @@ expect(data.size).to eq(1) end - # rubocop:enable RSpec/ExampleLength - # rubocop:disable RSpec/ExampleLength it "includes the lesson when the user is a school-student within the lesson's class" do student = create(:student, school:) authenticated_in_hydra_as(student) @@ -214,7 +209,6 @@ expect(data.size).to eq(1) end - # rubocop:enable RSpec/ExampleLength it "does not include the lesson when the user is not a school-student within the lesson's class" do student = create(:student, school:) diff --git a/spec/features/lesson/showing_a_lesson_spec.rb b/spec/features/lesson/showing_a_lesson_spec.rb index 4856da1e..674b0481 100644 --- a/spec/features/lesson/showing_a_lesson_spec.rb +++ b/spec/features/lesson/showing_a_lesson_spec.rb @@ -38,7 +38,6 @@ expect(data[:user_name]).to eq('School Teacher') end - # rubocop:disable RSpec/ExampleLength it "responds with nil attributes for the user if their user profile doesn't exist" do user_id = SecureRandom.uuid stub_user_info_api_for_unknown_users(user_id:) @@ -49,7 +48,6 @@ expect(data[:user_name]).to be_nil end - # rubocop:enable RSpec/ExampleLength it 'responds 404 Not Found when no lesson exists' do get('/api/lessons/not-a-real-id', headers:) diff --git a/spec/features/lesson/updating_a_lesson_spec.rb b/spec/features/lesson/updating_a_lesson_spec.rb index 97c61cd6..b43f04e5 100644 --- a/spec/features/lesson/updating_a_lesson_spec.rb +++ b/spec/features/lesson/updating_a_lesson_spec.rb @@ -115,7 +115,6 @@ expect(response).to have_http_status(:ok) end - # rubocop:disable RSpec/ExampleLength it 'responds 422 Unprocessable Entity when trying to re-assign the lesson to a different class' do school = create(:school, id: SecureRandom.uuid) teacher = create(:teacher, school:) @@ -126,6 +125,5 @@ expect(response).to have_http_status(:unprocessable_entity) end - # rubocop:enable RSpec/ExampleLength end end diff --git a/spec/features/school_class/listing_school_classes_spec.rb b/spec/features/school_class/listing_school_classes_spec.rb index a1f11395..99c15f47 100644 --- a/spec/features/school_class/listing_school_classes_spec.rb +++ b/spec/features/school_class/listing_school_classes_spec.rb @@ -45,7 +45,6 @@ expect(data.first[:teacher_name]).to be_nil end - # rubocop:disable RSpec/ExampleLength it "does not include school classes that the school-teacher doesn't teach" do teacher = create(:teacher, school:) authenticated_in_hydra_as(teacher) @@ -56,9 +55,7 @@ expect(data.size).to eq(1) end - # rubocop:enable RSpec/ExampleLength - # rubocop:disable RSpec/ExampleLength it "does not include school classes that the school-student isn't a member of" do teacher = create(:teacher, school:) authenticated_in_hydra_as(student) @@ -69,7 +66,6 @@ expect(data.size).to eq(1) end - # rubocop:enable RSpec/ExampleLength it 'responds 401 Unauthorized when no token is given' do get "/api/schools/#{school.id}/classes" diff --git a/spec/features/school_teacher/listing_school_teachers_spec.rb b/spec/features/school_teacher/listing_school_teachers_spec.rb index f5b379ea..02fed958 100644 --- a/spec/features/school_teacher/listing_school_teachers_spec.rb +++ b/spec/features/school_teacher/listing_school_teachers_spec.rb @@ -3,37 +3,22 @@ require 'rails_helper' RSpec.describe 'Listing school teachers', type: :request do - before do - authenticated_in_hydra_as(owner) - stub_profile_api_list_school_teachers(user_id: teacher.id) - stub_user_info_api_for(teacher) - end - let(:headers) { { Authorization: UserProfileMock::TOKEN } } let(:school) { create(:school) } - let(:teacher) { create(:teacher, school:, name: 'School Teacher') } let(:owner) { create(:owner, school:) } + let(:teacher) { create(:teacher, school:, name: 'School Teacher') } + let(:teacher_2) { create(:teacher, school:) } - it 'responds 200 OK' do - get("/api/schools/#{school.id}/teachers", headers:) - expect(response).to have_http_status(:ok) + before do + authenticated_in_hydra_as(owner) + stub_user_info_api_for(teacher) end - it 'responds 200 OK when the user is a school-teacher' do - teacher = create(:teacher, school:) - authenticated_in_hydra_as(teacher) - + it 'responds 200 OK' do get("/api/schools/#{school.id}/teachers", headers:) expect(response).to have_http_status(:ok) end - it 'responds with the school teachers JSON' do - get("/api/schools/#{school.id}/teachers", headers:) - data = JSON.parse(response.body, symbolize_names: true) - - expect(data.first[:name]).to eq('School Teacher') - end - it 'responds 401 Unauthorized when no token is given' do get "/api/schools/#{school.id}/teachers" expect(response).to have_http_status(:unauthorized) @@ -55,4 +40,22 @@ get("/api/schools/#{school.id}/teachers", headers:) expect(response).to have_http_status(:forbidden) end + + it 'responds 200 OK when the user is a school-teacher' do + stub_user_info_api_for_users([teacher.id, teacher_2.id], users: [teacher, teacher_2]) + authenticated_in_hydra_as(teacher_2) + + get("/api/schools/#{school.id}/teachers", headers:) + expect(response).to have_http_status(:ok) + end + + it 'responds with the school teachers JSON' do + stub_user_info_api_for_users([teacher.id, teacher_2.id], users: [teacher, teacher_2]) + authenticated_in_hydra_as(teacher_2) + + get("/api/schools/#{school.id}/teachers", headers:) + data = JSON.parse(response.body, symbolize_names: true) + + expect(data.pluck(:name)).to include(teacher_2.name) + end end diff --git a/spec/lib/classroom_management_spec.rb b/spec/lib/classroom_management_spec.rb index 72c0ffca..6e787cb8 100644 --- a/spec/lib/classroom_management_spec.rb +++ b/spec/lib/classroom_management_spec.rb @@ -23,7 +23,7 @@ create(:lesson, school_id: school.id, user_id: creator_id) end - # rubocop:disable RSpec/ExampleLength, RSpec/MultipleExpectations + # rubocop:disable RSpec/MultipleExpectations it 'destroys all seed data' do task.invoke expect(Role.where(user_id: [creator_id, teacher_id, student_1, student_2])).not_to exist @@ -33,7 +33,7 @@ expect(Lesson.where(school_id: school.id)).not_to exist expect(Project.where(school_id: school.id)).not_to exist end - # rubocop:enable RSpec/ExampleLength, RSpec/MultipleExpectations + # rubocop:enable RSpec/MultipleExpectations end describe ':seed_an_unverified_school' do @@ -80,7 +80,7 @@ expect(Role.teacher.where(user_id: teacher_id, school_id: school.id)).to exist end - # rubocop:disable RSpec/MultipleExpectations, RSpec/ExampleLength + # rubocop:disable RSpec/MultipleExpectations it 'assigns students' do school_id = School.find_by(creator_id:).id school_class_id = SchoolClass.find_by(school_id:).id @@ -89,6 +89,6 @@ expect(Role.student.where(user_id: student_2, school_id:)).to exist expect(ClassMember.where(student_id: student_2, school_class_id:)).to exist end - # rubocop:enable RSpec/MultipleExpectations, RSpec/ExampleLength + # rubocop:enable RSpec/MultipleExpectations end end diff --git a/spec/lib/profile_api_client_spec.rb b/spec/lib/profile_api_client_spec.rb index 1663fb2a..48ed7ce3 100644 --- a/spec/lib/profile_api_client_spec.rb +++ b/spec/lib/profile_api_client_spec.rb @@ -172,7 +172,6 @@ def create_school expect(WebMock).to have_requested(:get, list_safeguarding_flags_url).with(headers: { 'accept' => 'application/json' }) end - # rubocop:disable RSpec/ExampleLength it 'returns list of safeguarding flags if successful' do flag = { id: '7ac79585-e187-4d2f-bf0c-a1cbe72ecc9a', @@ -188,7 +187,6 @@ def create_school .to_return(status: 200, body: [flag].to_json, headers: { 'content-type' => 'application/json' }) expect(list_safeguarding_flags).to eq([expected]) end - # rubocop:enable RSpec/ExampleLength it 'raises exception if anything other than a 200 status code is returned' do stub_request(:get, list_safeguarding_flags_url) @@ -459,7 +457,6 @@ def create_school_student expect(WebMock).to have_requested(:post, list_students_url).with(body: student_ids) end - # rubocop:disable RSpec/ExampleLength it 'returns the student(s) if successful' do student = { id: '549e4674-6ffd-4ac6-9a97-b4d7e5c0e5c5', @@ -475,7 +472,6 @@ def create_school_student .to_return(status: 200, body: [student].to_json, headers: { 'content-type' => 'application/json' }) expect(list_school_students).to eq([expected]) end - # rubocop:enable RSpec/ExampleLength it 'raises exception if anything other that 200 status code is returned' do stub_request(:post, list_students_url) @@ -637,7 +633,6 @@ def update_school_student expect(WebMock).to have_requested(:get, student_url).with(headers: { 'accept' => 'application/json' }) end - # rubocop:disable RSpec/ExampleLength it 'returns the student(s) if successful' do student = { id: '549e4674-6ffd-4ac6-9a97-b4d7e5c0e5c5', @@ -653,7 +648,6 @@ def update_school_student .to_return(status: 200, body: student.to_json, headers: { 'content-type' => 'application/json' }) expect(school_student).to eq(expected) end - # rubocop:enable RSpec/ExampleLength it 'raises exception if anything other than a 200 status code is returned' do stub_request(:get, student_url) diff --git a/spec/support/profile_api_mock.rb b/spec/support/profile_api_mock.rb index dc43ff60..c5c1db29 100644 --- a/spec/support/profile_api_mock.rb +++ b/spec/support/profile_api_mock.rb @@ -15,10 +15,6 @@ def stub_profile_api_remove_school_owner allow(ProfileApiClient).to receive(:remove_school_owner) end - def stub_profile_api_list_school_teachers(user_id:) - allow(ProfileApiClient).to receive(:list_school_teachers).and_return(ids: [user_id]) - end - def stub_profile_api_remove_school_teacher allow(ProfileApiClient).to receive(:remove_school_teacher) end diff --git a/spec/support/user_profile_mock.rb b/spec/support/user_profile_mock.rb index bdcee714..4f56e353 100644 --- a/spec/support/user_profile_mock.rb +++ b/spec/support/user_profile_mock.rb @@ -51,4 +51,11 @@ def stub_user_info_api(user_id:, users:) .with(headers: { Authorization: "Bearer #{UserInfoApiClient::API_KEY}" }, body: /#{user_id}/) .to_return({ body: { users: }.to_json, headers: { 'Content-Type' => 'application/json' } }) end + + # Stubs the api to accept multiple user ids and return multiple users + def stub_user_info_api_for_users(user_ids, users:) + stub_request(:get, "#{UserInfoApiClient::API_URL}/users") + .with(headers: { Authorization: "Bearer #{UserInfoApiClient::API_KEY}" }, body: { userIds: user_ids }.to_json) + .to_return({ body: { users: }.to_json, headers: { 'Content-Type' => 'application/json' } }) + end end