Skip to content

Commit

Permalink
Merge pull request #184 from hitobito/feature/jubla-162-reduce-alumnu…
Browse files Browse the repository at this point in the history
…s-management-above-flock

Reduce alumnus management above flock
  • Loading branch information
kronn authored Dec 18, 2024
2 parents 8a5bb04 + 9464edf commit aad966d
Show file tree
Hide file tree
Showing 7 changed files with 208 additions and 27 deletions.
4 changes: 2 additions & 2 deletions app/domain/jubla/role/alumnus_manager.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

# Copyright (c) 2021, Jungwacht Blauring Schweiz. This file is part of
# Copyright (c) 2021-2024, Jungwacht Blauring Schweiz. This file is part of
# hitobito_jubla and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_jubla.
Expand Down Expand Up @@ -55,7 +55,7 @@ def destroy_alumnus_role
person
.roles
.joins(:group)
.where(group: group, type: group.alumnus_class.name)
.where(group: group, type: group.alumnus_class.sti_name)
.destroy_all
end

Expand Down
50 changes: 50 additions & 0 deletions app/domain/jubla/role/light_alumnus_manager.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
# frozen_string_literal: true

# Copyright (c) 2021-2024, Jungwacht Blauring Schweiz. This file is part of
# hitobito_jubla and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_jubla.

# Trimmed down version of the Jubla::Role::AlumnusManager
#
# This one only creates the "exiting"-role that can be filtered. It does not add
# the person to the "former people"-group
module Jubla::Role
class LightAlumnusManager
attr_reader :role, :group, :person

def initialize(role)
@role = role
@group = role.group
@person = role.person
end

def create
create_alumnus_role if last_in_group?
end

def destroy
destroy_alumnus_role
end

private

def create_alumnus_role
alumnus_role = group.alumnus_class.new(person: person, group: group)
alumnus_role.label = role.class.label
alumnus_role.save!
end

def destroy_alumnus_role
person
.roles
.joins(:group)
.where(group: group, type: group.alumnus_class.sti_name)
.destroy_all
end

def last_in_group?
group.roles.where(person_id: person.id).empty?
end
end
end
4 changes: 0 additions & 4 deletions app/models/group/root.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,6 @@ class Admin < ::Role
self.permissions = [:layer_and_below_full, :admin]
self.two_factor_authentication_enforced = true

def skip_alumnus_callback
true
end

def alumnus_manager
@alumnus_manager ||= Jubla::Role::NullAlumnusManager.new
end
Expand Down
10 changes: 6 additions & 4 deletions app/models/jubla/role.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# frozen_string_literal: true

# Copyright (c) 2012-2021, Jungwacht Blauring Schweiz. This file is part of
# Copyright (c) 2012-2024, Jungwacht Blauring Schweiz. This file is part of
# hitobito_jubla and licensed under the Affero General Public License version 3
# or later. See the COPYING file at the top-level directory or at
# https://github.com/hitobito/hitobito_jubla.
Expand Down Expand Up @@ -143,8 +143,10 @@ def alumnus_manager_destroy_for_alumnus
end

def alumnus_manager
@alumnus_manager ||= Jubla::Role::AlumnusManager.new(
self, skip_alumnus_callback: skip_alumnus_callback
)
@alumnus_manager ||= if %w[Flock ChildGroup FlockAlumnusGroup].include? type.split("::", 3)[1]
Jubla::Role::AlumnusManager.new(self, skip_alumnus_callback: skip_alumnus_callback)
else
Jubla::Role::LightAlumnusManager.new(self)
end
end
end
4 changes: 0 additions & 4 deletions app/models/nejb_role.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,6 @@

class NejbRole < Role
module AlumnusFreeRole
def skip_alumnus_callback
true
end

def alumnus_manager
@alumnus_manager ||= Jubla::Role::NullAlumnusManager.new
end
Expand Down
4 changes: 0 additions & 4 deletions spec/models/nejb_role_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,6 @@
expect(subject).to be_a Jubla::Role
end

it "disables the alumnus-callback" do
expect(subject.skip_alumnus_callback).to be_truthy
end

it "uses the NullAlumnusManager" do
expect(subject.alumnus_manager).to be_a Jubla::Role::NullAlumnusManager
end
Expand Down
159 changes: 150 additions & 9 deletions spec/models/role_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -65,14 +65,146 @@
end
end

