diff --git a/app/models/group.rb b/app/models/group.rb index 40b613afa4d..91ac56e2f6c 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -41,6 +41,7 @@ class Group < Principal end validate :no_circular_parent, if: -> { parent_id.present? } + validate :no_organizational_unit_mismatch, if: -> { parent_id.present? } # Register a partial to be rendered on the synchronized groups tab of the groups admin page # @@ -170,4 +171,13 @@ class Group < Principal errors.add(:parent_id, :circular_dependency) end end + + def no_organizational_unit_mismatch + parent = self.class.find_by(id: parent_id) + return unless parent + + if organizational_unit? != parent.organizational_unit? + errors.add(:parent_id, :organizational_unit_mismatch) + end + end end diff --git a/config/locales/en.yml b/config/locales/en.yml index 01db1ca5e0a..a756518e6fe 100644 --- a/config/locales/en.yml +++ b/config/locales/en.yml @@ -2107,6 +2107,7 @@ en: attributes: parent_id: circular_dependency: "would create a circular group hierarchy." + organizational_unit_mismatch: "must have the same organizational unit setting as the group." ldap_auth_source: attributes: tls_certificate_string: diff --git a/spec/models/group_spec.rb b/spec/models/group_spec.rb index 68f5b966024..1233f13fc56 100644 --- a/spec/models/group_spec.rb +++ b/spec/models/group_spec.rb @@ -363,6 +363,35 @@ RSpec.describe Group do expect(child).to be_valid end end + + describe "organizational unit mismatch prevention" do + let(:department) { create(:department) } + let(:regular_group) { create(:group) } + + it "is invalid when assigning organizational unit as parent to regular group" do + regular_group.parent_id = department.id + expect(regular_group).not_to be_valid + expect(regular_group.errors[:parent_id]).to be_present + end + + it "is invalid when assigning regular group as parent to organizational unit" do + department.parent_id = regular_group.id + expect(department).not_to be_valid + expect(department.errors[:parent_id]).to be_present + end + + it "is valid when both are organizational units" do + child_department = create(:department) + child_department.parent_id = department.id + expect(child_department).to be_valid + end + + it "is valid when both are regular groups" do + child_group = create(:group) + child_group.parent_id = regular_group.id + expect(child_group).to be_valid + end + end end it_behaves_like "creates an audit trail on destroy" do