Defect #28925

Custom field values for enumerations not saved

Added by Andriy Lesyuk 5 months ago. Updated about 1 month ago.

Status:ClosedStart date:
Priority:NormalDue date:
Assignee:Jean-Philippe Lang% Done:

0%

Category:Custom fields
Target version:3.4.7
Resolution:Fixed Affected version:3.4.5

Description

The Enumerations controller drops custom_field_values from params, therefore they are never saved. This is done in the code:

  def enumeration_params
    # can't require enumeration on #new action
    params.permit(:enumeration => [:name, :active, :is_default, :position])[:enumeration]
  end

Here :custom_field_values is missing.

Steps to reproduce:

  1. Add a custom field for activities (time tracking)
  2. Edit any activity (time tracking) in Enumerations
  3. Try to save a value for the previously created custom field (it won't be saved)

fix_28925.patch Magnifier (2.97 KB) Mizuki ISHIKAWA, 2018-06-18 07:43

fix_28925_v2.patch Magnifier (2.99 KB) Mizuki ISHIKAWA, 2018-06-18 08:17

fix_28925_v3.patch Magnifier - updated Mizuki Ishikawa's patch (according to #28925#note-5) (2.94 KB) Go MAEDA, 2018-06-19 02:47


Related issues

Related to Redmine - Defect #26564: Enumerations sorting does not work Closed

Associated revisions

Revision 17484
Added by Jean-Philippe Lang about 1 month ago

Custom field values for enumerations not saved (#28925).

Patch by Mizuki ISHIKAWA.

Revision 17485
Added by Jean-Philippe Lang about 1 month ago

Merged r17484 to 3.4-stable (#28925).

Revision 17488
Added by Jean-Philippe Lang about 1 month ago

Create fixture correctly.

Revision 17489
Added by Jean-Philippe Lang about 1 month ago

Merged r17488 to 3.4-stable (#28925).

History

#1 Updated by Go MAEDA 5 months ago

  • Status changed from New to Confirmed
  • Priority changed from Low to Normal
  • Target version set to Candidate for next minor release

#2 Updated by Go MAEDA 5 months ago

  • Related to Defect #26564: Enumerations sorting does not work added

#3 Updated by Mizuki ISHIKAWA 4 months ago

I wrote a patch to fix this defect.
The key of custom_field_values allows only id of custom field of enumeration.

#4 Updated by Mizuki ISHIKAWA 4 months ago

I changed the test a little.

#5 Updated by Go MAEDA 4 months ago

Maybe we can use available_custom_fields to collect ids of custom fields. I think the following change can make the patch simpler and faster. Mizuki, do you think it is correct?

--- app/controllers/enumerations_controller.rb.orig    2018-06-18 15:28:18.000000000 +0000
+++ app/controllers/enumerations_controller.rb    2018-06-18 15:28:36.000000000 +0000
@@ -107,7 +107,7 @@

   def enumeration_params
     # can't require enumeration on #new action
-    custom_field_keys = (@enumeration.try(:custom_field_values) || []).map{|c| c.custom_field_id.to_s}
+    custom_field_keys = @enumeration.available_custom_fields.map{|c| c.id.to_s}
     params.permit(:enumeration => [:name, :active, :is_default, :position, :custom_field_values => custom_field_keys])[:enumeration]
   end
 end

#6 Updated by Pavel Rosický 4 months ago

why enumerations use strong params instead of safe_attributes like all other models do?

#7 Updated by Mizuki ISHIKAWA 4 months ago

Go MAEDA wrote:

Maybe we can use available_custom_fields to collect ids of custom fields. I think the following change can make the patch simpler and faster. Mizuki, do you think it is correct?

[...]

I did not know available_custom_fields. I think that the code you propose is good.

Pavel Rosický wrote:

why enumerations use strong params instead of safe_attributes like all other models do?

I do not know the reason, It seems that it was intentionally changed to use strong_params instead of safe_attribute in r16602 and r16603.

#8 Updated by Go MAEDA 4 months ago

  • File fix_28925_v3.patchMagnifier added
  • Target version changed from Candidate for next minor release to 3.4.7

Mizuki ISHIKAWA wrote:

I did not know available_custom_fields. I think that the code you propose is good.

Thank you for reviewing the change I suggested in #28925#note-5. I have slightly changed your patch.

Setting target version to 3.4.7.

#9 Updated by Jean-Philippe Lang about 1 month ago

  • Status changed from Confirmed to Closed
  • Assignee set to Jean-Philippe Lang
  • Resolution set to Fixed

Committed, thanks.

#10 Updated by Jean-Philippe Lang about 1 month ago

Patch broke 3.4-stable, fixed.

Also available in: Atom PDF