Project

General

Profile

Actions

Defect #28925

closed

Custom field values for enumerations not saved

Added by Andriy Lesyuk almost 6 years ago. Updated over 5 years ago.

Status:
Closed
Priority:
Normal
Category:
Custom fields
Target version:
Start date:
Due date:
% Done:

0%

Estimated time:
Resolution:
Fixed
Affected version:

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)

Files

fix_28925.patch (2.97 KB) fix_28925.patch Mizuki ISHIKAWA, 2018-06-18 07:43
fix_28925_v2.patch (2.99 KB) fix_28925_v2.patch Mizuki ISHIKAWA, 2018-06-18 08:17
fix_28925_v3.patch (2.94 KB) fix_28925_v3.patch updated Mizuki Ishikawa's patch (according to #28925#note-5) Go MAEDA, 2018-06-19 02:47

Related issues

Related to Redmine - Defect #26564: Enumerations sorting does not workClosedJean-Philippe Lang

Actions
Actions #1

Updated by Go MAEDA almost 6 years ago

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

Updated by Go MAEDA almost 6 years ago

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

Updated by Mizuki ISHIKAWA almost 6 years ago

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

Actions #4

Updated by Mizuki ISHIKAWA almost 6 years ago

I changed the test a little.

Actions #5

Updated by Go MAEDA almost 6 years 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
Actions #6

Updated by Pavel Rosický over 5 years ago

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

Actions #7

Updated by Mizuki ISHIKAWA over 5 years 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.

Actions #8

Updated by Go MAEDA over 5 years ago

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.

Actions #9

Updated by Jean-Philippe Lang over 5 years ago

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

Committed, thanks.

Actions #10

Updated by Jean-Philippe Lang over 5 years ago

Patch broke 3.4-stable, fixed.

Actions

Also available in: Atom PDF