change validation of WP children

- do not add error on a symbol called :#123 (123 is the childrens ID)
    - this is against the semantics of ActiveRecord::Errors
    - creating symbols in a loop used to be a bad idea (might be better today)
    - it breaks error messages in the APIv3 (which is due to APIv3 working beyond the borders of AR errors, not the best of ideas...)
- use :base instead, the place for unlocatable errors
- include the fact that #123 refers to a child element in the error message

Basically I feel that this makes the children validation cleaner and better. But somehow I fear, that there is some code out there that depends on the fact that error keys like :#123 existed...
pull/3166/head
Jan Sandbrink 9 years ago
parent 0dd77ecb5f
commit 17940bf02c
  1. 5
      app/models/work_package/validations.rb
  2. 1
      config/locales/en.yml
  3. 4
      spec/requests/api/v3/work_package_resource_spec.rb

@ -125,8 +125,9 @@ module WorkPackage::Validations
def validate_children def validate_children
children.select { |c| !c.valid? }.each do |child| children.select { |c| !c.valid? }.each do |child|
child.errors.each do |_, value| child.errors.each do |attribute, message|
errors.add(:"##{child.id}", value) full_message = child.errors.full_message(attribute, message)
errors.add(:base, "#{I18n.t('label_child_element')} ##{child.id}: #{full_message}")
end end
end end
end end

@ -674,6 +674,7 @@ en:
label_changes_details: "Details of all changes" label_changes_details: "Details of all changes"
label_changeset_plural: "Changesets" label_changeset_plural: "Changesets"
label_checked: "checked" label_checked: "checked"
label_child_element: "Child element"
label_chronological_order: "In chronological order" label_chronological_order: "In chronological order"
label_close_versions: "Close completed versions" label_close_versions: "Close completed versions"
label_closed_work_packages: "closed" label_closed_work_packages: "closed"

@ -897,7 +897,9 @@ h4. things we like
it_behaves_like 'multiple errors of the same type with messages' do it_behaves_like 'multiple errors of the same type with messages' do
let(:message) { let(:message) {
[child_1.id, child_2.id].map { |id| "##{id} cannot be in another project." } [child_1.id, child_2.id].map { |id|
"Child element ##{id}: Parent cannot be in another project."
}
} }
end end
end end

Loading…
Cancel
Save