Rename the I18n keys for associations' restrict_dependent_destroy errors #11416

Merged
merged 1 commit into from Jul 21, 2013

Conversation

Projects
None yet
3 participants
Contributor

tigrish commented Jul 13, 2013

The :one and :many keys are used for pluralisations, but as such are incorrect in english (since we should be using :one for singular and :other for plurals).

However in this particular case, we're not describing a pluralized entity, we're describing an error for a particular type of association.

This commit changes the I18n key to be the type of association rather than a misleading pluralization key, so that apps that test the validity of their pluralizations can continue to work.

Owner

senny commented Jul 13, 2013

If this is going to make it in, we need some kind of deprecation and a fallback to work with the old keys. We need to give the users a chance to notice the problem when they upgrade so they can change the keys.

Contributor

kuroda commented Jul 15, 2013

+1

I closed svenfuchs/rails-i18n#340 by adding temporary (ugly) matchers to accommodate these irregular key names (:one and :many), but I still think the @tigrish's proposal is valid.

Contributor

tigrish commented Jul 15, 2013

@senny any pointers on how to add the fallback given that we're not setting the message directly, rather we're adding an error to the model? I guess this also applies to the deprecation warning.

senny merged commit 9dc8aef into rails:master Jul 21, 2013

1 check passed

default The Travis CI build passed
Details

@senny senny added a commit that referenced this pull request Jul 22, 2013

@senny senny Revert "Merge pull request #11416 from tigrish/master"
This reverts commit 9dc8aef, reversing
changes made to 02e8dae.
ff3739e
Owner

senny commented Jul 22, 2013

@tigrish I only just now noticed that I merged this one by accident. (don't ask me how I could confuse it with another one). I reverted the commit as GitHub won't let me reopen the PR, can you please resubmit it?

I think before thinking about the deprecation warning it's good to get more feedback if it's going to make it in or not.

Owner

senny commented Jul 22, 2013

Sorry for the inconveniences 😓

Contributor

tigrish commented Jul 29, 2013

Bummer, you totally got my hopes up :'(
Anyway resubmitted...

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