context "alumnus group" do
context "alumnus group with AlumnusManager" do
let(:group) { groups(:bern) }
let(:alumni_group) { groups(:bern_ehemalige) }

def self.create_samples
[%w[Group::Flock::Leader bern -1],
%w[Group::Flock::CampLeader bern -1],
%w[Group::Flock::GroupAdmin bern -1]]
end

def self.destroy_samples
create_samples + [%w[Group::Flock::External bern 0],
%w[Group::Flock::DispatchAddress bern 0],
%w[Group::FlockAlumnusGroup::Leader bern_ehemalige 0]]
end

context "uses the full manager" do
destroy_samples.each do |role_type, group, _change|
it "#{role_type} uses AlumnusManager" do
role = Fabricate(role_type, group: groups(group))
expect(role.send(:alumnus_manager)).to be_a Jubla::Role::AlumnusManager
end
end
end

def alumnus_member_count
Group::FlockAlumnusGroup::Member.where(group: alumni_group).count
end

context "create" do
create_samples.each do |role_type, group, change|
it "#{role_type} changes alumni members by #{change}" do
role = Fabricate(Group::FlockAlumnusGroup::Member.to_s, group: alumni_group)
expect do
Fabricate(role_type, person: role.person, group: groups(group))
end.to change { alumnus_member_count }.by(change.to_i)
end
end
end

context "destroy" do
let(:too_young) { Settings.alumni_administrations.min_age_for_alumni_member - 1 }

destroy_samples.each do |role_type, group, change|
it "#{role_type} changes alumni members by #{change.to_i * -1}, enqueues job" do
role = Fabricate(role_type, group: groups(group), created_at: some_time_ago)
expect do
expect { role.destroy }.to change { Delayed::Job.count }.by(change.to_i * -1)
end.to change { alumnus_member_count }.by(change.to_i * -1)
end
end

it "still creates alumni member, if person has an alumnus role in same layer" do
person = Fabricate(Group::Flock::Alumnus.sti_name, group: groups(:bern)).person
role = Fabricate(Group::Flock::Leader.sti_name, group: groups(:bern), created_at: some_time_ago, person: person)
expect { role.destroy }.to change { alumnus_member_count }.by(1)
end

it "does not creates alumni member, if person is too young" do
role = Fabricate(Group::ChildGroup::Child.sti_name, group: groups(:asterix), created_at: some_time_ago)
role.person.update(birthday: too_young.years.ago)
expect { role.destroy }.not_to change { alumnus_member_count }
end

it "does not not enqueue mail job when no email is set" do
role = Fabricate(Group::Flock::Leader.sti_name, group: groups(:bern), created_at: some_time_ago)
role.person.update(email: nil)
expect { role.destroy }.not_to change { Delayed::Job.count }
end
end

context "after alumnus creation" do
it "create alumni member if new alumni role added" do
expect do
Fabricate("Group::Flock::Alumnus", group: groups(:bern))
end.to change { alumnus_member_count }.by(1)
end

it "only create one alumni member if multiple new alumni roles added in same layer" do
expect do
role = Fabricate("Group::Flock::Alumnus", group: groups(:bern))
Fabricate("Group::Flock::Alumnus", group: groups(:bern), person: role.person)
end.to change { alumnus_member_count }.by(1)
end
end

context "after alumnus deletion" do
it "destroies alumnus membership if last role" do
role = Fabricate("Group::Flock::Alumnus", group: groups(:bern))
expect do
role.destroy
end.to change { alumnus_member_count }.by(-1)
end

it "does nothing if there are some alumnus roles left in same layer" do
role = Fabricate("Group::Flock::Alumnus", group: groups(:bern))
Fabricate("Group::Flock::Alumnus", group: groups(:bern), person: role.person)
expect do
role.destroy
end.to change { alumnus_member_count }.by(0)
end
end

context "validations" do
it "allows creating alumnus leader if active role exists in same layer" do
person = Fabricate(Group::Flock::Leader.to_s, group: groups(:bern)).person
role = person.roles.build(type: Group::FlockAlumnusGroup::Leader.to_s, group: alumni_group)
expect(role).to be_valid
end

