[27833] Fix selecting multi CF value from filters

https://community.openproject.com/wp/27833

frontend/app/components/wp-edit-form/work-package-changeset.ts

@@ -53,14 +53,14 @@ export class WorkPackageChangeset {
   public wpEditing:WorkPackageEditingService;
 
   // The changeset to be applied to the work package
-  private changes:{[attribute:string]:any} = {};
+  private changes:{ [attribute:string]:any } = {};
   public inFlight:boolean = false;
 
   // The current work package form
   public wpForm = input<FormResourceInterface>();
 
   // The current editing resource
-  public resource:WorkPackageResourceInterface|null;
+  public resource:WorkPackageResourceInterface | null;
 
   constructor(public workPackage:WorkPackageResourceInterface, form?:FormResourceInterface) {
     $injectFields(
@@ -139,7 +139,7 @@ export class WorkPackageChangeset {
     if (this.wpForm.hasValue()) {
       return Promise.resolve(this.wpForm.value!);
     } else {
-      return new Promise((resolve, ) => this.wpForm.valuesPromise().then(resolve));
+      return new Promise((resolve,) => this.wpForm.valuesPromise().then(resolve));
     }
   }
 
@@ -270,31 +270,41 @@ export class WorkPackageChangeset {
   }
 
   private get minimalPayload() {
-    return { lockVersion: this.workPackage.lockVersion, _links: {} };
+    return {lockVersion: this.workPackage.lockVersion, _links: {}};
   }
 
   /**
    * Extract the link(s) in the given changed value
    */
   private getLinkedValue(val:any, fieldSchema:op.FieldSchema) {
-    var isArray = (fieldSchema.type || '').startsWith('[]');
+    // Links should always be nullified as { href: null }, but
+    // this wasn't always the case, so ensure null values are returned as such.
+    if (_.isNil(val)) {
+      return {href: null};
+    }
 
-    if (isArray) {
-      var links:{ href:string }[] = [];
+    // Test if we either have a CollectionResource or a HAL array,
+    // or a single hal value.
+    let isArrayType = (fieldSchema.type || '').startsWith('[]');
+    let isArray = false;
 
-      if (val) {
-        var elements = (val.forEach && val) || val.elements;
+    if (val.forEach || val.elements) {
+      isArray = true;
+    }
 
-        elements.forEach((link:{ href:string }) => {
-          if (link.href) {
-            links.push({href: link.href});
-          }
-        });
-      }
+    if (isArray && isArrayType) {
+      var links:{ href:string }[] = [];
+      var elements = val.forEach ? val : val.elements;
+
+      elements.forEach((link:{ href:string }) => {
+        if (link.href) {
+          links.push({href: link.href});
+        }
+      });
 
       return links;
     } else {
-      return { href: _.get(val, 'href', null) };
+      return {href: _.get(val, 'href', null)};
     }
   }
 

spec/features/work_packages/new/attributes_from_filter_spec.rb

@@ -44,13 +44,7 @@ RSpec.feature 'Work package create uses attributes from filters', js: true, sele
   let(:filters) { ::Components::WorkPackages::Filters.new }
 
   let(:role) { FactoryGirl.create :existing_role, permissions: [:view_work_packages] }
-  let!(:assignee) do
-    FactoryGirl.create(:user,
-                      firstname: 'An',
-                      lastname: 'assignee',
-                      member_in_project: project,
-                      member_through_role: role)
-  end
+
 
   let!(:query) do
     FactoryGirl.build(:query, project: project, user: user).tap do |query|
@@ -68,63 +62,120 @@ RSpec.feature 'Work package create uses attributes from filters', js: true, sele
 
     filters.expect_filter_count 0
     filters.open
-    filters.add_filter_by('Assignee', 'is', assignee.name)
     filters.add_filter_by('Type', 'is', type_task.name)
   end
 
-  it 'uses the assignee filter in inline-create and split view' do
-    wp_table.click_inline_create
+  context 'with a multi-value custom field' do
+    let(:type_task) { FactoryGirl.create(:type_task, custom_fields: [custom_field]) }
+    let!(:project) {
+      FactoryGirl.create :project,
+                         types: [type_task],
+                         work_package_custom_fields: [custom_field]
+    }
+
+    let!(:custom_field) do
+      FactoryGirl.create(
+        :list_wp_custom_field,
+        multi_value: true,
+        is_filter: true,
+        name: "Gate",
+        possible_values: %w(A B C),
+
+        is_required: false
+      )
+    end
+
+    before do
+      filters.add_filter_by('Gate', 'is', 'A', "customField#{custom_field.id}")
+    end
+
+    it 'allows to save with a single value (Regression test #27833)' do
+      split_page = wp_table.create_wp_split_screen type_task.name
+
+      subject = split_page.edit_field(:subject)
+      subject.expect_active!
+      subject.set_value 'Foobar!'
+      subject.submit_by_enter
+
+      wp_table.expect_notification(
+        message: 'Successful creation. Click here to open this work package in fullscreen view.'
+      )
+      wp_table.dismiss_notification!
+      wp = WorkPackage.last
+      expect(wp.subject).to eq 'Foobar!'
+      expect(wp.send("custom_field_#{custom_field.id}")).to eq %w(A)
+      expect(wp.type_id).to eq type_task.id
+    end
+  end
+
+  context 'with assignee filter' do
+    let!(:assignee) do
+      FactoryGirl.create(:user,
+                         firstname: 'An',
+                         lastname: 'assignee',
+                         member_in_project: project,
+                         member_through_role: role)
+    end
 
-    subject_field = wp_table.edit_field(nil, :subject)
-    subject_field.expect_active!
 
-    # Expect assignee to be set to the current user
-    assignee_field = wp_table.edit_field(nil, :assignee)
-    assignee_field.expect_state_text assignee.name
+    before do
+      filters.add_filter_by('Assignee', 'is', assignee.name)
+    end
 
-    # Expect type set to task
-    assignee_field = wp_table.edit_field(nil, :type)
-    assignee_field.expect_state_text type_task.name
+    it 'uses the assignee filter in inline-create and split view' do
+      wp_table.click_inline_create
 
-    # Save the WP
-    subject_field.set_value 'Foobar!'
-    subject_field.submit_by_enter
+      subject_field = wp_table.edit_field(nil, :subject)
+      subject_field.expect_active!
 
-    wp_table.expect_notification(
-      message: 'Successful creation. Click here to open this work package in fullscreen view.'
-    )
-    wp_table.dismiss_notification!
+      # Expect assignee to be set to the current user
+      assignee_field = wp_table.edit_field(nil, :assignee)
+      assignee_field.expect_state_text assignee.name
 
-    wp = WorkPackage.last
-    expect(wp.subject).to eq 'Foobar!'
-    expect(wp.assigned_to_id).to eq assignee.id
-    expect(wp.type_id).to eq type_task.id
+      # Expect type set to task
+      assignee_field = wp_table.edit_field(nil, :type)
+      assignee_field.expect_state_text type_task.name
 
-    # Open split view create
-    split_view_create.click_create_wp_button(type_bug)
+      # Save the WP
+      subject_field.set_value 'Foobar!'
+      subject_field.submit_by_enter
 
-    # Subject
-    subject_field = split_view_create.edit_field :subject
-    subject_field.expect_active!
-    subject_field.set_value 'Split Foobar!'
+      wp_table.expect_notification(
+        message: 'Successful creation. Click here to open this work package in fullscreen view.'
+      )
+      wp_table.dismiss_notification!
 
-    # Type field IS NOT synced
-    type_field = split_view_create.edit_field :type
-    type_field.expect_state_text type_bug
+      wp = WorkPackage.last
+      expect(wp.subject).to eq 'Foobar!'
+      expect(wp.assigned_to_id).to eq assignee.id
+      expect(wp.type_id).to eq type_task.id
 
-    # Assignee is synced
-    assignee_field = split_view_create.edit_field :assignee
-    assignee_field.expect_value "/api/v3/users/#{assignee.id}"
+      # Open split view create
+      split_view_create.click_create_wp_button(type_bug)
 
-    within '.work-packages--edit-actions' do
-      click_button 'Save'
-    end
+      # Subject
+      subject_field = split_view_create.edit_field :subject
+      subject_field.expect_active!
+      subject_field.set_value 'Split Foobar!'
 
-    wp_table.expect_notification(message: 'Successful creation.')
+      # Type field IS NOT synced
+      type_field = split_view_create.edit_field :type
+      type_field.expect_state_text type_bug
 
-    wp = WorkPackage.last
-    expect(wp.subject).to eq 'Split Foobar!'
-    expect(wp.assigned_to_id).to eq assignee.id
-    expect(wp.type_id).to eq type_bug.id
+      # Assignee is synced
+      assignee_field = split_view_create.edit_field :assignee
+      assignee_field.expect_value "/api/v3/users/#{assignee.id}"
+
+      within '.work-packages--edit-actions' do
+        click_button 'Save'
+      end
+
+      wp_table.expect_notification(message: 'Successful creation.')
+
+      wp = WorkPackage.last
+      expect(wp.subject).to eq 'Split Foobar!'
+      expect(wp.assigned_to_id).to eq assignee.id
+      expect(wp.type_id).to eq type_bug.id
+    end
   end
 end