Ruby on Rails | Screencasts | Download | Documentation | Weblog | Community | Source

Ticket #8270 (closed enhancement: fixed)

Opened 2 years ago

Last modified 2 years ago

[PATCH] More options for error_messages_for

Reported by: zach-inglis-lt3 Assigned to: nzkoz
Priority: normal Milestone: 2.0
Component: ActionPack Version: edge
Severity: minor Keywords: error_messages_for verified
Cc:

Description (Last modified by nzkoz)

I want to give more options to people using error_messages_for

I suggest:

module ActionView
    module Helpers
      module ActiveRecordHelper
        def error_messages_for(*params)
          options = params.last.is_a?(Hash) ? params.pop.symbolize_keys : {}
          objects = params.collect {|object_name| instance_variable_get("@#{object_name}") }.compact
          count   = objects.inject(0) {|sum, object| sum + object.errors.count }
          unless count.zero?
            html = {}
            [:id, :class].each do |key|
              if options.include?(key)
                value = options[key]
                html[key] = value unless value.blank?
              else
                html[key] = 'errorExplanation'
              end
            end
            header_message = "#{pluralize(count, 'error')} prohibited this #{(options[:object_name] || params.first).to_s.gsub('_', ' ')} from being saved"
            error_messages = objects.map {|object| object.errors.full_messages.map {|msg| content_tag(:li, msg) } }
            content_tag(:div,
              content_tag(options[:header_tag] || :h2, options[:header_message] || header_message) <<
                content_tag(:p, options[:explanation] || 'There were problems with the following fields:') <<
                content_tag(:ul, error_messages),
              html
            )
          else
            ''
          end
        end        
      end
...
}}

Attachments

more_options_on_error_messages_for_with_tests.diff (4.2 kB) - added by rmm5t on 09/16/07 01:17:33.
New :header_message and :message options. Rdoc and tests included. (rebased)
more_options_on_error_messages_for_with_tests_rebased.diff (4.6 kB) - added by rmm5t on 10/14/07 00:15:35.
Rebased after Koz's suggestion

Change History

05/05/07 08:55:35 changed by zach-inglis-lt3

  • type changed from defect to enhancement.

05/06/07 17:47:11 changed by rmm5t

  • component changed from ActiveRecord to ActionPack.

I was coincidentally looking into a very similar enhancement. The original request could use a little more umph, so I added the attached patch that allows for :header_message and :message options passed to error_messages_for. You can also pass nil or an empty string to avoid one of the messages altogether.

I found that this patch helps avoid writing custom error handling using object.errors when a specific output is desired.

05/06/07 18:35:11 changed by rmm5t

Here's an example of how one could override the behavior of error_messages_for after this patch is applied:

  def error_messages_for_with_custom(*params)
    options = params.last.is_a?(Hash) ? params.pop.symbolize_keys : {}
    options.reverse_merge!(:header_message => 'Yikes!', :message => nil)
    error_messages_for_without_custom(params, options)
  end
  alias_method_chain :error_messages_for, :custom

(could be added to application_helper.rb)

05/07/07 02:30:04 changed by rmm5t

  • summary changed from More options for error_messages_for to [PATCH] More options for error_messages_for.
  • milestone changed from 1.x to 1.2.4.

07/09/07 10:12:29 changed by zach-inglis-lt3

  • priority changed from low to normal.

07/09/07 10:13:25 changed by zach-inglis-lt3

  • keywords set to error_messages_for.

09/11/07 02:51:44 changed by rmm5t

+1 (to hopefully get this moving)

09/16/07 01:17:33 changed by rmm5t

  • attachment more_options_on_error_messages_for_with_tests.diff added.

New :header_message and :message options. Rdoc and tests included. (rebased)

09/21/07 16:52:21 changed by chmurph2

+1. Works.

09/25/07 16:45:41 changed by tmass

+1

Patch applied cleanly. Test runs.

09/25/07 17:18:13 changed by tmass

  • keywords changed from error_messages_for to error_messages_for verified.

10/13/07 23:30:53 changed by nzkoz

  • description changed.

10/13/07 23:35:58 changed by nzkoz

  • keywords changed from error_messages_for verified to error_messages_for.
  • owner changed from core to nzkoz.
  • status changed from new to assigned.
  • summary changed from [PATCH] More options for error_messages_for to [XPATCH] More options for error_messages_for.

Sorry for the delay in responding to this.

It doesn't seem to apply cleanly as we've added a few new features to error_messages_for.

The two features certainly look interesting, if you can rebase this I'll take another look.

Finally, I wouldn't advise using alias_method_chain to provide your example. Simple delegation would work fine :).

10/13/07 23:42:33 changed by bitsweat

  • status changed from assigned to closed.
  • resolution set to wontfix.

Everyone displays errors differently. Writing your own helper method, which may wrap errors_messages_for, is the way to go here.

10/14/07 00:15:35 changed by rmm5t

  • attachment more_options_on_error_messages_for_with_tests_rebased.diff added.

Rebased after Koz's suggestion

10/14/07 00:21:26 changed by rmm5t

  • keywords changed from error_messages_for to error_messages_for verified.
  • status changed from closed to reopened.
  • summary changed from [XPATCH] More options for error_messages_for to [PATCH] More options for error_messages_for.
  • resolution deleted.
  • milestone changed from 1.2.5 to 2.0.

Koz, I rebased as you requested. You're right about delegation. Thanks.

Jeremy, I understand your sentiments, but it's difficult to just wrap error_messages_for without duplicating a bunch of the logic that already exists in error_messages_for. I agree that everyone displays errors differently, but with these proposed changes, I've been able to reuse error_messages_for and display errors differently with just CSS. Thanks for the feedback either way.

10/14/07 02:09:50 changed by bitsweat

Ok, I do agree. In the future, please don't change ticket resolution just because you wish it wasn't wontfixed, though.

10/14/07 02:18:28 changed by rmm5t

Jeremy, Sorry, my bad. Understood.

10/14/07 03:01:44 changed by bitsweat

  • status changed from reopened to closed.
  • resolution set to fixed.