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

🐛 Ensure knapsack translations are first choice #63

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Oct 5, 2023

Prior to this change, assuming my default translation was "en" when I
called the following—I18n.t('dog_biscuits.fields.date_issued')—I would
get Translation missing: en.dog_biscuits.fields.date_issued.

We had added DogBiscuits into that path but we had not reloaded the
back-end.

Further, the catalog controller was no longer doing useful work. The
Rails.root is hyrax-webapp; which would already be working. For
more on this anti-pattern, see [Potential Issues Around Knapsack and Rails.root][1].

With this change, I have a small test to verify that dog biscuits
translations are in fact loaded.

Related to:

[1]: #60)

Prior to this change, assuming my default translation was "en" when I
called the following—`I18n.t('dog_biscuits.fields.date_issued')`—I would
get `Translation missing: en.dog_biscuits.fields.date_issued`.

We had added DogBiscuits into that path but we had not reloaded the
back-end.

Further, the catalog controller was no longer doing useful work.  The
`Rails.root` is `hyrax-webapp`; which would already be working.  For
more on this anti-pattern, see [Potential Issues Around Knapsack and Rails.root][1].

With this change, I have a small test to verify that dog biscuits
translations are in fact loaded.

Related to:

- https://github.com/scientist-softserv/adventist-dl/issues/611
- https://github.com/scientist-softserv/adventist-dl/issues/227
- scientist-softserv/adventist-dl#244

[1]: #60)
@jeremyf jeremyf merged commit 740fd2a into main Oct 5, 2023
2 checks passed
@jeremyf jeremyf deleted the dog-biscuits-translations branch October 5, 2023 20:02
jeremyf added a commit that referenced this pull request Oct 6, 2023
Prior to this change, we had ensured knapsack's translations took
precedence over Hyku's.  However, that ensuring occurred at the
after_initialize.  We had missed the CatalogController's translation
moment as well.

When we specify the index fields in the CatalogController, blacklight
caches those translations.  However, in the case of dogbiscuits, those
are not yet loaded.  Which results in a translation error; even though
we later load the dog biscuits translations.

With this change, I have added two tests to:

1. verify that translations are correct for the catalog controller
2. that when the app boots, the Knapsack translations take precedence
   over Hyku.

Related to:

- https://github.com/scientist-softserv/adventist-dl/issues/611
- https://github.com/scientist-softserv/adventist-dl/issues/227
- scientist-softserv/adventist-dl#244
- #63

[1]: #60)
jeremyf added a commit that referenced this pull request Oct 6, 2023
Prior to this change, we had ensured knapsack's translations took
precedence over Hyku's.  However, that ensuring occurred at the
after_initialize.  We had missed the CatalogController's translation
moment as well.

When we specify the index fields in the CatalogController, blacklight
caches those translations.  However, in the case of dogbiscuits, those
are not yet loaded.  Which results in a translation error; even though
we later load the dog biscuits translations.

With this change, I have added two tests to:

1. verify that translations are correct for the catalog controller
2. that when the app boots, the Knapsack translations take precedence
   over Hyku.

Related to:

- https://github.com/scientist-softserv/adventist-dl/issues/611
- https://github.com/scientist-softserv/adventist-dl/issues/227
- scientist-softserv/adventist-dl#244
- #63

[1]: #60)
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.

2 participants