🐛 Ensuring we inject Hyrax::DOI views before Hyrax #122
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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: - notch8/palni-palci#731 - samvera/bulkrax#855 - samvera-labs/hyrax-doi#49
This was referenced Aug 22, 2023
summer-cook
approved these changes
Aug 22, 2023
jeremyf
added a commit
to notch8/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
added a commit
to notch8/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
aprilrieger
approved these changes
Aug 22, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 havehyrax-a. The result being that we pre-prended the view path into the application. That created the conditions where theview_pathsfirst 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: