Islandora / controlled_access_terms

Drupal module for subject and agents
GNU General Public License v2.0
7 stars 30 forks source link

EDTF Indexing Improvements (Issue 1852) #70

Closed seth-shaw-unlv closed 3 years ago

seth-shaw-unlv commented 3 years ago

GitHub Issue: Islandora/documentation/issues/1852

What does this Pull Request do?

Improves the EDTF indexer by allowing sites to optionally ignore undated ("XXXX") or open start and end dates. It also uses the Professional-Wiki EDTF library for parsing.

What's new?

How should this be tested?

Additional Notes:

Currently we only need the Professional Wiki library for the EDTF date processor, so if you aren't using it you don't have to update PHP to 7.4 or install the library (although composer will probably complain to you if you try to forego them). That said, I would like to start replacing bits of our own code with their library. This is just a first step.

Interested parties

@Islandora/8-x-committers and @kspurgin

kspurgin commented 3 years ago

@seth-shaw-unlv, @dannylamb - yeah, I'm not going to be able to test this. Sorry.

Decided it was a bad idea to try to do it in the dev environment I've already got set up, as it is LYR-specific and there may be who knows what customizations I don't know about already in there. I wasn't seeing the EDTF Processor as an option in the Search API settings despite having controlled-access-terms installed, either

I spent 1.5 hours and finally have a working make local instance via isle-dc but it does not have controlled-access-terms (or islandora-defaults) in it at all and I don't see them anywhere in the codebase folder.

Also I have no idea how to access the Solr admin with an isle-dc instance. And there are probably at least 27 other things I don't have the background knowledge/context for that would take forever, and I gotta get back to some deadlines.

seth-shaw-unlv commented 3 years ago

No worries, @kspurgin. It happens. I don't suppose @elizoller has a few minutes for this? If not, we can just do a minor release whenever someone gets to it.

elizoller commented 3 years ago

Yeah, i can take a look

elizoller commented 3 years ago

i don't think that test matrix will take effect until the PR is merged into the default branch btw

elizoller commented 3 years ago

@seth-shaw-unlv i tested undated (XXXX), ../1985, 1985/.., and a regular year like 1988 with the suggested configuration values. those all worked well. then i tested a range like 1986/1987 and got back an array - which seems right.

one thing i did wonder about was when i had multiple dates like 1986 in one and 1987 in another, i only got 1986. this seems unrelated to this PR so i am not concerned about it for this.

elizoller commented 3 years ago

@dannylamb i don't seem to be able to merge this PR due to the build not going on PHP 7.3 which is removed as part of this PR

seth-shaw-unlv commented 3 years ago

🎉