Defect #28925
Custom field values for enumerations not saved
| Status: | Closed | Start date: | ||
|---|---|---|---|---|
| Priority: | Normal | Due 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:
- Add a custom field for activities (time tracking)
- Edit any activity (time tracking) in Enumerations
- Try to save a value for the previously created custom field (it won't be saved)
fix_28925.patch 
(2.97 KB)
fix_28925_v2.patch
(2.99 KB)
fix_28925_v3.patch
- updated Mizuki Ishikawa's patch (according to #28925#note-5)
(2.94 KB)
Related issues
Associated revisions
Custom field values for enumerations not saved (#28925).
Patch by Mizuki ISHIKAWA.
Create fixture correctly.
History
#1
Updated by Go MAEDA about 2 years 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 about 2 years ago
- Related to Defect #26564: Enumerations sorting does not work added
#3
Updated by Mizuki ISHIKAWA almost 2 years ago
- File fix_28925.patch added
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 almost 2 years ago
- File fix_28925_v2.patch added
I changed the test a little.
#5
Updated by Go MAEDA almost 2 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
#6
Updated by Pavel Rosický almost 2 years ago
why enumerations use strong params instead of safe_attributes like all other models do?
#7
Updated by Mizuki ISHIKAWA almost 2 years ago
Go MAEDA wrote:
Maybe we can use
available_custom_fieldsto 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 almost 2 years ago
- File fix_28925_v3.patch 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 over 1 year 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 over 1 year ago
Patch broke 3.4-stable, fixed.