From bfc64871e233daeeb445bec7a7c177c3fc7d8e90 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Tue, 1 Jul 2025 19:55:38 +0200 Subject: [PATCH] Specs: Use default study group for internal users When creating a teacher (or admin) through the respective factory, they were not yet automatically assigned to the respective default study group. This commit changes said behavior. The actual creation process within the UI remains unchanged, and it is still required to choose the appropriate study groups manually. Part of #2986 --- spec/controllers/exercises_controller_spec.rb | 5 +++-- spec/factories/internal_user.rb | 4 ++++ spec/factories/shared_traits.rb | 12 +++++++++--- spec/policies/exercise_policy_spec.rb | 18 +++++++++++++----- .../request_for_comment_policy_spec.rb | 2 +- spec/services/proforma_service/import_spec.rb | 2 +- spec/system/authentication_system_spec.rb | 2 +- 7 files changed, 32 insertions(+), 13 deletions(-) diff --git a/spec/controllers/exercises_controller_spec.rb b/spec/controllers/exercises_controller_spec.rb index 555cf093f..51209b3ab 100644 --- a/spec/controllers/exercises_controller_spec.rb +++ b/spec/controllers/exercises_controller_spec.rb @@ -202,7 +202,7 @@ context 'with other users accessing an unpublished exercise' do let(:exercise) { create(:fibonacci, unpublished: true) } - let(:user) { create(:teacher) } + let(:user) { create(:external_teacher) } before { perform_request.call } @@ -442,7 +442,8 @@ end context 'when the user cannot update the exercise' do - let(:codeharbor_link) { create(:codeharbor_link, api_key: 'anotherkey') } + let(:user) { create(:external_teacher) } + let(:codeharbor_link) { create(:codeharbor_link, user:, api_key: 'anotherkey') } it 'renders correct response' do post_request diff --git a/spec/factories/internal_user.rb b/spec/factories/internal_user.rb index c52bf94e6..808ec92c1 100644 --- a/spec/factories/internal_user.rb +++ b/spec/factories/internal_user.rb @@ -27,6 +27,10 @@ transient do teacher_in_study_group { true } end + + factory :external_teacher do + consumer { association :consumer, name: 'Other Consumer' } + end end factory :learner, class: 'InternalUser' do diff --git a/spec/factories/shared_traits.rb b/spec/factories/shared_traits.rb index 3f6cbf634..0e6501c86 100644 --- a/spec/factories/shared_traits.rb +++ b/spec/factories/shared_traits.rb @@ -23,10 +23,16 @@ trait :member_of_study_group do after(:create) do |user, evaluator| - # Do not create a study group if already passed - if user.study_groups.blank? - study_group = create(:study_group) + if user.study_groups.blank? && user.is_a?(ExternalUser) + # Do not create a study group if already passed + study_group = create(:study_group, consumer: user.consumer) user.study_groups << study_group + elsif user.is_a?(InternalUser) + # Always add the user to the default study group + default_study_group = user.consumer.study_groups.find_by(external_id: nil) + StudyGroupMembership.find_or_create_by!(study_group: default_study_group, user:) + # Reload the study groups to ensure the membership is reflected + user.study_groups.reload end user.study_group_memberships.update(role: 'teacher') if evaluator.teacher_in_study_group diff --git a/spec/policies/exercise_policy_spec.rb b/spec/policies/exercise_policy_spec.rb index bcad83fe4..2c25ffd5f 100644 --- a/spec/policies/exercise_policy_spec.rb +++ b/spec/policies/exercise_policy_spec.rb @@ -209,10 +209,18 @@ expect(policy).to permit(exercise.author, exercise) end - it 'does not grant access to everyone' do - %i[external_user teacher].each do |factory_name| - expect(policy).not_to permit(create(factory_name), exercise) - end + it 'grants access to teachers of the same study group' do + # By default, the exercise author and the teacher are internal users and share the same study group + expect(policy).to permit(create(:teacher), exercise) + end + + it 'does not grant access to teachers of another consumer' do + # With another consumer, the teacher is in another study group, too. + expect(policy).not_to permit(create(:external_teacher), exercise) + end + + it 'does not grant access to external users' do + expect(policy).not_to permit(create(:external_user), exercise) end end end @@ -222,7 +230,7 @@ describe '#resolve' do let(:admin) { create(:admin) } let(:external_user) { create(:external_user) } - let(:teacher) { create(:teacher) } + let(:teacher) { create(:external_teacher) } before do [admin, teacher].each do |user| diff --git a/spec/policies/request_for_comment_policy_spec.rb b/spec/policies/request_for_comment_policy_spec.rb index 5ec21715b..7c225666e 100644 --- a/spec/policies/request_for_comment_policy_spec.rb +++ b/spec/policies/request_for_comment_policy_spec.rb @@ -157,7 +157,7 @@ end end - let(:rfc_author) { create(:learner, consumer: author_consumer, study_groups: author_study_groups) } + let(:rfc_author) { create(:external_user, consumer: author_consumer, study_groups: author_study_groups) } let(:author_study_groups) { create_list(:study_group, 1, consumer: author_consumer) } let(:rfc) { create(:rfc, user: rfc_author) } diff --git a/spec/services/proforma_service/import_spec.rb b/spec/services/proforma_service/import_spec.rb index e5239c2da..985ea0e62 100644 --- a/spec/services/proforma_service/import_spec.rb +++ b/spec/services/proforma_service/import_spec.rb @@ -182,7 +182,7 @@ end context 'when another user imports the exercise' do - let(:import_user) { create(:teacher) } + let(:import_user) { create(:external_teacher) } it 'raises a proforma error' do expect { imported_exercise.save! }.to raise_error ProformaXML::ExerciseNotOwned diff --git a/spec/system/authentication_system_spec.rb b/spec/system/authentication_system_spec.rb index 2ab18c575..fb27e918b 100644 --- a/spec/system/authentication_system_spec.rb +++ b/spec/system/authentication_system_spec.rb @@ -3,7 +3,7 @@ require 'rails_helper' RSpec.describe 'Authentication' do - let(:user) { create(:teacher) } + let(:user) { create(:external_teacher) } let(:password) { attributes_for(:teacher)[:password] } context 'when signed out' do