it "allows creating alumnus member if active role exists in other layer" do
person = Fabricate(Group::Flock::Leader.to_s, group: groups(:muri)).person
role = person.roles.build(type: Group::FlockAlumnusGroup::Member.to_s, group: alumni_group)
expect(role).to be_valid
end

it "allows creating alumnus member if only role in same layer is in another alumnus group" do
group = Group::FlockAlumnusGroup.create!(name: "other", parent: groups(:bern))
person = Fabricate(Group::FlockAlumnusGroup::Member.to_s, group: group).person
role = person.roles.build(type: Group::FlockAlumnusGroup::Member.to_s, group: alumni_group)
expect(role).to be_valid
end

it "does not allow creating alumnus member if active role in same layer exists" do
person = Fabricate(Group::Flock::Leader.to_s, group: groups(:bern)).person
role = person.roles.build(type: Group::FlockAlumnusGroup::Member.to_s, group: alumni_group)
expect(role).not_to be_valid
expect(role.errors.full_messages.first).to eq "Es befinden sich noch andere aktive Rollen in diesem Layer"
end
end
end

context "alumnus group with LightAlumnusManager" do
let(:group) { groups(:ch) }
let(:alumni_group) { groups(:ch_ehemalige) }

def self.create_samples
[%w[Group::FederalBoard::Member federal_board -1],
%w[Group::FederalBoard::President federal_board -1],
%w[Group::FederalBoard::GroupAdmin federal_board -1]]
[%w[Group::FederalBoard::Member federal_board 0],
%w[Group::FederalBoard::President federal_board 0],
%w[Group::FederalBoard::GroupAdmin federal_board 0]]
end

def self.destroy_samples
Expand All @@ -85,6 +217,15 @@ def alumnus_member_count
Group::FederalAlumnusGroup::Member.where(group: alumni_group).count
end

context "uses the stripped down manager" do
destroy_samples.each do |role_type, group, _change|
it "#{role_type} uses LightAlumnusManager" do
role = Fabricate(role_type, group: groups(group))
expect(role.send(:alumnus_manager)).to be_a Jubla::Role::LightAlumnusManager
end
end
end

context "create" do
create_samples.each do |role_type, group, change|
it "#{role_type} changes alumni members by #{change}" do
Expand All @@ -111,7 +252,7 @@ def alumnus_member_count
it "still creates alumni member, if person has an alumnus role in same layer" do
person = Fabricate(Group::OrganizationBoard::Alumnus.sti_name, group: groups(:organization_board)).person
role = Fabricate(Group::FederalBoard::Member.sti_name, group: groups(:federal_board), created_at: some_time_ago, person: person)
expect { role.destroy }.to change { alumnus_member_count }.by(1)
expect { role.destroy }.to_not change { alumnus_member_count }
end

it "does not creates alumni member, if person is too young" do
Expand All @@ -131,14 +272,14 @@ def alumnus_member_count
it "create alumni member if new alumni role added" do
expect do
Fabricate("Group::FederalBoard::Alumnus", group: groups(:federal_board))
end.to change { alumnus_member_count }.by(1)
end.to_not change { alumnus_member_count }
end

it "only create one alumni member if multiple new alumni roles added in same layer" do
expect do
role = Fabricate("Group::FederalBoard::Alumnus", group: groups(:federal_board))
Fabricate("Group::OrganizationBoard::Alumnus", group: groups(:organization_board), person: role.person)
end.to change { alumnus_member_count }.by(1)
end.to_not change { alumnus_member_count }
end
end

Expand All @@ -147,15 +288,15 @@ def alumnus_member_count
role = Fabricate("Group::FederalBoard::Alumnus", group: groups(:federal_board))
expect do
role.destroy
end.to change { alumnus_member_count }.by(-1)
end.to_not change { alumnus_member_count }
end

it "does nothing if there are some alumnus roles left in same layer" do
role = Fabricate("Group::FederalBoard::Alumnus", group: groups(:federal_board))
Fabricate("Group::OrganizationBoard::Alumnus", group: groups(:organization_board), person: role.person)
expect do
role.destroy
end.to change { alumnus_member_count }.by(0)
end.to_not change { alumnus_member_count }
end
end

Expand Down

0 comments on commit aad966d

Please sign in to comment.