Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Make error messages translatable #87

Closed
wants to merge 6 commits into
from

Conversation

Projects
None yet

knapo commented Feb 25, 2011

Error messages were hardococed. They should be defined in yml files and be translatable.

+1 on this request

@dasch dasch and 1 other commented on an outdated diff Jul 12, 2011

lib/active_merchant/common/validateable.rb
@@ -42,35 +43,45 @@ module ActiveMerchant #:nodoc:
# if more than one error is available we will only return the first. If no error is available
# we return an empty string
def on(field)
- self[field].to_a.first
+ type = [self[field]].flatten.first
+
+ return nil unless type
+
+ if type.is_a?(String)
+ key, default = nil, type
+ else
+ key, default = :"activemerchant.errors.models.#{@i18n_key}.attributes.#{field}.#{type}", :"activemerchant.errors.messages.#{type}"
@dasch

dasch Jul 12, 2011

Contributor

That's a pretty long line - perhaps split it in two?

@knapo

knapo Jul 12, 2011

Absolutely agree! Done.

Contributor

ntalbott commented Feb 27, 2012

Curious what the Shopify gang's take on this is - is this a good approach for ActiveMerchant? Don't want to ask @knapo for an updated PR if we're not interested in merging something like this.

At this point I'm ambivalent, though I could see this being handy for us at Spreedly eventually.

@ntalbott ntalbott referenced this pull request Feb 28, 2012

Closed

Support for i18n #254

Contributor

ZenCocoon commented May 7, 2012

That would be really useful to us at BookingSync as well. Looking forward to hear from Shopify guys ( @jduff @odorcicd ... ) on that.

michaek commented Jun 1, 2012

I think this is a good idea and a good implementation, but it seems like it can't be merged at this point and another PR will be needed.

Contributor

ZenCocoon commented Jun 1, 2012

@michaek I'm curious to know why this can't be merged. What would break ? The additional I18n dependency makes you trouble ?

michaek commented Jun 1, 2012

Maybe I'm wrong - I tried to run the patch against the current code and it couldn't find the chunks.

Contributor

ZenCocoon commented Jun 1, 2012

Hmm I see, I guess this patch might need some updates. It's is pretty old but I guess updating it is not the current issue, the main question remaining is whether this update have it's place into activemerchant.

michaek commented Jun 1, 2012

Yes, that is the question. I mean that I'm in favor of the change, but it will probably need a new pull request.

Contributor

ntalbott commented Jul 2, 2012

I would review and merge a fresh patch that tracked with how Rails does i18n, but it probably makes sense to wait to do that until we break backwards compatibility with < 3.0.

@ntalbott ntalbott closed this Jul 2, 2012

cawel commented Nov 17, 2012

ntalbott: any idea when active_merchant will support i18n so that we can have translatable error messages?

RKushnir commented Jan 2, 2013

@cawel 👍

troex commented Nov 1, 2013

This was really good implementation!

I18n can be useful for English users too (I'm not speaking about the rest of the world!). Recently needed to make field names more friendly like Your credit card number is invalid, hacked based on this idea:

module ActiveMerchant
  module Validateable
    class Errors < HashWithIndifferentAccess
      def full_messages
        result = []

        self.each do |key, messages|
          next if messages.blank?
          if key == 'base'
            result << "#{messages.first}"
          else
            # result << "#{key.to_s.humanize} #{messages.first}"
            object = @base.class.to_s.demodulize.underscore
            result << "%s %s" % [I18n.t("activemerchant.#{object}.#{key}", :default => key.to_s.humanize), messages.first]
          end
        end

        result
      end
    end
  end
end

sunny commented Nov 14, 2013

+1 !

👍 any news on this?

+1 !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment