diff --git a/app/abilities/youth/person_ability.rb b/app/abilities/youth/person_ability.rb index cdb70012..afb3adaf 100644 --- a/app/abilities/youth/person_ability.rb +++ b/app/abilities/youth/person_ability.rb @@ -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 @@ -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 diff --git a/app/controllers/youth/people_controller.rb b/app/controllers/youth/people_controller.rb index 1a24fc4e..f0953139 100644 --- a/app/controllers/youth/people_controller.rb +++ b/app/controllers/youth/people_controller.rb @@ -33,6 +33,25 @@ 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) @@ -40,7 +59,7 @@ def permitted_params 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 @@ -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 diff --git a/app/models/people_manager.rb b/app/models/people_manager.rb index c59ba549..c5e1996c 100644 --- a/app/models/people_manager.rb +++ b/app/models/people_manager.rb @@ -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 diff --git a/app/views/people/_fields_youth.html.haml b/app/views/people/_fields_youth.html.haml index 5bdaafb2..c3d40cf4 100644 --- a/app/views/people/_fields_youth.html.haml +++ b/app/views/people/_fields_youth.html.haml @@ -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, ' '.html_safe) do - %span.help-block - = t('.manageds_help') = field_set_tag do = f.labeled_input_fields(:ahv_number, :j_s_number) diff --git a/app/views/people/manageds/_create_new_managed_person.html.haml b/app/views/people/manageds/_create_new_managed_person.html.haml new file mode 100644 index 00000000..6f083782 --- /dev/null +++ b/app/views/people/manageds/_create_new_managed_person.html.haml @@ -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') diff --git a/app/views/people/manageds/_fields.html.haml b/app/views/people/manageds/_fields.html.haml index a32d107f..da9be1d5 100644 --- a/app/views/people/manageds/_fields.html.haml +++ b/app/views/people/manageds/_fields.html.haml @@ -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 \ No newline at end of file + = f.object.managed.full_name diff --git a/config/locales/views.youth.de.yml b/config/locales/views.youth.de.yml index 9a9f715d..d275878f 100644 --- a/config/locales/views.youth.de.yml +++ b/config/locales/views.youth.de.yml @@ -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 diff --git a/lib/hitobito_youth/wagon.rb b/lib/hitobito_youth/wagon.rb index dc729b0f..18b795d2 100644 --- a/lib/hitobito_youth/wagon.rb +++ b/lib/hitobito_youth/wagon.rb @@ -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] diff --git a/spec/abilities/person_ability_spec.rb b/spec/abilities/person_ability_spec.rb index 9791b0fb..b97dc26e 100644 --- a/spec/abilities/person_ability_spec.rb +++ b/spec/abilities/person_ability_spec.rb @@ -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| @@ -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 @@ -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 diff --git a/spec/controllers/people_controller_spec.rb b/spec/controllers/people_controller_spec.rb index 9834f6e6..a4bc261c 100644 --- a/spec/controllers/people_controller_spec.rb +++ b/spec/controllers/people_controller_spec.rb @@ -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 diff --git a/spec/features/people_manager_relation_spec.rb b/spec/features/people_manager_relation_spec.rb index 83c18277..5b11aa62 100644 --- a/spec/features/people_manager_relation_spec.rb +++ b/spec/features/people_manager_relation_spec.rb @@ -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)