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

Geonames labels #5493

Merged
merged 1 commit into from Mar 22, 2022
Merged

Geonames labels #5493

merged 1 commit into from Mar 22, 2022

Conversation

dlpierce
Copy link
Contributor

@dlpierce dlpierce commented Mar 4, 2022

Fixes #1065, #3269

Retrieves, indexes and displays the full location label

Previously when indexing the Based Near/Location field on works, only the city name was available as it was all that is available in the rdf data that was retrieved. This PR add a service that retrieves a full label from another Geonames endpoint via questioning authority, caching the results locally. That service is then used when creating the solr doc and rendering the edit form.

Geonames wants a username to use this service. This is defined in config/initializers/hyrax.rb as config.geonames_username = Qa::Authorities::Geonames.username = '{username}' to set the user in Qa at the same time.

Changes proposed in this pull request:

  • Add service based on Qa::Authorities::Geonames to supply a full label
  • Index that full label
  • Display that full label on the work edit form

Guidance for testing, such as acceptance criteria or new user interface behaviors:

  • Creating a work with a location
  • Verify full label (e.g. City, State, Country) is shown on work details and edit form

@samvera/hyrax-code-reviewers

@elrayle
Copy link
Contributor

elrayle commented Mar 14, 2022

I used this branch to set a location and got an error while saving the new work...

image

@elrayle
Copy link
Contributor

elrayle commented Mar 14, 2022

Looks like it is happening when trying to generate the cache key...

image

If you drop the !, it doesn't try to update the URI. Looks like that may be all that is needed.

Copy link
Contributor

@elrayle elrayle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm seeing an error when trying to run this.

@elrayle
Copy link
Contributor

elrayle commented Mar 14, 2022

PR #5533 fixes the issue where prevents canonicalize from returning the string version of the URI. This should unblock this PR.

elrayle
elrayle previously approved these changes Mar 15, 2022
Copy link
Contributor

@elrayle elrayle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a good improvement to have the full location name. Great for disambiguation.

FYI... location doesn't work with Monographs (i.e. valkyrie works). But it didn't before either. A new issue needs to be opened.

@dlpierce
Copy link
Contributor Author

Removed configuration changes as they were unneeded.

Copy link
Contributor

@elrayle elrayle left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will significantly improve the usefulness of the Location lookups.

@elrayle elrayle merged commit 96bfa02 into main Mar 22, 2022
@elrayle elrayle deleted the geonames_labels branch March 22, 2022 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editing works shows location URI instead of location label Index full place name from geonames
2 participants