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.

Sign up for GitHub

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

Jump to bottom

Allow translating resource names in CRUD messages #1626

Merged
merged 1 commit into from
Oct 6, 2020
Merged

Allow translating resource names in CRUD messages #1626

merged 1 commit into from
Oct 6, 2020

Conversation

betelgeuse
Copy link
Contributor

Fixes #1242

@betelgeuse betelgeuse force-pushed the translate_model_name_in_crud_actions branch from 72f997e to 672438f Compare April 28, 2020 11:45
@betelgeuse betelgeuse marked this pull request as draft April 28, 2020 11:46
@betelgeuse betelgeuse force-pushed the translate_model_name_in_crud_actions branch 4 times, most recently from 54ecfbd to d5cc51c Compare April 28, 2020 12:03
@betelgeuse betelgeuse marked this pull request as ready for review April 28, 2020 12:08
@betelgeuse
Copy link
Contributor Author

How do I found out the issue that @HounD is still reporting?

@sedubois
Copy link
Contributor

@betelgeuse seems like all warnings are fixed now

nickcharlton
nickcharlton reviewed Apr 29, 2020
View reviewed changes
spec/features/edit_page_spec.rb Outdated Show resolved Hide resolved
spec/lib/administrate/resource_resolver_spec.rb Outdated
it "handles global-namepsace models" do
class User < ActiveRecord::Base; self.abstract_class = true; end
Copy link
Member

Choose a reason for hiding this comment

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

What's stopping us from using ApplicationRecord here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It’s not in scope when just running the file. Could be required of course.

@nickcharlton nickcharlton added feature new functionality that’s not yet implemented i18n translations and language support labels Apr 29, 2020
@betelgeuse
Copy link
Contributor Author

Concerning the labels this is more of a bug fix than a feature.

@nickcharlton nickcharlton force-pushed the translate_model_name_in_crud_actions branch from d5cc51c to a26278c Compare October 6, 2020 16:26
@nickcharlton nickcharlton merged commit 5a7ddca into thoughtbot:master Oct 6, 2020
@nickcharlton
Copy link
Member

Thanks for opening this! I've merged it now.

Re: labels: I do think anything with relating to translations is an issue which needs to be solved, but in this case, we weren't explicitly trying to translate the flash messages. If we were and it were broken, it would've been a bug. Is it a bit arbitrary? yeah, but I came up with them!

@flipsasser
Copy link

Ahoy hoy! Hi my name is Flip; I'm a big fan of Administrate.

@nickcharlton out of curiosity - what is the release cycle? I'd really love this patch to make its way to rubygems.org.

PS. Thanks @betelgeuse

@nickcharlton
Copy link
Member

I'm expecting that we'll do a release next before Christmas.

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

@nickcharlton nickcharlton nickcharlton left review comments

Assignees
No one assigned
Labels
feature new functionality that’s not yet implemented i18n translations and language support
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Resource names are not being translated properly on notice messages
4 participants
@betelgeuse @sedubois @nickcharlton @flipsasser