From 14a6d56d7b8c70f9d697a2e42c7725a4281c6cef Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Wed, 28 Feb 2024 15:11:35 +0200 Subject: [PATCH 1/2] [#32940] Unsetting a user value in bulk is not possible https://community.openproject.org/work_packages/32940 --- .../work_packages/bulk_controller.rb | 15 +-- app/helpers/custom_fields_helper.rb | 12 ++- .../bulk/update_work_package_spec.rb | 95 +++++++++++++++++++ 3 files changed, 113 insertions(+), 9 deletions(-) diff --git a/app/controllers/work_packages/bulk_controller.rb b/app/controllers/work_packages/bulk_controller.rb index b0495d346c58..d834d0ec3829 100644 --- a/app/controllers/work_packages/bulk_controller.rb +++ b/app/controllers/work_packages/bulk_controller.rb @@ -109,12 +109,9 @@ def destroy_work_packages(work_packages) def attributes_for_update return {} unless params.has_key? :work_package - permitted_params - .update_work_package - .tap { |attributes| attributes[:custom_field_values]&.reject! { |_k, v| v.blank? } } - .compact_blank - .transform_values { |v| v == 'none' ? '' : v } - .to_h + attributes = permitted_params.update_work_package + attributes[:custom_field_values] = transform_attributes(attributes[:custom_field_values]) + transform_attributes(attributes) end def user @@ -124,4 +121,10 @@ def user def default_breadcrumb I18n.t(:label_work_package_plural) end + + def transform_attributes(attributes) + Hash(attributes) + .compact_blank + .transform_values { |v| Array(v).include?('none') ? '' : v } + end end diff --git a/app/helpers/custom_fields_helper.rb b/app/helpers/custom_fields_helper.rb index e72b411f4676..8cf2720cc997 100644 --- a/app/helpers/custom_fields_helper.rb +++ b/app/helpers/custom_fields_helper.rb @@ -153,6 +153,7 @@ def custom_field_tag_for_bulk_edit(name, custom_field, project = nil) field_name = "#{name}[custom_field_values][#{custom_field.id}]" field_id = "#{name}_custom_field_values_#{custom_field.id}" field_format = OpenProject::CustomFieldFormat.find_by_name(custom_field.field_format) + case field_format.try(:edit_as) when 'date' angular_component_tag 'op-modal-single-date-picker', @@ -164,12 +165,17 @@ def custom_field_tag_for_bulk_edit(name, custom_field, project = nil) styled_text_area_tag(field_name, '', id: field_id, rows: 3, with_text_formatting: true) when 'bool' styled_select_tag(field_name, options_for_select([[I18n.t(:label_no_change_option), ''], + ([I18n.t(:label_none), 'none'] unless custom_field.required?), [I18n.t(:general_text_yes), '1'], - [I18n.t(:general_text_no), '0']]), id: field_id) + [I18n.t(:general_text_no), '0']].compact), id: field_id) when 'list' + base_options = [[I18n.t(:label_no_change_option), '']] + unless custom_field.required? + unset_label = custom_field.field_format == 'user' ? :label_nobody : :label_none + base_options << [I18n.t(unset_label), 'none'] + end styled_select_tag(field_name, - options_for_select([[I18n.t(:label_no_change_option), - '']] + custom_field.possible_values_options(project)), + options_for_select(base_options + custom_field.possible_values_options(project)), id: field_id, multiple: custom_field.multi_value?) else diff --git a/spec/features/work_packages/bulk/update_work_package_spec.rb b/spec/features/work_packages/bulk/update_work_package_spec.rb index 4795fd1734d8..55f4629fe373 100644 --- a/spec/features/work_packages/bulk/update_work_package_spec.rb +++ b/spec/features/work_packages/bulk/update_work_package_spec.rb @@ -204,6 +204,101 @@ end end + describe 'unsetting values for different fields' do + let(:boolean_cf) do + create(:boolean_wp_custom_field, + name: 'Bool CF', + types: [type], + projects: [project]) + end + let(:required_boolean_cf) do + create(:boolean_wp_custom_field, + name: 'Required Bool CF', + types: [type], + projects: [project], + is_required: true) + end + let(:list_cf) do + create(:list_wp_custom_field, + name: 'List CF', + types: [type], + projects: [project], + possible_values: %w[A B C]) + end + let(:required_list_cf) do + create(:list_wp_custom_field, + name: 'Required List CF', + types: [type], + projects: [project], + possible_values: %w[A B C], + is_required: true) + end + let(:multi_list_cf) do + create(:list_wp_custom_field, :multi_list, + name: 'Multi select List CF', + types: [type], + projects: [project], + possible_values: %w[A B C]) + end + let(:user_cf) do + create(:user_wp_custom_field, + name: 'User CF', + types: [type], + projects: [project]) + end + let(:multi_user_cf) do + create(:user_wp_custom_field, :multi_user, + name: 'Multi user CF', + types: [type], + projects: [project]) + end + + let(:default_cf_values) do + { + boolean_cf.id => true, + required_boolean_cf.id => false, + list_cf.id => list_cf.custom_options.find_by(value: "B"), + required_list_cf.id => required_list_cf.custom_options.find_by(value: "B"), + multi_list_cf.id => multi_list_cf.custom_options.find_by(value: "B"), + user_cf.id => dev, + multi_user_cf.id => [dev, mover] + } + end + + before do + [work_package, work_package2].each do |wp| + wp.update!(custom_field_values: default_cf_values) + end + + refresh + wait_for_reload + end + + it 'clears the chosen values' do + # Required fields should not have a 'none' option + expect(page).to have_no_select(required_boolean_cf.name, options: ['none']) + expect(page).to have_no_select(required_list_cf.name, options: ['none']) + + # Unset any non-required fields + select 'none', from: boolean_cf.name + select 'none', from: list_cf.name + select 'none', from: multi_list_cf.name + select 'nobody', from: user_cf.name + select 'nobody', from: multi_user_cf.name + + click_on 'Submit' + + expect_angular_frontend_initialized + wp_table.expect_work_package_count 2 + + # It clears all the values except the required fields + expect(work_package.reload.custom_field_values.pluck(:value).compact) + .to eq(['f', required_list_cf.custom_options.find_by(value: "B").id.to_s]) + expect(work_package2.reload.custom_field_values.pluck(:value).compact) + .to eq(['f', required_list_cf.custom_options.find_by(value: "B").id.to_s]) + end + end + context 'when custom fields are removed from types' do it 'does not display them on the form' do expect(page).to have_field custom_field_removed.name From 5dc12a8e25bad0afc6caf27793787b1361dc1b37 Mon Sep 17 00:00:00 2001 From: Dombi Attila <83396+dombesz@users.noreply.github.com> Date: Mon, 4 Mar 2024 20:35:53 +0200 Subject: [PATCH 2/2] Update select matcher to use `with_options` instead of `options` The `options` was incorrect, because it would try to match a select with the single specified option. The intention here was to find the select that contains the specified option. Hence the `with_options` matcher is the correct one. Co-authored-by: Aaron Contreras <61627014+aaron-contreras@users.noreply.github.com> --- spec/features/work_packages/bulk/update_work_package_spec.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/features/work_packages/bulk/update_work_package_spec.rb b/spec/features/work_packages/bulk/update_work_package_spec.rb index 55f4629fe373..7f07d525b897 100644 --- a/spec/features/work_packages/bulk/update_work_package_spec.rb +++ b/spec/features/work_packages/bulk/update_work_package_spec.rb @@ -276,8 +276,8 @@ it 'clears the chosen values' do # Required fields should not have a 'none' option - expect(page).to have_no_select(required_boolean_cf.name, options: ['none']) - expect(page).to have_no_select(required_list_cf.name, options: ['none']) + expect(page).to have_no_select(required_boolean_cf.name, with_options: ['none']) + expect(page).to have_no_select(required_list_cf.name, with_options: ['none']) # Unset any non-required fields select 'none', from: boolean_cf.name