-
Notifications
You must be signed in to change notification settings - Fork 0
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 that Bulkrax sidebar navigation appears. #731
Merged
Merged
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
The view paths for the application are a bit unexpected. - /gems/hyrax-doi-d494a50ef8ce/app/views - /gems/bulkrax-5.3.0/app/views - /app/samvera/hyrax-webapp/app/views - /gems/iiif_print-eb320b1c454f/app/views - /gems/hyrax-doi-d494a50ef8ce/app/views - /gems/qa-5.10.0/app/views - /gems/hyrax-ab3185d3069f/app/views - /gems/browse-everything-1.2.0/app/views - /gems/hydra-editor-5.0.5/app/views - /gems/blacklight-gallery-0.12.0/app/views - /gems/mailboxer-0.15.1/app/views - /gems/flipflop-2.7.1/app/views - /gems/devise_invitable-1.7.5/app/views - /gems/devise-i18n-1.10.2/app/views - /gems/devise-4.8.1/app/views - /gems/bulkrax-5.3.0/app/views - /gems/blacklight_range_limit-7.0.1/app/views - /gems/blacklight_advanced_search-6.4.1/app/views - /gems/blacklight-6.25.0/app/views - /gems/kaminari-core-1.2.2/app/views note - hyrax-doi & bulkrax are above the webapp. This means whenever we look up views, whatever is in those 2 applications will take priority. - https://github.com/samvera-labs/hyrax-doi/blob/d494a50ef8ce3eae594c7ed7148c33b3c977d4a7/lib/hyrax/doi/engine.rb#L29-L39 - https://github.com/samvera-labs/bulkrax/blob/e1d8a4e7f80074908991907b7aeaa38ba865258d/lib/bulkrax/engine.rb#L25-L35 The code we added comes from the Bulkrax generator. See link below https://github.com/samvera-labs/bulkrax/blob/e1d8a4e7f80074908991907b7aeaa38ba865258d/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb#L82-L85 Co-authored-by: Jeremy Friesen <jeremy.n.friesen@gmail.com>
jeremyf
approved these changes
Aug 22, 2023
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I saw that this worked for Summer. Other approvers would be great!
labradford
approved these changes
Aug 22, 2023
alishaevn
approved these changes
Aug 22, 2023
jeremyf
added a commit
to samvera/bulkrax
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 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
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
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 samvera/bulkrax
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 Bulkrax engine into the view path. Related to: - scientist-softserv/palni-palci#731
jeremyf
added a commit
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
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.
Story
The view paths for the application are a bit unexpected.
note - hyrax-doi & bulkrax are above the webapp. This means whenever we look up views, whatever is in those 2 applications will take priority.
The code we added comes from the Bulkrax generator. See link below
https://github.com/samvera-labs/bulkrax/blob/e1d8a4e7f80074908991907b7aeaa38ba865258d/lib/generators/bulkrax/templates/config/initializers/bulkrax.rb#L82-L85