-
Notifications
You must be signed in to change notification settings - Fork 31
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
Fix PHP warnings when indexing in solr #114
Conversation
I'm curious what case would cause |
Yeah it was a misconfigured edtf year processor. |
If it only happens when misconfigured (bad config load? manual shenanigans?) then shouldn't we leave that error there, so that the site admin can see something is wrong and fix it? Failing silently (and failing to index the content) seems like it'll make it hard to find out about and correct your site. |
This is still an issue if a node bundle is being indexed that does not have an EDTF field set in the processor settings. See https://islandora.slack.com/archives/CM5PPAV28/p1713205364774709?thread_ts=1712939510.602939&cid=CM5PPAV28 for an example. |
In my case this happened because I have multiple content types that each use field_edtf_date_issued. The code as it is written now will loop through all of them, but the $bundle variable only matches on one, so the other ones all throw the $edtf doesn't exist error. You should be able to reproduce the errors in a fresh install by adding a new content type that uses field_edtf_date_issued and then trying to index some objects in Solr. |
Replacing my EDTFYear.php with the one in this PR allowed me to reindex solr without those errors, so I would say this works as advertised. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Based off of Josh's comment, I feel like it's been tested and works.
GitHub Issue: (link)
Release pull requests, etc.)
What does this Pull Request do?
Continues iterating over the loop if an EDTF value isn't defined
Was getting these warnings in my watchdog when indexing nodes in solr:
What's new?
Just declaring some variables that cause warning if the assumptions in the code aren't met.
How should this be tested?
I tested by just making the code changes and seeing the watchdog messages go away.
Additional Notes:
Any additional information that you think would be helpful when reviewing this
PR.
Interested parties
@Islandora/committers