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 initial EDTFYear processor #68

Merged
merged 2 commits into from
May 3, 2021
Merged

Add initial EDTFYear processor #68

merged 2 commits into from
May 3, 2021

Conversation

seth-shaw-unlv
Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv commented Apr 30, 2021

GitHub Issue: Partially addresses Islandora/documentation#962

What does this Pull Request do?

Builds on ASU's IssueYear search_api plugin to transform EDTF dates into Year integers suitable for searching and facets. This one also supports intervals and sets.

This one also allows sites to pick which EDTF fields they want to include and, for open intervals, the beginning and end years to use. (We currently need set year boundaries; we can't support truly open intervals, yet.)

What's new?

  • Added src/Plugin/search_api/processor/EDTFYear.php
  • Does this change require documentation to be updated? Probably should document its existence and use...
  • Does this change add any new dependencies? No.
  • Does this change require any other modifications to be made to the repository
    (i.e. Regeneration activity, etc.)? If added to a search index config a site would have to re-index (as usual for adding search fields).

How should this be tested?

  • Apply the PR
  • Go to the search_api_solr index processors config form (/admin/config/search/search-api/index/default_solr_index/processors)
    • Check the "EDTF Year" box.
    • Adjust the processor settings (down at the bottom) to select which fields you want to include. Optionally configure the open interval start and end year options.
    • Save the processors form
  • Go to the search_api_solr index field config form (/admin/config/search/search-api/index/default_solr_index/fields)
    • Click "Add Field"
    • Find "EDTF Creation Date Year (edtf_year)" under "General Fields" and click "Add" in the modal window
    • Click "Done"
    • Save the form
  • Create some items using the EDTF fields you selected. Bonus: adjust the widget to permit intervals and sets and create records to test them, too.
  • You may need to manually tell the site to re-index depending on your settings.
  • Query your SOLR instance to view all the edtf_year values OR create a facet using the new field and place the block on the search page to see it at work. Note: ideally you could use the range facet widget, but I can't seem to get it to work. ☹️🤷‍♂️

Additional Notes

The facets range widget (the primary use-case for this processor) has a bug for D9 (https://www.drupal.org/project/facets/issues/3186953 and https://www.drupal.org/project/facets/issues/3153622) but @elizoller was able to get it working. It should be okay for D8 sites.

Interested parties

@Islandora/8-x-committers, esp @elizoller

@seth-shaw-unlv seth-shaw-unlv changed the title Add intial EDTFYear processor Add initial EDTFYear processor Apr 30, 2021
@seth-shaw-unlv seth-shaw-unlv marked this pull request as ready for review April 30, 2021 17:24
@elizoller
Copy link
Member

@seth-shaw-unlv how do you want to take this forward? i did a test in a vanilla (bartik) D9 instance built from the playbook.
in order to get the range slider working i had to

  • configure the facet as a range slider
  • composer install and enable drupal/jquery-ui-slider
  • composer install bower-asset/jquery-ui-slider-pips (which required some additional changes to the composer.json since it has to install correctly in the libraries directory)
  • install a patch to facets (https://www.drupal.org/project/facets/issues/3186953)

@seth-shaw-unlv
Copy link
Contributor Author

seth-shaw-unlv commented May 3, 2021

@elizoller, first, let's get this merged. 😁

Second, add instructions for creating the slider facet to the Islandora 8 Cookbook. Ideally we would be able to add the facet to islandora_defaults, but I don't think we should while the D9 solution requires a patch.

Copy link
Member

@elizoller elizoller left a comment

Choose a reason for hiding this comment

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

I tested with date intervals and single dates as well as configuration to allow single or multiple fields. Seems good to go from my perspective.

@seth-shaw-unlv
Copy link
Contributor Author

@elizoller, do you have admin rights to squash and merge despite Travis holding it up?

@elizoller
Copy link
Member

i do not :(

@seth-shaw-unlv
Copy link
Contributor Author

@dannylamb, I know you're in a meeting, but would you please use your admin powers to merge this based on @elizoller's review when you are done?

@elizoller
Copy link
Member

cookbook PR up: Islandora-Labs/Islandora-Cookbook#13

@seth-shaw-unlv
Copy link
Contributor Author

@elizoller, it looks like it is no longer looking for Travis' approval. You should be able to merge it now.

I'll checkout the cookbook in a minute.

@elizoller elizoller merged commit 70cab93 into Islandora:8.x-1.x May 3, 2021
@seth-shaw-unlv seth-shaw-unlv deleted the edtf-year-preprocessor branch May 3, 2021 19:41
@kayakr
Copy link
Contributor

kayakr commented May 26, 2021

@seth-shaw-unlv FYI, on applying this to an existing Islandora instance, when actually using the facet I get a PHP warning. I haven't had a chance to dig into the data yet.

Warning: array_shift() expects parameter 1 to be array, string given in Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->createFilterQuery() (line 3133 of modules/contrib/search_api_solr/src/Plugin/search_api/backend/SearchApiSolrBackend.php).
Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->createFilterQuery('itm_edtf_year', '1990', 'BETWEEN', Object, Array) (Line: 2980)
Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->createFilterQueries(Object, Array, Object, Array) (Line: 2994)
Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->createFilterQueries(Object, Array, Object) (Line: 2838)
Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->getFilterQueries(Object, Array) (Line: 1454)
Drupal\search_api_solr\Plugin\search_api\backend\SearchApiSolrBackend->search(Object) (Line: 473)
Drupal\search_api\Entity\Server->search(Object) (Line: 539)
Drupal\search_api\Query\Query->execute() (Line: 580)
Drupal\search_api\Plugin\views\query\SearchApiQuery->execute(Object) (Line: 1426)
Drupal\views\ViewExecutable->execute(NULL) (Line: 1454)
Drupal\views\ViewExecutable->render() (Line: 183)
Drupal\views\Plugin\views\display\Page->execute() (Line: 1630)
Drupal\views\ViewExecutable->executeDisplay('page_1', Array) (Line: 77)
Drupal\views\Element\View::preRenderViewElement(Array)
call_user_func_array(Array, Array) (Line: 100)
Drupal\Core\Render\Renderer->doTrustedCallback(Array, Array, 'Render #pre_render callbacks must be methods of a class that implements \Drupal\Core\Security\TrustedCallbackInterface or be an anonymous function. The callback was %s. Support for this callback implementation is deprecated in 8.8.0 and will be removed in Drupal 9.0.0. See https://www.drupal.org/node/2966725', 'silenced_deprecation', 'Drupal\Core\Render\Element\RenderCallbackInterface') (Line: 781)
Drupal\Core\Render\Renderer->doCallback('#pre_render', Array, Array) (Line: 372)
Drupal\Core\Render\Renderer->doRender(Array, ) (Line: 200)
Drupal\Core\Render\Renderer->render(Array, ) (Line: 226)
Drupal\Core\Render\MainContent\HtmlRenderer->Drupal\Core\Render\MainContent\{closure}() (Line: 573)
Drupal\Core\Render\Renderer->executeInRenderContext(Object, Object) (Line: 227)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare(Array, Object, Object) (Line: 117)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse(Array, Object, Object) (Line: 90)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray(Object, 'kernel.view', Object)
call_user_func(Array, Object, 'kernel.view', Object) (Line: 111)
Drupal\Component\EventDispatcher\ContainerAwareEventDispatcher->dispatch('kernel.view', Object) (Line: 156)
Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object, 1) (Line: 68)
Symfony\Component\HttpKernel\HttpKernel->handle(Object, 1, 1) (Line: 57)
Drupal\Core\StackMiddleware\Session->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object, 1, 1) (Line: 106)
Drupal\page_cache\StackMiddleware\PageCache->pass(Object, 1, 1) (Line: 85)
Drupal\page_cache\StackMiddleware\PageCache->handle(Object, 1, 1) (Line: 47)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object, 1, 1) (Line: 52)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object, 1, 1) (Line: 23)
Stack\StackedHttpKernel->handle(Object, 1, 1) (Line: 708)
Drupal\Core\DrupalKernel->handle(Object) (Line: 19)

@seth-shaw-unlv
Copy link
Contributor Author

Huh. Let me know what you find with this. I didn't run into this myself and I don't know how to replicate it.

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.

3 participants