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

🐛 Ensuring we inject Bulkrax views before Hyrax #855

Merged
merged 1 commit into from
Aug 22, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Aug 22, 2023

Prior to this commit, the logic for finding the Hyrax in the view path was assuming a version number (e.g. hyrax-2.); however if the we pinned Hyrax to a branch/sha then we might have hyrax-a. The result being that we pre-prended the view path into the application. That created the conditions where the view_paths first element might not have been the application but instead a gem/engine.

Which means that the convention of overriding views in the application would not work.

With this commit, we're using the Hyrax's engine's root to determine the view path suffix. Further, if Hyrax is not in the view path, we don't again inject at the beginning of the list the Bulkrax engine into the view path.

Related to:

Prior to this commit, the logic for finding the Hyrax in the view path
was assuming a version number (e.g. `hyrax-2.`); however if the we
pinned Hyrax to a branch/sha then we might have `hyrax-a`.  The result
being that we pre-prended the view path into the application.  That
created the conditions where the `view_paths` first element might not
have been the application but instead a gem/engine.

Which means that the convention of overriding views in the application
would not work.

With this commit, we're using the Hyrax's engine's root to determine the
view path suffix.  Further, if Hyrax is not in the view path, we don't
again inject at the beginning of the list the Bulkrax engine into the
view path.

Related to:

- scientist-softserv/palni-palci#731
jeremyf added a commit to samvera-labs/hyrax-doi that referenced this pull request Aug 22, 2023
Prior to this commit, the logic for finding the Hyrax in the view path
was assuming a version number (e.g. `hyrax-2.`); however if the we
pinned Hyrax to a branch/sha then we might have `hyrax-a`.  The result
being that we pre-prended the view path into the application.  That
created the conditions where the `view_paths` first element might not
have been the application but instead a gem/engine.

Which means that the convention of overriding views in the application
would not work.

With this commit, we're using the Hyrax's engine's root to determine the
view path suffix.  Further, if Hyrax is not in the view path, we don't
again inject at the beginning of the list the Hyrax::DOI engine into the
view path.

Related to:

- scientist-softserv/palni-palci#731
- samvera/bulkrax#855
@jeremyf jeremyf added patch-ver for release notes bug-fix labels Aug 22, 2023
jeremyf added a commit to samvera-labs/allinson_flex that referenced this pull request Aug 22, 2023
Prior to this commit, the logic for finding the Hyrax in the view path
was assuming a version number (e.g. `hyrax-2.`); however if the we
pinned Hyrax to a branch/sha then we might have `hyrax-a`.  The result
being that we pre-prended the view path into the application.  That
created the conditions where the `view_paths` first element might not
have been the application but instead a gem/engine.

Which means that the convention of overriding views in the application
would not work.

With this commit, we're using the Hyrax's engine's root to determine the
view path suffix.  Further, if Hyrax is not in the view path, we don't
again inject at the beginning of the list the Hyrax::DOI engine into the
view path.

Related to:

- scientist-softserv/palni-palci#731
- samvera/bulkrax#855
- samvera-labs/hyrax-doi#49
jeremyf added a commit to scientist-softserv/palni-palci that referenced this pull request Aug 22, 2023
Prior to this commit, and with three engines (e.g. Bulkrax, Hyrax::DOI,
and AllinsonFlex), we had a situation in which the first view path may
not have been the application.  The consequence being that the view
lookup would potentially behave in unexpected ways.

Below is the commit message on the patches submitted for the three
engines.  The message further details the problem:

> Prior to this commit, the logic for finding the Hyrax in the view path
> was assuming a version number (e.g. `hyrax-2.`); however if the we
> pinned Hyrax to a branch/sha then we might have `hyrax-a`.  The result
> being that we pre-prended the view path into the application.  That
> created the conditions where the `view_paths` first element might not
> have been the application but instead a gem/engine.
>
> Which means that the convention of overriding views in the application
> would not work.
>
> With this commit, we're using the Hyrax's engine's root to determine
> the view path suffix.  Further, if Hyrax is not in the view path, we
> don't again inject at the beginning of the list the Bulkrax engine
> into the view path.

Related to:

- #731
- samvera/bulkrax#855
- samvera-labs/allinson_flex#122
- samvera/bulkrax#855
@jeremyf jeremyf merged commit 88bd56d into main Aug 22, 2023
7 of 8 checks passed
@jeremyf jeremyf deleted the ensuring-proper-view-logic branch August 22, 2023 20:03
jeremyf added a commit to scientist-softserv/palni-palci that referenced this pull request Aug 22, 2023
Prior to this commit, and with three engines (e.g. Bulkrax, Hyrax::DOI,
and AllinsonFlex), we had a situation in which the first view path may
not have been the application.  The consequence being that the view
lookup would potentially behave in unexpected ways.

Below is the commit message on the patches submitted for the three
engines.  The message further details the problem:

> Prior to this commit, the logic for finding the Hyrax in the view path
> was assuming a version number (e.g. `hyrax-2.`); however if the we
> pinned Hyrax to a branch/sha then we might have `hyrax-a`.  The result
> being that we pre-prended the view path into the application.  That
> created the conditions where the `view_paths` first element might not
> have been the application but instead a gem/engine.
>
> Which means that the convention of overriding views in the application
> would not work.
>
> With this commit, we're using the Hyrax's engine's root to determine
> the view path suffix.  Further, if Hyrax is not in the view path, we
> don't again inject at the beginning of the list the Bulkrax engine
> into the view path.

Related to:

- #731
- samvera/bulkrax#855
- samvera-labs/allinson_flex#122
- samvera/bulkrax#855
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug-fix patch-ver for release notes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants