-
Notifications
You must be signed in to change notification settings - Fork 3
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
Upgrade to geoblacklight 4.0 #963
Conversation
d1b2218
to
21d9ddb
Compare
Heads up!This branch (and not Once the indexing is solid and the new production core is populated with Aardvark-format records, it can be merged and CD can be recoupled to To do that, you can just revert 1602a5e. |
188272b
to
5f5c53b
Compare
c9c74be
to
064dcc9
Compare
043e237
to
2fe797f
Compare
a54599c
to
f75f645
Compare
This was all for old MIT layers that are no longer indexed.
Don't display values on the show page, but do use them to indicate what file format will be downloaded from the dropdown.
Aardvark provides these fields natively.
This raises ArgumentError because it was deprecated in 7.0 and removed in 7.1.
# rubocop:disable Layout/LineLength | ||
config.add_show_tools_partial :web_services, if: proc { |_context, _config, options| | ||
options[:document] && (Settings.WEBSERVICES_SHOWN & options[:document].references.refs.map { |r| r.type.to_s }).any? | ||
} |
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.
Why is this removed?
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.
Seems like maybe the web services partial is now added by default in _show_sidebar.html.erb in GeoBlacklight so it no longer needs to be added a show tools partial separately. (Seems that way looking at GeoBlacklight itself)
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.
Yeah, we don't need to explicitly add this partial any longer
gem 'geo_monitor', '~> 0.7', github: 'geoblacklight/geo_monitor' | ||
# Not compatible with GeoBlacklight 4.x | ||
# https://github.com/geoblacklight/geo_monitor/issues/12 | ||
# gem 'geo_monitor', '~> 0.7', github: 'geoblacklight/geo_monitor' |
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.
What is our plan in general for GeoMonitor once this upgrade is in place? Will we need to work out a way later to use it, or will be table this for good? Thanks
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 see this down below geoblacklight/geo_monitor#12 so I think that answers that question : )
@@ -3,6 +3,7 @@ class CheckLayerJob < ApplicationJob | |||
|
|||
## | |||
# @param [GeoMonitor::Layer] layer | |||
# TODO: migrate this to the geo_monitor gem itself? |
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.
Is this still a to do? Thanks
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.
It is; I added it as geoblacklight/geo_monitor#17 but left a reminder here for later. Something to do after GBLv4 compatibility.
Looks good in general to me. @dnoneill and I have a few questions. Since I've seen this branch functioning pretty well on staging already, happy to approve although I think we will need to hold off on merging because of indexing, etc. |
Blocked by changing all our metadata to the new schema used by v4: https://opengeometadata.org/docs/about-ogm-aardvark