Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource name in flash messages is in camel case #1819

Closed
orenyk opened this issue Nov 4, 2020 · 5 comments
Closed

Resource name in flash messages is in camel case #1819

orenyk opened this issue Nov 4, 2020 · 5 comments
Labels
bug breakages in functionality that is implemented resolver

Comments

@orenyk
Copy link

orenyk commented Nov 4, 2020

  • What were you trying to do?
    • Set up a dashboard with a resource name that is two acronyms combined (e.g. FOO BAR), when both FOO and BAR are registered as acronyms with ActiveSupport (so that the path names work, e.g. app/models/foo_bar.rb). I expect that in flash messages that the name will be represented correctly (e.g. FOO BAR was successfully created.)
  • What did you end up with (logs, or, even better, example apps are great!)?
    • The resource name in flash messages is the two acronyms combined without the space (e.g. FOOBAR was successfully created.). The root cause is here, where the ResourceResolver first converts the resource name to camel case, causing the space to be lost when it is subsequently converted to snake case in #resource_name.
  • What versions are you running?
    • Rails
      • 6.0.3.4
    • administrate
      • 0.14.0

If anyone else is experiencing this, for the time being my workaround is to override translate_with_resource method in that specific resource's controller to hard-code the resource name with the space instead of relying on ResourceResolver.

Hope this helps!


EDIT 2020-11-05 I don't believe this is exclusive to resources whose names begin with an acronym, as I'm also seeing it with non-acronym resources. I'm not sure why the following lines aren't working (I checked that the model_name.human methods work in the Rails console, so maybe there's an issue with resource_class?):

def resource_title
resource_class.model_name.human
end

@orenyk orenyk added the bug breakages in functionality that is implemented label Nov 4, 2020
@pablobm
Copy link
Collaborator

pablobm commented Nov 5, 2020

Thank you for filing this issue. In general the resolver is due an revision, and this is the sort of behaviour that we need to keep in mind.

@orenyk
Copy link
Author

orenyk commented Nov 5, 2020

No problem! If I have some time I'll see if I can take a stab at a PR for at least this issue - it feels like it must be possible to leverage the same ActiveSupport mechanisms that allow acronyms to be registered to begin with. Thanks!

@orenyk
Copy link
Author

orenyk commented Nov 5, 2020

Just as a follow-up, I was incorrect in my assumption that this was specific to acronyms; I'm seeing the same behavior with regular camel-cased resources (FooBar). I'll still try to take a look and investigate further.

@orenyk orenyk changed the title Resource name in flash messages is incorrect when the first word is an acronym Resource name in flash messages is in camel case Nov 5, 2020
@orenyk
Copy link
Author

orenyk commented Nov 5, 2020

A little bit more info - I'm able to replicate this issue in the console and it's kinda weird:

irb(main):011:0> Administrate::ResourceResolver.new('admin/foo_bars').resource_title
=> "FooBar"
irb(main):012:0> Administrate::ResourceResolver.new('admin/foo_bars').resource_class.model_name.human
=> "Foo bar"

given that this is the resource_title method:

def resource_title
resource_class.model_name.human
end

@pablobm
Copy link
Collaborator

pablobm commented Nov 7, 2020

Good news! It looks like this is fixed in master. Not sure when, but I suspect it's due to #1633. Here's an illustration of 0.14 vs. master:

Before: "FooBar was successfully created". After: "Foo bar was successfully created"..

I'm going to close this as I think that's it, but feel free to comment or provide other info if you see an issue. For example, I guess the capitalisation is up for debate, but I think that should be customisable via i18n.

@pablobm pablobm closed this as completed Nov 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug breakages in functionality that is implemented resolver
Projects
None yet
Development

No branches or pull requests

2 participants