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

HydraConnect refactor #55

Merged

Conversation

ebenenglish
Copy link
Contributor

So here's the refactor based on our discussions at Hydra Connect and on the Blacklight-development list. To summarize, the main changes are:

  • add catalog#map view which displays all objects in Solr index with geospatial data
  • catalog#index map view uses facet data to populate map, so all results are immediately viewable (rather than just current page's worth of items)
  • maplet widget available for catalog#show view
  • use GeoJSON to store coordinates with placenames in Solr
  • marker popups display location name (or coordinates) and link to search params (rather than displaying results in the map view itself)
  • marker clusters/markers display number of hits, rather than locations
  • search control on map allows search for all items within current map window bounds
  • map bounds and zoom are set dynamically when more than one location displayed on map
  • add spatial search functionality to Blacklight
  • render spatial constraints in catalog#index

Note: there are a handful of tests that are 'pending' because I couldn't get them passing despite extensive effort. But, they seemed important so I left them in -- hopefully someone can get these working?

Other note: I haven't updated the version. Let me know if that's something I should be doing before submitting the PR.

Thanks for everyone's patience on this!

@@ -23,6 +23,7 @@ Gem::Specification.new do |spec|
spec.add_dependency "leaflet-rails"
spec.add_dependency "leaflet-markercluster-rails"
spec.add_dependency "leaflet-sidebar-rails", "~> 0.0.2"
#spec.add_dependency "c_geohash", "~> 1.1.2" # don't need if location_rpt can't be faceted properly
Copy link
Contributor

Choose a reason for hiding this comment

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

This seems like cruft here. Is it necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, leftover from some experimentation I was doing. Will remove.

@ebenenglish
Copy link
Contributor Author

I pushed up a commit to address the gemspec/Rspec issues you commented on.

@mejackreed
Copy link
Contributor

@ebenenglish great thanks. Also it looks like it will also need a rebase (merge conflicts). Can you please do a rebase off of master and then push back up?

@ebenenglish
Copy link
Contributor Author

Rebase done; looks like build failing due to a few tests failing with Ruby 1.9.3. Are we still supporting this version of Ruby? If so I'll see if I can get these working...

@ebenenglish
Copy link
Contributor Author

All tests should now be passing in Ruby 1.9.3. Latest build failed under 2.2.0 and 2.0.0 (but passed for 2.1.0 and 1.9.3) because of a weird Poltergeist timeout error:

Capybara::Poltergeist::TimeoutError:

Timed out waiting for response to {"name":"visit","args":["http://127.0.0.1:35506/catalog?q=korea\u0026view=maps"]}. It's possible that this happened because something took a very long time (for example a page load was slow). If so, setting the Poltergeist :timeout option to a higher value will help (see the docs for details). If increasing the timeout does not help, this is probably a bug in Poltergeist - please report it to the issue tracker.

Not sure if this is a random momentary glitch or what, but this didn't come up in previous builds, and all tests are passing on my end.

@mejackreed
Copy link
Contributor

Ok removing jruby and 1.9.3 from travis with #57 ... rerunning travis to see if I can get it to pass.

@mejackreed
Copy link
Contributor

@ebenenglish Do you have this deployed somewhere I can play with? Thanks!

@ebenenglish
Copy link
Contributor Author

Yes, it's been deployed on the Digital Commonwealth production site:

The deployment is basically "out of the box" except for the BlacklightMapsHelperBehavior#link_to_placename_field method, which I'm overriding locally to deal with converting state abbreviations ("NV") to their full names ("Nevada").

mejackreed added a commit that referenced this pull request Feb 5, 2015
@mejackreed mejackreed merged commit 9860aa9 into projectblacklight:master Feb 5, 2015
@ebenenglish ebenenglish deleted the hydra-connect-refactor branch March 4, 2015 14:05
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.

None yet

2 participants