optimism icon indicating copy to clipboard operation
optimism copied to clipboard

Adds a raw error message config option

Open mepatterson opened this issue 5 years ago • 11 comments

In the case where you might want Optimism to only send the literal error message string (without the attribute name humanized in front of it), you need to not go through full_messages since that prefixes the error message with the attribute.

This PR adds an optional config switch (default: false) to tell Optimism to just send the 'raw' error message for an invalid attribute. (e.g. instead of sending "Name is too short" it will send "is too short" which can be useful if the consumer is manually constructing their own messages.

mepatterson avatar Jul 06 '20 03:07 mepatterson

Hey, thanks for this - it's interesting.

First, I need you to read through this PR: https://github.com/leastbad/optimism/pull/4

Long story short, I learned the hard way that using full_message impacts more than just the string we perceive.

Now, I'm open to the idea that not displaying the full message could be really helpful but I admit I'm skeptical that even you want it that way for every field. This is such an all-or-nothing approach, after all.

I did have an alternative idea. I don't know how much merit it has. What if instead of omitting the attribute from the string, we wrapped the attribute in a span and put a class on it? Then you could display: none that class on a per-attribute basis and not remove the attribute entirely. Could something like that work?

leastbad avatar Jul 06 '20 11:07 leastbad

Yeah, I had considered that a sledgehammer approach like I did would maybe not be the best. Your idea about returning it as HTML with some classes that can be hidden is interesting.

something like: <span class='form-attr'>Name</span>is too short

I wonder, though, if it could end up hurting someone later who happens to be trying to use that exact same class for something else?

mepatterson avatar Jul 06 '20 23:07 mepatterson

We could create a configurable property for that class...

leastbad avatar Jul 07 '20 13:07 leastbad

That would work... On Jul 7, 2020, 8:59 AM -0500, leastbad [email protected], wrote:

We could create a configurable property for that class... — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

mepatterson avatar Jul 07 '20 14:07 mepatterson

Hey Matt, is this still something that's making life hard for you? I haven't had time to approach creating a configurable property for this, but I will if you need it.

leastbad avatar Jul 22 '20 17:07 leastbad

I hacked around it for now. I need to investigate how it plays with i18n also. That might be a better solution to rewriting the messages anyway On Jul 22, 2020, 12:46 PM -0500, leastbad [email protected], wrote:

Hey Matt, is this still something that's making life hard for you? I haven't had time to approach creating a configurable property for this, but I will if you need it. — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe.

mepatterson avatar Jul 22 '20 17:07 mepatterson

Okay, I am going to stop thinking about this again, then. Too many things! :)

Ping me if you want to revisit.

leastbad avatar Jul 22 '20 17:07 leastbad

I haven't taken much time to go through the code so forgive me if this is way off-base, but why not have this as an option passed into error_for, e.g. <%= form.error_for :username, raw_message: true %>?

Would this not be a more acceptable solution than CSS hacks or global configuration options?

I found myself in need of exactly this functionality just now which is what lead me here. I have an Email model with an address field and I use nested attributes for it, which makes the error messages come up as "Address ...".

A suitable alternative, at least in my case, would be an option like <%= nested_email.error_for :address, attribute_name: 'Email address' %>

It seems both of these options would be really useful to someone, somewhere, in some case. So maybe an option attribute_name that can be set to either false or a string/symbol, where false would give back the raw message and string/symbol would replace the attribute name?

Inkybro avatar Aug 18 '20 13:08 Inkybro

Ah, I see that wouldn't work so easily. Hmm...

Inkybro avatar Aug 18 '20 14:08 Inkybro

Scratch all of that, I just realized the behavior I needed already exists. I had never really messed with I18n before today so this ended up being a nice learning experience. Apologies for cluttering things up.

Inkybro avatar Aug 18 '20 16:08 Inkybro

@mepatterson I know you worked around your issue for now, but does #18 seem to solve that?

Inkybro avatar Aug 22 '20 08:08 Inkybro