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

Correct EDTF data type in JSON-LD #998 #18

Merged
merged 9 commits into from
Jan 30, 2019
Merged

Correct EDTF data type in JSON-LD #998 #18

merged 9 commits into from
Jan 30, 2019

Conversation

seth-shaw-unlv
Copy link
Contributor

@seth-shaw-unlv seth-shaw-unlv commented Jan 4, 2019

GitHub Issue: /Islandora/documentation/issues/998

See also the thread on /Islandora/documentation/issues/916

What does this Pull Request do?

Enables us to dynamically set the JSON-LD date value type based on the field's value. E.g. the EDTF string "2019?" would become "2019" with the data type gYear while the string value "~2019-01" would become "2019-01" with the data type gYearMonth, etc.

Note, this adds the value with the appropriate date type along-side the existing string value. This allows us to store the actual EDTF value in our triple-store or repository (as strings) AND have a date type we can run queries against.

What's new?

  • Changes controlled_access_terms_jsonld_alter_normalized_array to such that fields of the type edtf get a date value, correctly typed, in addition to the string value.
  • Changes the controlled_access_terms_default_configurations that use the text-based EDTF to the new field with corresponding widget and formatter.

How should this be tested?

  • Create any Person with birth dates using an EDTF value and view their JSON-LD. You will see a string value.
  • Apply the PR
  • Run the module update (drush updb -y)
  • Check the person's JSON-LD again. You will see both the string value and the correctly formatted and typed date version.

Additional Notes:

This leaves in place the existing text_edtf widget and formatters; however, any field that uses the EDTF widget will be changed to the new EDTF Field Type. Once this is merged we can do separate PRs to update islandora_demo.

Interested parties

@Islandora-CLAW/committers (esp @DiegoPino )

- update normalized array hook to update data type for edtf fields
- update default configurations to use edtf field
Natkeeran
Natkeeran previously approved these changes Jan 18, 2019
Copy link
Contributor

@Natkeeran Natkeeran left a comment

Choose a reason for hiding this comment

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

Tested this. Works as described.

{"@graph":[{"@id":"http:\/\/localhost:8000\/taxonomy\/term\/26?_format=jsonld","@type":["http:\/\/schema.org\/Person"],"http:\/\/schema.org\/name":[{"@value":"John Adam 2","@language":"en"}],"http:\/\/schema.org\/birthDate":[{"@value":"193u","@type":"http:\/\/www.w3.org\/2001\/XMLSchema#string"},{"@value":"1930","@type":"http:\/\/www.w3.org\/2001\/XMLSchema#gYear"}]}]}
{"@graph":[{"@id":"http:\/\/localhost:8000\/taxonomy\/term\/26?_format=jsonld","@type":["http:\/\/schema.org\/Person"],"http:\/\/schema.org\/name":[{"@value":"John Adam 2","@language":"en"}],"http:\/\/schema.org\/birthDate":[{"@value":"2004-06?","@type":"http:\/\/www.w3.org\/2001\/XMLSchema#string"},{"@value":"2004-06","@type":"http:\/\/www.w3.org\/2001\/XMLSchema#gYearMonth"}]}]}

@seth-shaw-unlv
Looks like the current support is for Level 1. Would be good to document/clarify that: http://www.loc.gov/standards/datetime/pre-submission.html.

Additional Notes:
This PR uses this controlled_access_terms_jsonld_alter_normalized_array hook to modify the jsonld to add a normalized date triple. It implements EDTFConverter::dateIso8601Value function to get the normalized value.

@Natkeeran
Copy link
Contributor

@DiegoPino, Please do a final review and merge. Thanks.

@DiegoPino
Copy link

DiegoPino commented Jan 18, 2019 via email

also remove unused storage config
@seth-shaw-unlv
Copy link
Contributor Author

@Natkeeran, as per Islandora/documentation#1011 (comment) I added an update hook that supports in-place updates to existing String fields that use the EDTF widget. I will update the testing instructions.

@dannylamb
Copy link
Contributor

@seth-shaw-unlv I can give this a whirl today

@dannylamb
Copy link
Contributor

@seth-shaw-unlv I pulled down the PR and ran drush updb but am running into this:

vagrant@claw:/var/www/html/drupal/web/modules/contrib/controlled_access_terms$ sudo drush updb -y
 ------------------------- ----------- --------------- ------------------------------------------------------------------ 
  Module                    Update ID   Type            Description                                                       
 ------------------------- ----------- --------------- ------------------------------------------------------------------ 
  controlled_access_terms   8002        hook_update_n   Change fields using the EDTF Widget to the new EDTF Field Type.   
 ------------------------- ----------- --------------- ------------------------------------------------------------------ 

 // Do you wish to run the specified pending updates?: yes.                                                             

 [notice] Update started: controlled_access_terms_update_8002
 [error]  Error: Class 'FieldConfig' not found in controlled_access_terms_update_8002() (line 100 of /var/www/html/drupal/web/modules/contrib/controlled_access_terms/controlled_access_terms.module) #0 /var/www/html/drupal/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(192): controlled_access_terms_update_8002(Array)
