Feature: Support short hand in more Rails controller methods (private methods, before_actions)

[bjacobson26]

I encountered this problem in rails.

I have a controller with a before action:

class ThingsController
  before_action :redirect_if_view_only, only: [:update, :destroy]
  
  def update
  end
  
  def destroy
  end
  
  private
  
  def redirect_if_view_only
    flash[:error] = t('.not_authorized')
    redirect_back_or_to root_path
  end
end

in my en.yml:

things:
  not_authorized: You are not authorized to perform this action.

This works in the application but when I run the check it says I have a missing translation for:

things.redirect_if_view_only.not_authorized

and says I have an unused key:

things.not_authorized

if I change the yml to:

things:
  redirect_if_view_only:
    not_authorized: You are not authorized to perform this action.

it passes the check but it fails when I try to load the page saying there is no translation found for the given key.

the workaround is to just give the full path for the translation.

t('things.redirect_if_view_only.not_authorized')

Not sure if this is expected behavior or a bug so I figured I'd open an issue. Thanks!

[davidwessman]

Yeah, this is a limitation to the implementation today.

It is quite hard to figure out what value we should translate it to, since it can be:

• things.not_authorized
• things.update.not_authorized
• things.destroy.not_authorized

and the last two might have different values.
@glebm Do you have any idea how we could make this behaviour better?
Some kind of different handling for private controller methods?

[glebm]

According to rails code, the key is always scoped to #{controller_path.tr('/', '.')}.#{action_name}:

https://github.com/rails/rails/blob/179b979ddbb7bcc4d1a12d0d71779f47c1c9d9cd/actionpack/lib/abstract_controller/translation.rb#L7-L22

If I understand the above correctly, this relative key is actually 2 keys:

things.update.not_authorized
things.destroy.not_authorized

Detecting this automatically would be quite tricky (we'd need to parse method calls, before_action, etc).

Perhaps one way to handle this would be to require # i18n-tasks-use annotations for such keys.
So we'd ignore these keys and issue a warning if there is no associated # i18n-tasks-use annotation.
What do you think?

[mockdeep]

I ran into this as well, but my use case doesn't have a before_action:

def destroy
  ...
  destroy_step(step, campaign)
end

private

def destroy_step(step, campaign)
  ...
  flash[:success] = t('.success')
end

This seems like maybe a case that is a little more clear for i18n-tasks to parse.

[davidwessman]

Yeah, but we have to create a custom matcher that knows it is in a controller.

I will try to look at this during my spare time

[mockdeep]

@davidwessman does the shorthand work in places where it wouldn't be treated this way? I haven't tested mailers, but I assume they would behave the same way.

[davidwessman]

The logic for the shorthand is quite limited, I spent some time trying to write something different and I have a working PoC.
But I am not sure if it fits into the model of i18n-tasks today, because we can no longer do:

1. Parse all calls to translations
2. Map the translations to full keys based on file names
3. Calculate used_keys

Instead we need to combine 1 and 2 while parsing, so we know what we are looking for, and then we need to have different parser rules for a controller, a mailer and for views. Might have some more time to work on it during this month.

[jclusso]

Any progress on this or recommended workarounds?

[davidwessman]

No, lately I have been thinking if this is something that would fit better in e.g. rails-lsp and then maybe i18n-tasks could integrate with it.

The only workaround is magic comments :/

[jclusso]

The only workaround is magic comments :/

Is there a magic comment to ignore a missing translation. I couldn't figure that out so I have to add the key it's looking for in the ignore_missing which kinda sucks.

[davidwessman]

Unfortunately not, then you need to add it to the ignore_missing config.