Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/sac 136 household and managed adjustments #42

Merged
merged 6 commits into from
Jan 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions app/abilities/youth/person_ability.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ module Youth::PersonAbility
may(:show, :show_details, :show_full, :history, :update, :update_email, :primary_group,
:log, :totp_reset).
herself

class_side(:create_households).if_any_writing_permission_or_any_manageds
end

# People with update permission on a managed person also have the permission to update the
Expand All @@ -30,9 +32,15 @@ module Youth::PersonAbility
non_restricted_in_same_layer_except_self
permission(:layer_and_below_full).may(:change_managers).
non_restricted_in_same_layer_or_visible_below_except_self

class_side(:lookup_manageds).if_any_writing_permissions
end
end

def if_any_writing_permission_or_any_manageds
if_any_writing_permissions || user_context.user.manageds.any?
end

def non_restricted_in_same_group_except_self
non_restricted_in_same_group && !herself
end
Expand Down
22 changes: 21 additions & 1 deletion app/controllers/youth/people_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,14 +33,33 @@ def set_manager_notice
end.compact
end

def permitted_attrs
return unless FeatureGate.enabled?('people.people_managers.self_service_managed_creation')

self.class.permitted_attrs += [
people_manageds_attributes: [:id,
:managed_id,
:_destroy,
managed_attributes: [
:first_name,
:last_name,
:gender,
:birthday
]
]
]

self.class.permitted_attrs.uniq
end

def permitted_params
permitted = super
if cannot?(:change_managers, entry)
permitted = permitted.except(:people_managers_attributes)
end
permitted[:people_manageds_attributes]&.keep_if do |index, attrs|
managed = extract_managed(attrs)
managed.present? && can?(:change_managers, managed)
managed.present? && (managed.new_record? || can?(:change_managers, managed))
end
permitted
end
Expand All @@ -54,6 +73,7 @@ def accessible_people_managers_attributes
def extract_managed(attrs)
return Person.find_by(id: attrs[:managed_id]) if attrs[:managed_id].present?
return PeopleManager.find_by(id: attrs[:id]).managed if attrs[:id].present?
return Person.new(attrs[:managed_attributes]) if attrs[:managed_attributes].values.any?
nil
end

Expand Down
1 change: 1 addition & 0 deletions app/models/people_manager.rb
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ class PeopleManager < ActiveRecord::Base
belongs_to :manager, class_name: 'Person'
belongs_to :managed, class_name: 'Person'

accepts_nested_attributes_for :managed
validates :manager_id, uniqueness: { scope: :managed_id }
validate :assert_manager_is_not_managed

Expand Down
3 changes: 0 additions & 3 deletions app/views/people/_fields_youth.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -15,9 +15,6 @@

= field_set_tag do
= f.labeled_inline_fields_for :people_manageds, 'people/manageds/fields', f.object.people_manageds
= f.labeled(:manageds_help, '&nbsp;'.html_safe) do
%span.help-block
= t('.manageds_help')

= field_set_tag do
= f.labeled_input_fields(:ahv_number, :j_s_number)
Expand Down
10 changes: 10 additions & 0 deletions app/views/people/manageds/_create_new_managed_person.html.haml
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
= f.fields_for(:managed_attributes, Person.new) do |fields|
= field_set_tag do
= fields.labeled_input_fields :first_name, :last_name, required: true
= fields.labeled(:gender) do
- (Person::GENDERS + ['']).each do |key|
= fields.inline_radio_button(:gender, key, entry.gender_label(key))

= fields.labeled_input_field(:birthday,
help_inline: t('.birthday_hint'),
class: 'col-2 form-control form-control-sm d-inline')
10 changes: 7 additions & 3 deletions app/views/people/manageds/_fields.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,11 @@
-# https://github.com/hitobito/hitobito_youth.

- if f.object.new_record?
= f.person_field(:managed, { data: { url: query_people_path(limit_by_permission: :change_managers) } })
- if cannot?(:lookup_manageds, Person) && FeatureGate.enabled?('people.people_managers.self_service_managed_creation')
= render('people/manageds/create_new_managed_person', f: f)
- else
%span
= f.person_field(:managed, { disabled: cannot?(:create_households, Person), data: { url: query_people_path(limit_by_permission: :change_managers) } })
%span.help-block= t('.manageds_help')
- else
.col-4
= f.object.managed.full_name
= f.object.managed.full_name
6 changes: 5 additions & 1 deletion config/locales/views.youth.de.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,13 @@ de:
destroyed_manageds:
one: "Kind %{labels} entfernt."
other: "Kinder %{labels} entfernt."
manageds:
fields:
manageds_help: Du kannst nur Personen als Kinder hinzufügen, auf welche du Schreibzugriff hast.
create_new_managed_person:
birthday_hint: (dd.mm.yyyy)
fields_youth:
no_permission_to_change_managers: Du bist nicht berechtigt, die Verwalter*innen dieser Person zu ändern.
manageds_help: Du kannst nur Personen als Kinder hinzufügen, auf welche du Schreibzugriff hast.
nationality:
placeholder: z.B. CH

