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

[BUG] Some EDTF date intervals fail to parse into ISO8601 #129

Open
patdunlavey opened this issue Jul 3, 2024 · 4 comments
Open

[BUG] Some EDTF date intervals fail to parse into ISO8601 #129

patdunlavey opened this issue Jul 3, 2024 · 4 comments
Assignees
Labels
bug Something isn't working

Comments

@patdunlavey
Copy link
Contributor

patdunlavey commented Jul 3, 2024

What steps does it take to reproduce the issue?

  • Enter an EDTF date interval in the form YYYY-MM/YYYY-MM, e.g. "1960-01/1960-02"
  • Attempt to parse it using controlled_access_terms\EDTFUtils::iso8601Value. Example:
 drush ev 'print_r(\Drupal\controlled_access_terms\EDTFUtils::iso8601Value("1960-02/1960-03"));'
 [warning] Undefined array key 3 EDTFUtils.php:344
T00:00:00%
  • It throws a php warning, and returns an invalid ISO 8601 date string "T00:00:00%"
  • We found this problem in our own custom code that attempts to parse values of an EDTF date field in a search_api processor.
  • I have not encountered other cases that produce wonky results, but I'm guessing there are some.

When does this issue occur?

  • Any place that tries to convert and EDTF date to ISO8601:
  • In the controlled_access_terms module, I see this in the following hook implementations: controlled_access_terms_jsonld_alter_normalized_array(), controlled_access_terms_tokens()

Any related open or closed issues to this bug report?

@patdunlavey patdunlavey added the bug Something isn't working label Jul 3, 2024
@patdunlavey
Copy link
Contributor Author

Dumb question: why don't we use professional-wiki/edtf's EDTF date parsing and conversion to do this? (We use EDTF\EdtfFactory::newParser() in \Drupal\controlled_access_terms\Plugin\search_api\processor\EDTFYear, but nowhere else that I can see.)

@seth-shaw-asu
Copy link
Member

  1. Thanks for reporting the bug; yes, we should fix it.
  2. Not a dumb question at all. The short answer is, most of the module was written before Professional Wiki's. I've wanted to replace chunks of our code with it for some time to reduce our tech-debt, but simply haven't had the time or need to do so yet. PRs to do so are welcome.

@patdunlavey
Copy link
Contributor Author

I'll see if I can get time to work on a PR. Assign to me?

@patdunlavey
Copy link
Contributor Author

Thanks Seth. I hope to get some time to work in this in a couple weeks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants