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

Islandora/islandora#2338: Handle unset structured_text_term_uri option #1048

Conversation

xurizaemon
Copy link

@xurizaemon xurizaemon commented Aug 7, 2024

GitHub Issue: Islandora/documentation#2338

What does this Pull Request do?

Prevents Islandora emitting a warning when the structured_text_term_uri option is not configured in iiif_manifest view REST export.

What's new?

After viewing content via Mirador, mapped to node/%/book-manifest, when structured_text_term_uri and search_endpoint fields are null (default state), a warning is displayed or logged:

Warning: Undefined array key "structured_text_term_uri" in Drupal\islandora_iiif\Plugin\views\style\IIIFManifest->getStructuredTextTerm() (line 714 of /app/web/modules/contrib/islandora/modules/islandora_iiif/src/Plugin/views/style/IIIFManifest.php)

How should this be tested?

Steps to reproduce behaviour:

  1. At /admin/config/development/logging set " Error messages to display " to "All messages, with backtrace information"
  2. At /admin/structure/views/view/iiif_manifest/edit/rest_export_3, do not have structured_text_term_uri field set (default state).
  3. View an image in Mirador viewer
  4. Reload the page, or click to another page
  5. Error should be displayed

Steps to test:

  1. Apply patch
  2. Do steps to reproduce, do not see error at 5

Documentation Status

No docs required

Additional Notes:

n/a

Interested parties

@adam-vessey, @kayakr

@xurizaemon
Copy link
Author

I wonder about the need for $this->structuredTextTermMemoized at all, when we could instead check if $this->structuredTextTerm is NULL, but that's another issue.

However, this change also modifies the behaviour slightly:

  • Previously, if the lookup failed then $this->structuredTextTermMemoized would be TRUE and $this->structuredTextTerm would be NULL.
  • With this change, if the lookup failed then $this->structuredTextTermMemoized will be FALSE and $this->structuredTextTerm will remain NULL.

That looks more correct to me, but I'm new here, so please consider that behaviour also!

@xurizaemon
Copy link
Author

Closing, see #1047 (comment)

@xurizaemon xurizaemon closed this Aug 7, 2024
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.

1 participant