Defect #28925
closed
Custom field values for enumerations not saved
0%
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)
Files
Related issues
Updated by Go MAEDA about 7 years ago
- Status changed from New to Confirmed
- Priority changed from Low to Normal
- Target version set to Candidate for next minor release
Updated by Go MAEDA about 7 years ago
- Related to Defect #26564: Enumerations sorting does not work added
Updated by Mizuki ISHIKAWA about 7 years ago
- File fix_28925.patch 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.
Updated by Mizuki ISHIKAWA about 7 years ago
- File fix_28925_v2.patch fix_28925_v2.patch added
I changed the test a little.
Updated by Go MAEDA about 7 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
Updated by Pavel Rosický about 7 years ago
why enumerations use strong params instead of safe_attributes like all other models do?
Updated by Mizuki ISHIKAWA about 7 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.
Updated by Go MAEDA about 7 years ago
- File fix_28925_v3.patch 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.
Updated by Jean-Philippe Lang almost 7 years ago
- Status changed from Confirmed to Closed
- Assignee set to Jean-Philippe Lang
- Resolution set to Fixed
Committed, thanks.
Updated by Jean-Philippe Lang almost 7 years ago
Patch broke 3.4-stable, fixed.