Expand Down
3 changes: 2 additions & 1 deletion lib/hitobito_youth/wagon.rb
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,8 @@ class Wagon < Rails::Engine
:_destroy],
people_manageds_attributes: [:id,
:managed_id,
:_destroy]]
:_destroy]
]
EventsController.permitted_attrs += [:tentative_applications]
Event::KindsController.permitted_attrs += [:kurs_id_fiver, :vereinbarungs_id_fiver]

Expand Down
27 changes: 27 additions & 0 deletions spec/abilities/person_ability_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,14 @@
end
end

it 'may not create households' do
is_expected.to_not be_able_to(:create_households, Person)
end

it 'may not lookup manageds' do
is_expected.to_not be_able_to(:lookup_manageds, Person)
end

it 'may access member if is manager of member' do
member.people_managers.create(manager_id: manager.id)
actions.each do |action|
Expand All @@ -34,6 +42,18 @@
end
end

context 'manager permission' do
before { member.people_managers.create(manager_id: manager.id) }

it 'may create households' do
is_expected.to be_able_to(:create_households, Person)
end

it 'may not lookup manageds' do
is_expected.to_not be_able_to(:lookup_manageds, Person)
end
end

context 'group leader permission to change managers' do
it 'cannot change managers' do
# precondition: no relation to member, so cannot update
Expand All @@ -43,6 +63,13 @@
is_expected.not_to be_able_to(:change_managers, member)
end

it 'may lookup manageds if has write permissions on managed person' do
Fabricate(Group::BottomLayer::Leader.name.to_sym,
group_id: member_role.group_id,
person: manager)
is_expected.to be_able_to(:lookup_manageds, Person)
end

it 'cannot change managers if only related to the managed person via manager relation' do
member.people_managers.create(manager_id: manager.id)
# precondition: is manager of member, so can update
Expand Down
69 changes: 69 additions & 0 deletions spec/controllers/people_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,75 @@

expect(pm).to be_present
end

context 'trying to create new person' do
context 'with feature gate disabled' do
before do
allow(FeatureGate).to receive(:enabled?).and_return(true)
allow(FeatureGate).to receive(:enabled?).with('people.people_managers.self_service_managed_creation').and_return(false)
end

it 'does not work' do
managed_attrs = {
people_manageds_attributes: {
'99' => {
managed_id: nil,
managed_attributes: {
first_name: 'Bob',
last_name: 'Miller',
birthday: '19.12.2002',
gender: 'w'
}
}
}
}

expect do
put :update, params: { id: subject.id,
group_id: subject.primary_group_id,
person: managed_attrs }
end.to_not change { Person.count }

expect(Person.exists?(first_name: 'Bob', last_name: 'Miller')).to eq(false)
end
end

context 'with feature gate enabled' do
before do
allow(FeatureGate).to receive(:enabled?).and_return(true)
allow(FeatureGate).to receive(:enabled?).with('people.people_managers.self_service_managed_creation').and_return(true)
end

it 'works' do
managed_attrs = {
people_manageds_attributes: {
'99' => {
managed_id: nil,
managed_attributes: {
first_name: 'Bob',
last_name: 'Miller',
birthday: '19.12.2002',
gender: 'w'
}
}
}
}

expect do
put :update, params: { id: subject.id,
group_id: subject.primary_group_id,
person: managed_attrs }
end.to change { Person.count }.by(1)
.and change { PeopleManager.count }.by(1)

managed = Person.find_by(first_name: 'Bob', last_name: 'Miller')

pm = PeopleManager.find_by(manager: subject, managed: managed)

expect(pm).to be_present
end
end
end
end

context 'as top_leader', versioning: true do
Expand Down
1 change: 1 addition & 0 deletions spec/features/people_manager_relation_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
let(:root) { people(:root) }
before do
allow(FeatureGate).to receive(:enabled?).with('people.people_managers').and_return(true)
allow(FeatureGate).to receive(:enabled?).with('people.people_managers.self_service_managed_creation').and_return(true)
allow(FeatureGate).to receive(:enabled?).with(:self_registration_reason).and_return(false)
allow_any_instance_of(FeatureGate).to receive(:enabled?).with(:self_registration_reason).and_return(false)
allow_any_instance_of(FeatureGate).to receive(:enabled?).with('people.people_managers').and_return(true)
Expand Down
Loading