#1 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(235): Drush\Commands\core\UpdateDBCommands->updateDoOne('controlled_acce...', 8002, Array, Object(DrushBatchContext))
#2 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(183): _drush_batch_worker()
#3 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(95): _drush_batch_command('6')
#4 /var/www/html/drupal/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(144): drush_batch_command('6')
#5 [internal function]: Drush\Commands\core\UpdateDBCommands->process('6', Array)
#6 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(235): call_user_func_array(Array, Array)
#7 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(181): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#8 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(150): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#9 /var/www/html/drupal/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(404): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#10 /var/www/html/drupal/vendor/symfony/console/Command/Command.php(251): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/drupal/vendor/symfony/console/Application.php(964): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/drupal/vendor/symfony/console/Application.php(248): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/drupal/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#14 /var/www/html/drupal/vendor/drush/drush/src/Runtime/Runtime.php(112): Symfony\Component\Console\Application->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 /var/www/html/drupal/vendor/drush/drush/src/Runtime/Runtime.php(41): Drush\Runtime\Runtime->doRun(Array)
#16 /var/www/html/drupal/vendor/drush/drush/drush.php(66): Drush\Runtime\Runtime->run(Array)
#17 /var/www/html/drupal/vendor/drush/drush/includes/preflight.inc(17): require('/var/www/html/d...')
#18 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#19 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#20 {main}. 
Error: Class 'FieldConfig' not found in /var/www/html/drupal/web/modules/contrib/controlled_access_terms/controlled_access_terms.module on line 100 #0 /var/www/html/drupal/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(192): controlled_access_terms_update_8002(Array)
#1 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(235): Drush\Commands\core\UpdateDBCommands->updateDoOne('controlled_acce...', 8002, Array, Object(DrushBatchContext))
#2 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(183): _drush_batch_worker()
#3 /var/www/html/drupal/vendor/drush/drush/includes/batch.inc(95): _drush_batch_command('6')
#4 /var/www/html/drupal/vendor/drush/drush/src/Commands/core/UpdateDBCommands.php(144): drush_batch_command('6')
#5 [internal function]: Drush\Commands\core\UpdateDBCommands->process('6', Array)
#6 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(235): call_user_func_array(Array, Array)
#7 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(181): Consolidation\AnnotatedCommand\CommandProcessor->runCommandCallback(Array, Object(Consolidation\AnnotatedCommand\CommandData))
#8 /var/www/html/drupal/vendor/consolidation/annotated-command/src/CommandProcessor.php(150): Consolidation\AnnotatedCommand\CommandProcessor->validateRunAndAlter(Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#9 /var/www/html/drupal/vendor/consolidation/annotated-command/src/AnnotatedCommand.php(404): Consolidation\AnnotatedCommand\CommandProcessor->process(Object(Symfony\Component\Console\Output\ConsoleOutput), Array, Array, Object(Consolidation\AnnotatedCommand\CommandData))
#10 /var/www/html/drupal/vendor/symfony/console/Command/Command.php(251): Consolidation\AnnotatedCommand\AnnotatedCommand->execute(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#11 /var/www/html/drupal/vendor/symfony/console/Application.php(964): Symfony\Component\Console\Command\Command->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#12 /var/www/html/drupal/vendor/symfony/console/Application.php(248): Symfony\Component\Console\Application->doRunCommand(Object(Consolidation\AnnotatedCommand\AnnotatedCommand), Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#13 /var/www/html/drupal/vendor/symfony/console/Application.php(148): Symfony\Component\Console\Application->doRun(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#14 /var/www/html/drupal/vendor/drush/drush/src/Runtime/Runtime.php(112): Symfony\Component\Console\Application->run(Object(Drush\Symfony\LessStrictArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
#15 /var/www/html/drupal/vendor/drush/drush/src/Runtime/Runtime.php(41): Drush\Runtime\Runtime->doRun(Array)
#16 /var/www/html/drupal/vendor/drush/drush/drush.php(66): Drush\Runtime\Runtime->run(Array)
#17 /var/www/html/drupal/vendor/drush/drush/includes/preflight.inc(17): require('/var/www/html/d...')
#18 phar:///usr/local/bin/drush/bin/drush.php(141): drush_main()
#19 /usr/local/bin/drush(10): require('phar:///usr/loc...')
#20 {main}
Error: Class 'FieldConfig' not found in controlled_access_terms_update_8002() (line 100 of /var/www/html/drupal/web/modules/contrib/controlled_access_terms/controlled_access_terms.module).
 [error]  Drush command terminated abnormally due to an unrecoverable error. 
 [success] Finished performing updates.

Did I need to import the feature first, maybe?

@seth-shaw-unlv
Copy link
Contributor Author

I think you are right, @dannylamb. Please try that and let me know.

@seth-shaw-unlv
Copy link
Contributor Author

No, wait, @dannylamb . I missed some use statements.... when did I lose them? And why was Travis okay?

@seth-shaw-unlv
Copy link
Contributor Author

controlled_access_terms.module should also include:

use Drupal\field\Entity\FieldStorageConfig;
use Drupal\field\Entity\FieldConfig;

You can add them yourself, but it will take me a minute to retest before I commit the change.

@dannylamb
Copy link
Contributor

@seth-shaw-unlv Ok, I'll try again 👍

@dannylamb
Copy link
Contributor

@seth-shaw-unlv I can confirm it's working now. Looks good to me! I now have strings and gYears in my jsonld

screenshot from 2019-01-30 16-42-18

Copy link
Contributor

@dannylamb dannylamb left a comment

Choose a reason for hiding this comment

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

:shipit:

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.

4 participants