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

Reverted i18n gem to version 0.8.1 #1184

Closed
wants to merge 3 commits into from

Conversation

smithjp
Copy link

@smithjp smithjp commented Jul 20, 2017

Fixes test failures due to missing translations.

It appears that the curation_concerns gem is not compatible with i18n gem >= 0.8.3, so an explicit dependency for i18n version 0.8.1 is being added.

@samvera/sufia-code-reviewers

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 0484443 on smithjp:1-7-7-release into ** on samvera:1-7-7-release**.

@jcoyne
Copy link
Contributor

jcoyne commented Jul 20, 2017

Can you provide a replication case for the bug? Have you filed an issue with the maintainers of the i18n gem?

@jcoyne
Copy link
Contributor

jcoyne commented Jul 20, 2017

Was it this issue? CanCanCommunity/cancancan#415

@smithjp
Copy link
Author

smithjp commented Jul 21, 2017

The test failures look like this:

 ActionView::Template::Error:
   translation missing: en.false
 # ./app/renderers/curation_concerns/renderers/configured_microdata.rb:13:in `microdata_object?'
 # ./app/renderers/curation_concerns/renderers/configured_microdata.rb:17:in `microdata_object_attributes'
 # ./app/renderers/curation_concerns/renderers/attribute_renderer.rb:28:in `render'
 # ./app/presenters/curation_concerns/presents_attributes.rb:20:in `attribute_to_html'
 # ./app/views/curation_concerns/base/_attributes.html.erb:8:in `__home_travis_build_samvera_curation_concerns_app_views_curation_concerns_base__attributes_html_erb__2674987801978955349_140047760'
 # ./app/views/curation_concerns/base/show.html.erb:19:in `__home_travis_build_samvera_curation_concerns_app_views_curation_concerns_base_show_html_erb___168103936029684779_140777240'
 # ./spec/features/add_file_spec.rb:18:in `block (2 levels) in <top (required)>'
 # ------------------
 # --- Caused by: ---
 # I18n::MissingTranslationData:
 #   translation missing: en.curation_concerns.schema_org.permission_badge.type
 #   ./app/renderers/curation_concerns/renderers/configured_microdata.rb:13:in `microdata_object?'

Which can be seen in the automated tests for previous pull request at: https://travis-ci.org/samvera/curation_concerns/jobs/240798479.

@jcoyne
Copy link
Contributor

jcoyne commented Jul 21, 2017

@smithjp to me it looks like the problem is on this line: https://github.com/samvera/curation_concerns/blob/master/app/renderers/curation_concerns/renderers/configured_microdata.rb#L13

The default probably shouldn't be false. Maybe empty string or nil?

@jcoyne
Copy link
Contributor

jcoyne commented Jul 21, 2017

I think I see what is happening here:

When you say t('foo', default: false) the TranslationHelper in rails actually calls:
I18n.translate('foo', default: [false], raise: true). I think this was broken by this change:
ruby-i18n/i18n@1641e5f

I made an upstream issue: ruby-i18n/i18n#379

Please add a comment next to your change that points to that issue. That way we can back out your change when the issue is resolved upstream.

@smithjp
Copy link
Author

smithjp commented Jul 21, 2017

I have added a comment in the Gemfile.extra file. Is this ok?

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 594d84e on smithjp:1-7-7-release into ** on samvera:1-7-7-release**.

@coveralls
Copy link

Coverage Status

Changes Unknown when pulling 594d84e on smithjp:1-7-7-release into ** on samvera:1-7-7-release**.

@awead awead changed the base branch from 1-7-7-release to 1-7-stable February 22, 2019 18:16
@smithjp smithjp closed this Jan 21, 2022
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.

None yet

4 participants