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

Update EDTF field for 2018 spec compliance #19

Merged
merged 12 commits into from
Mar 7, 2019
Merged

Update EDTF field for 2018 spec compliance #19

merged 12 commits into from
Mar 7, 2019

Conversation

seth-shaw-unlv
Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv commented Feb 6, 2019

GitHub Issue: /Islandora-CLAW/CLAW/issues/995

What does this Pull Request do?

Fixes the Widget and Formatter for the EDTF Field Type to match the 2018 spec.

What's new?

  • Adds an EDTFUtils class to consolidate EDTF-related code.
  • Updates the validation code (now in EDTFUtils) to meet the 2018 spec.
  • Updates the Widget and Formatter to use EDTFUtils and meet the spec.
  • Adds an update hook to bring existing content in EDTF fields to meet the 2018 spec.

How should this be tested?

  • Take a site with controlled_access_terms enabled and add some content to EDTF fields (repository items, taxonomy terms, whatever). They all comply with the 2012 spec.
  • Apply the PR
  • Run updates (either 'siteurl/update.php' or drush updb -y)
  • Clear cache.
  • Check your content. All the values should display correctly (even if exact wording and display has been adjusted) and viewing them in the edit window should show new signifiers. E.g. '?~' is now '%' and unspecified values 'u' are now 'X'.

Interested parties

@Islandora-CLAW/committers

rosiel
rosiel previously approved these changes Feb 21, 2019
Copy link
Member

@rosiel rosiel left a comment

Choose a reason for hiding this comment

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

Works - I tested that it successfully transformed my 2012-style ETDF to 2018-ese.

@seth-shaw-unlv
Copy link
Contributor Author

Ah! Hold up. Yes, it works for the dates; however, I didn't update EDTFConverter to expect 2018 compliant dates; it still assumes older ones.

@seth-shaw-unlv
Copy link
Contributor Author

Would you mind reviewing again, @rosiel, so we can get this merged? Thanks.

Copy link
Member

@rosiel rosiel left a comment

Choose a reason for hiding this comment

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

Tested again, ran it through some paces. Looks good!

@dannylamb
Copy link
Contributor

Please feel free to merge when ready @rosiel. No 24h wait period here 🎉

@rosiel rosiel merged commit 54611e0 into 8.x-1.x Mar 7, 2019
@rosiel rosiel deleted the issue-995 branch March 7, 2019 19:07
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