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

Add a test for url_for_document, especially for a case that uses rail… #1964

Closed
wants to merge 1 commit into from

Conversation

cbeer
Copy link
Member

@cbeer cbeer commented Aug 19, 2018

…s polymorphic routing magic.

Fixes #1910

@coveralls
Copy link

coveralls commented Aug 22, 2018

Coverage Status

Coverage decreased (-1.04%) to 93.696% when pulling d4ee1d9 on 1910-search-state into c435cad on master.

Copy link
Member

@jcoyne jcoyne left a comment

Choose a reason for hiding this comment

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

Thank you!

if respond_to?(:blacklight_config) &&
blacklight_config.show.route &&
(!doc.respond_to?(:to_model) || doc.to_model.is_a?(SolrDocument))
if blacklight_config.show.route || (doc.respond_to?(:to_model) && doc == doc.to_model)
Copy link
Member

Choose a reason for hiding this comment

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

I think this || must still be an &&. Because the tests fail due to blacklight_config.show.route being nil

@barmintor barmintor added the ElasticSearch proposed v8 baseline feature label Oct 11, 2018
@barmintor barmintor added this to the 8.x milestone Oct 11, 2018
@cbeer cbeer moved this from Backlog to Needs Major Version in Blacklight Summit Triage Board Nov 9, 2021
@barmintor
Copy link
Contributor

On closer inspection, these tests are all present in main - @cbeer if I'm wrong there please let me know, but I think this is closeable.

@barmintor barmintor closed this Jan 18, 2022
@cbeer cbeer deleted the 1910-search-state branch January 18, 2022 17:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ElasticSearch proposed v8 baseline feature
Projects
Development

Successfully merging this pull request may close these issues.

None yet

4 participants