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

Potential Issues Around Knapsack and Rails.root #60

Open
14 tasks
jeremyf opened this issue Oct 5, 2023 · 0 comments
Open
14 tasks

Potential Issues Around Knapsack and Rails.root #60

jeremyf opened this issue Oct 5, 2023 · 0 comments

Comments

@jeremyf
Copy link
Contributor

jeremyf commented Oct 5, 2023 •
edited
Loading

In Hyku and Hyrax there are long-standing references to Rails.root. This works when Hyku is the base application. However, with Knapsack, we need to consider an alternate approach to Rails.root.

The issue is as follows:

  • Rails.root resolves to the root of the Hyku application
  • Knapsack is intended to be an overlay of that application; meaning if I add a static file in Knapsack we would want Hyku to pick that up.

A proposed solution for Hyku is available here:

However, Hyrax also uses Rails.root to reference static files. I used rg Rails.root app lib --files-with-matches | awk '{ print "- [ ] " $1 } to generate the list of files that need reviewing:

  • app/services/hyrax/simple_schema_loader.rb
  • app/services/hyrax/analytics/google.rb
  • app/services/hyrax/analytics/matomo.rb
  • app/services/hyrax/workflow/workflow_importer.rb
  • app/controllers/hyrax/downloads_controller.rb
  • lib/hyrax/configuration.rb
  • lib/hyrax/controlled_vocabulary/importer/language.rb
  • lib/tasks/universal_viewer.rake
  • lib/generators/hyrax/templates/config/initializers/redis_config.rb
  • lib/generators/hyrax/templates/config/initializers/hyrax.rb
  • lib/generators/hyrax/templates/config/initializers/riiif.rb
  • lib/hyrax/arkivo/config.rb
  • lib/hyrax/zotero/config.rb
  • lib/hyrax/specs/capybara.rb

This is also reported in:

We'll also want to add a rubocop to indicate the Rails.root is an anti-pattern.
@jeremyf jeremyf mentioned this issue Oct 5, 2023
Open
14 tasks
jeremyf added a commit that referenced this issue Oct 5, 2023
@jeremyf
🐛 Ensure knapsack translations are first choice
841e3f9 
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 mentioned this issue Oct 5, 2023
Merged
jeremyf added a commit that referenced this issue Oct 5, 2023
@jeremyf
🐛 Ensure knapsack translations are first choice
654bdb6 
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 added a commit that referenced this issue Oct 5, 2023
@jeremyf
🐛 Ensure knapsack translations are first choice
50dd0c7 
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 added a commit that referenced this issue Oct 5, 2023
@jeremyf
🐛 Ensure knapsack translations are first choice (#63)
740fd2a 
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 added a commit that referenced this issue Oct 6, 2023
@jeremyf
🐛 Ensure Catalog i18ns favor knapsack
3eeea64 
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 jeremyf mentioned this issue Oct 6, 2023
Merged
jeremyf added a commit that referenced this issue Oct 6, 2023
@jeremyf
🐛 Ensure Catalog i18ns favor knapsack (#65)
db63a45 
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)
@ShanaLMoore ShanaLMoore mentioned this issue Oct 9, 2023
Closed
2 tasks
jeremyf added a commit that referenced this issue Oct 9, 2023
@jeremyf
🧹 Remove override of thumbnail service
fcd8c65 
This is now obviated by upstream Hyku.  Prior to this commit, we were
looking in `Rails.root` but we needed to first interogate the Knapsack
and then Rails.root.

Related to:

- samvera/hyku#2010
- samvera/hyku#2012
- #60
- samvera-labs/hyku_knapsack#1
@jeremyf jeremyf mentioned this issue Oct 9, 2023
Merged
jeremyf added a commit that referenced this issue Oct 10, 2023
@jeremyf
🧹 Remove override of thumbnail service (#72)
d047bdd 
This is now obviated by upstream Hyku.  Prior to this commit, we were
looking in `Rails.root` but we needed to first interogate the Knapsack
and then Rails.root.

Related to:

- samvera/hyku#2010
- samvera/hyku#2012
- #60
- samvera-labs/hyku_knapsack#1
jeremyf added a commit that referenced this issue Oct 12, 2023
@jeremyf @laritakr
♻️ Return date_issued i18n to "Date Issued"
de1db3d 
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
- #60

Co-authored-by: LaRita Robinson <[email protected]>
jeremyf added a commit that referenced this issue Oct 12, 2023
@jeremyf @laritakr
♻️ Return date_issued i18n to "Date Issued"
71a51a2 
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
- #60

Co-authored-by: LaRita Robinson <[email protected]>
@jillpe jillpe mentioned this issue Dec 7, 2023
Closed
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@jeremyf