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

Fix I18n lookup for enum values in nested select fields #1342

Merged
merged 2 commits into from
Aug 31, 2021
Merged

Fix I18n lookup for enum values in nested select fields #1342

merged 2 commits into from
Aug 31, 2021

Conversation

Fs00
Copy link
Contributor

@Fs00 Fs00 commented Jun 4, 2021

This PR accomplishes the same as #1227, albeit with a more focused approach.
When constructing the key for looking up the localized enum value, we now use ActiveModel::Name.i18n_key instead of object_name, which returned something like user[posts_attributes][0] in case of nested forms.
I've added a test that reproduces the exact scenario that triggered the fixed bug (#1151).

Bonus: I've added a commit to update Formtastic version in Gemfile.lock (it was still 4.0.0.rc1).

Fixes #1151
Supersedes #1227

@Fs00
Copy link
Contributor Author

Fs00 commented Aug 31, 2021

@mikz @justinfrench Any plans to review/merge this in the near future?

Copy link
Contributor

@mikz mikz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, looks like I'm not subscribed to new issues 🤦🏻‍♂️

I think this looks great! It does not break tests, it adds a new one, it is very clear in what it does.

Great fix 👍

@mikz mikz merged commit 3c24db8 into formtastic:master Aug 31, 2021
@Fs00
Copy link
Contributor Author

Fs00 commented Aug 31, 2021

Thanks!
Will there be a new release anytime soon? I think that there have been some interesting fixes since 4.0.0 that may be worth a patch release.

@mikz
Copy link
Contributor

mikz commented Aug 31, 2021

@Fs00 indeed, but as we are not doing many there is always some element of fear included :)
I'd recommend pointing bundler to the git repo in the meantime.

@Fs00
Copy link
Contributor Author

Fs00 commented Aug 31, 2021

Oh well, less changes, less fear! 😄 I think that waiting will only make things more risky and difficult.

Anyway, I cannot point Bundler to the Git repo, since I use this gem in an enterprise environment where dependencies can't be pulled from the "outside world". I'll wait patiently 🤷‍♂️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

collection_from_enum I18n unusable with has_many nested forms
2 participants