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.

Sign up for GitHub

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

CTDA9-298: Add Islandora core feature dependency #4

Closed
wants to merge 4 commits into from
Closed

CTDA9-298: Add Islandora core feature dependency #4

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ed31a40
CTDA9-298: Add Islandora core feature dependency
May 2, 2023
91a8ac9
CTDA9-298: Add Islandora core feature dependency
May 2, 2023
b855faa
CTDA9-298: Fix vocabulary creation
May 4, 2023
ca7f171
CTDA9-298: Fix codesniffer things
May 4, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions islandora_hocr.info.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,3 +6,4 @@ core_version_requirement: ^8 || ^9
dependencies:
- islandora:islandora
- islandora_core_feature:islandora_core_feature
- drupal:taxonomy
11 changes: 11 additions & 0 deletions islandora_hocr.install
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,16 @@ use Drupal\taxonomy\Entity\Term;
* @throws \Drupal\Core\Entity\EntityStorageException
*/
function islandora_hocr_install() {
$entitytypes_fields = [
'taxonomy_term' => [
'islandora_media_use' => [
'field_external_uri',
],
],
];

islandora_hocr_ensure_fields_exist($entitytypes_fields);

// Generate hOCR media use term.
Term::create([
'parent' => [],
Expand All @@ -23,4 +33,5 @@ function islandora_hocr_install() {
'description' => 'hOCR Derivative Term',
'field_external_uri' => ['uri' => 'https://discoverygarden.ca/use#hocr'],
])->save();

}
Copy link
Contributor

Choose a reason for hiding this comment

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

This feels like a somewhat round-about way of attempting to account for a config dependency before creating content. Like, I feel that this is expecting there to be eventually references in code to this particular term (via the URI or whatever), so we need to create it, but this should not be the case: The URI should just be used in config. Like: Derivative actions are config. Views/filters for finding content with this term when building views and the like, where ultimately, a failure to find it just means there would be no results, but is config. Whatever eventual indexing processor stuff, should be config.

Which is to say: In the general case, strongly requiring the installation of a term in a particular vocab at module installation in the system is probably not desirable. Having a migration(/some other process) which users can invoke, which can either fail explicitly or be rolled-back/retried/updated if the term was not fully successfully created. With a migration, we could make use of the islandora migration_group that we already expect to invoke at site installation... ultimately, this is the kind of thing that could be part of a site-specific configuration, which seems to be the approach that the IF looking to pursue...

62 changes: 62 additions & 0 deletions islandora_hocr.module
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
<?php

/**
* @file
* Module file.
*/

/**
* Be sure the fields we're expecting are there, and create if not.
*
* ASSUMPTIONS:
* - You've already created these field configs, probably through the GUI.
* - They're stored in ../config.
*
* @param array $entitytypes_fields
* A 3-dimensional array. The first level is entity types, the second is
* bundles, and the third is the fields you want to ensure. E.g.
* [
* 'taxonomy_term' => [
* 'islandora_media_use' => [
* 'field_foo',
* 'field_bar',
* ],
* ],
* ].
*
* @throws \Drupal\Core\Entity\EntityStorageException
*/

use Drupal\Core\Site\Settings;
use Drupal\field\Entity\FieldConfig;
use Drupal\field\Entity\FieldStorageConfig;
use Drupal\Core\Config\FileStorage;

/**
* Make sure that fields and config exists.
*
* @throws \Drupal\Core\Entity\EntityStorageException
*/
function islandora_hocr_ensure_fields_exist(array $entitytypes_fields) {
$config_directory = new FileStorage(Settings::get('config_sync_directory'));
foreach ($entitytypes_fields as $entitytype => $bundles) {
foreach ($bundles as $bundle => $fields) {
$bundle_name = 'taxonomy.vocabulary.' . $bundle;
$config_record = $config_directory->read($bundle_name);
$config_storage = \Drupal::service('config.storage');
$config_storage->write($bundle_name, $config_record);
Comment on lines +44 to +47
Copy link
Contributor

Choose a reason for hiding this comment

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

In the general case, not sure we can(/should try to) really assert that this taxonomy exists? Like: If the config didn't happen to exist in the sync directory, how would this behave? Do we want to have to know how this would behave?

foreach ($fields as $field) {
$field_storage_name = 'field.storage.' . $entitytype . '.' . $field;
$config_record = $config_directory->read($field_storage_name);
if (!FieldStorageConfig::loadByName($config_record['entity_type'], $config_record['field_name'])) {
FieldStorageConfig::create($config_record)->save();
}
$field_config_name = 'field.field.' . $entitytype . '.' . $bundle . '.' . $field;
$config_record = $config_directory->read($field_config_name);
if (!FieldConfig::loadByName($config_record['entity_type'], $config_record['bundle'], $config_record['field_name'])) {
FieldConfig::create($config_record)->save();
}
}
}
}
Comment on lines +41 to +61
Copy link
Contributor

Choose a reason for hiding this comment

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

Unsure that we want to broach what is effectively dynamic config imports... like: Can see thing quickly getting overly complicated, if whatever configs in-turn define dependencies... like: Kind of gets into having to flood the graph, which could in the worst case result in a full import?

Otherwise, if we really wanted to, could possibly pursue something like the Config Importer and Tools module/project; however, yeah, still of the mind that strongly requiring the creation of this term at module installation is probably not desirable.

}