ISSUE-221 --- ADO Type condition plugin. #222

patdunlavey · 2022-08-16T20:40:32Z

This is a fairly simple condition plugin that permits conditioning block display and contexts on the ADO Type of the current node.

See #221

Potential changes/improvements:

Use a solr facet search to create the list of ADO Types for the configuration form, rather than depending on the presence of the two webform options lists.
Would changing to a solr search for the current node's ADO type be more performant? Or caching?
I didn't bother implementing javascript to display the context summary, which seems superfluous. Is that needed?

Requesting review from @aksm

aksm · 2022-08-16T20:42:41Z

@patdunlavey Thanks! Will look over first thing tomorrow.

aksm · 2022-08-17T20:12:13Z

Hi @patdunlavey I've begun looking at and testing this but got sidetracked. Will circle back tomorrow.

Either way it's probably best to have a second look from Diego when he gets back since my novice eye will likely miss something.

DiegoPino · 2022-08-22T13:23:28Z

src/Plugin/Condition/AdoType.php

+    $node = $this->getContextValue('node');
+
+    // Get the ADO type from the entity.
+    if ($node->hasField('field_descriptive_metadata')) {


@patdunlavey this here needs to change. There are archipelagos in the wild (e.g the ones using EADs) that have different field/names. The way we do this in almost every spot is using this helper methods.

An example:

if ($sbf_fields = \Drupal::service('strawberryfield.utility')->bearsStrawberryfield($entity))

But a fuller example is here: (The View Mode resolver)

format_strawberryfield/src/ViewModeResolver.php

Lines 95 to 110 in 8f4a772

$ado_types = [];

if ($sbf_fields = $this->strawberryfieldUtility->bearsStrawberryfield($entity)) {

foreach ($sbf_fields as $field_name) {

/* @var \Drupal\strawberryfield\Plugin\Field\FieldType\StrawberryFieldItem $field */

$field = $entity->get($field_name);

if (!$field->isEmpty()) {

foreach ($field->getIterator() as $delta => $itemfield) {

/** @var \Drupal\strawberryfield\Plugin\Field\FieldType\StrawberryFieldItem $itemfield */

$flat_values = (array) $itemfield->provideFlatten();

if (isset($flat_values['type'])) {

$ado_types = array_merge($ado_types, (array) $flat_values['type']);

}

}

}

}

}

Which also leads to:

Should we be aligned with how the View Mode resolver works? Using all "type" key values found in the JSON (independent of the hierarchy).

Good suggestion. I'll just "borrow" the view mode resolver code to get the type(s).

@patdunlavey thanks so much

DiegoPino · 2022-08-22T13:25:48Z

src/Plugin/Condition/AdoType.php

+   */
+  public function buildConfigurationForm(array $form, FormStateInterface $form_state) {
+    $type_options = [];
+    foreach(['schema_org_creative_works', 'schema_org_cw_collections'] as $webform_option_list_id) {


@patdunlavey question.

What if we make this a Textarea, like the "path" conditions work? Reason is, maybe users have more Webform options where to source the "types" from, or named differently. This of course could be also a setting provided by the Webform Strawberryfield module, since we also "sadly" do this hardcoded in AMI ... what do you think? We could also have a multi select where you choose first which webform_option (a select box) and then the type from there?

I like the idea of making it a textarea field where the user in instructed to provide a comma-separated list of ADO types. Keep it simple AND fool-proof!

@patdunlavey great. Maybe even a One Per line? That way we can reuse the code that splits/cleans that up from the other conditions that do that already and will also be a "I know how/common place" moment for users already understanding the multiple paths, each one in a single line, condition that comes standard with Drupal

DiegoPino · 2022-08-22T14:21:48Z

@patdunlavey re: performance. I think if using the suggested method if would not be too hard on the Server. Mostly because it will discard immediately any Bundle that does not carry a SBF and once you access the flatten JSON it will be either already in place (the static cache of the field will do that for you) or will provide by just accessing the data the static cache for whatever comes next (render/etc). let's give this a try, if we feel this adds a delay we can add an option to use Solr, but that requires config (like which field is holding the type values in a particular index) and might be an overkill.

DiegoPino · 2022-08-22T14:42:54Z

@patdunlavey last request. This requires a schema YML file too. Once you are settled with how the type config/text area is going to be stored, please attach it. You can use some other condition yml file as an example to make it easier. Thanks a lot

- Changed type selection widget from checkboxes populated from hard-coded webform options lists to a simple textarea with instructions to enter one ADO type per line. - Added option to match type values from everywhere in the metadata, not just at the top level.'

patdunlavey · 2022-08-22T16:20:00Z

Sorry @DiegoPino, I just saw your "last request" after I pushed up new changes.

I'm not sure I understand what you're talking about re. schema yml file. The storage scheme is exportable to a context.context.[context_name].yml file. I'm storing the ado_types as an array of strings.

DiegoPino · 2022-08-22T16:23:14Z

Hi, yes, but i “think” you still need to declare the plugin’s config as its own? Let me check…

On Mon, Aug 22, 2022 at 12:20 PM Pat Dunlavey ***@***.***> wrote: Sorry @DiegoPino <https://github.com/DiegoPino>, I just saw your "last request" after I pushed up new changes. I'm not sure I understand what you're talking about re. schema yml file. The storage scheme is exportable to a context.context.[context_name].yml file. I'm storing the ado_types as an array of strings. — Reply to this email directly, view it on GitHub <#222 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABU7ZZ7XXBJBK4S73WN63DLV2OSDXANCNFSM56XFSYPA> . You are receiving this because you were mentioned.Message ID: ***@***.***>

-- Diego Pino Navarro Digital Repositories Developer Metropolitan New York Library Council (METRO)

patdunlavey · 2022-08-22T16:24:44Z

Incidentally, this feature is one added configuration field away from being able to condition a context on the value of any field in the metadata. But I sense scope-creep!

DiegoPino

Also @patdunlavey regarding the schema that is missing.

Condition plugins do have one for their configurations.

See example

condition.plugin.entity_bundle:*:
  type: condition.plugin
  mapping:
    bundles:
      type: sequence
      sequence:
        type: string

And this one from a contributed module (groups)

https://www.drupal.org/files/issues/2018-12-14/group-group_type_condition_plugin_config_schema-3020554-2.patch

DiegoPino · 2022-08-22T16:29:49Z

src/Plugin/Condition/AdoType.php

+    $entity = $this->getContextValue('node');
+
+    $ado_types = [];
+    if ($sbf_fields = \Drupal::service('strawberryfield.utility')->bearsStrawberryfield($entity)) {


@patdunlavey can you inject this service please? Since you are already using ContainerFactoryPluginInterface that would be great (sorry you know how my code reviews are... so sorry)

Dang! You're going to make me learn how to program in Drupal!! Ok, I think I did this right (copying from another example in your code).

Re. the configs, this seems to come for free with context plugins. When I do drush cex, I get a context.context.[context_name].yml file. Or am I misunderstanding you?

Is it possible that the annotation makes the schema definition unnecessary?

DiegoPino · 2022-08-22T16:35:37Z

Incidentally, this feature is one added configuration field away from being able to condition a context on the value of any field in the metadata. But I sense scope-creep!

Yes, was thinking what if we add a JMESPATH and then BUM! hahaha. But what if we keep an open Ticket as a secondary Condition that extends this for further down the road? I kinda like the idea. E.g only show a MAP Block, IF there are lat/longitudes

…ado_types value.

DiegoPino · 2022-08-22T17:12:58Z

@patdunlavey you get the "saving" for free for the YML but you still need to add the definition (schema). Append it like the example (your two config entries) to https://github.com/esmero/format_strawberryfield/blob/1.0.0/config/schema/format_strawberryfield.schema.yml

…s request. I have ABSOLUTELY NO IDEA what this accomplishes or is supposed to accomplish. So maybe it does what it's supposed to?? What I can say is that it doesn't seem to break anything, at least that I could find.

patdunlavey · 2022-08-22T17:57:07Z

@DiegoPino I have added something to format_strawberryfield.schema.yml, but (as my commit message says), I have absolutely no idea what it's supposed to accomplish, how to tell if it's correct, etc.

DiegoPino · 2022-08-22T18:40:33Z

config/schema/format_strawberryfield.schema.yml

+  label: 'ADO Type Condition'
+  mapping:
+    ado_types:
+      type: array


Hey, almost! this is like this (there is no array type)

type: sequence
sequence:
type: string

Ok, updated.

config/schema/format_strawberryfield.schema.yml

… not a valid mapping field, so changed to 'sequence'

DiegoPino

@patdunlavey ok code looks good. Let me get this for a spin and test? If all good I merge tomorrow PM. Thanks so much, also thanks for taking care of all my concerns (but I might come back if I find an issue ok?)

src/Plugin/Condition/AdoType.php

DiegoPino · 2022-08-26T18:46:52Z

@patdunlavey ping, sorry. Could you please clean up the unused USE statements? All the rest is ready and I would love to merge your great pull. Thanks!!

'

patdunlavey · 2022-08-27T11:28:10Z

Unused use Drupal\webform\Entity\WebformOptions; has been removed. Thanks!

ISSUE-221 --- First try for an ADO Type condition plugin.

18a3827

patdunlavey requested a review from aksm August 16, 2022 20:40

patdunlavey self-assigned this Aug 16, 2022

DiegoPino reviewed Aug 22, 2022

View reviewed changes

DiegoPino requested changes Aug 22, 2022

View reviewed changes

patdunlavey added 2 commits August 22, 2022 12:35

ISSUE-221 --- Whoops, it would help if I actually stored the recurse_…

42c21d4

…ado_types value.

ISSUE-221 --- injected the strawberryfield.utility service.

6649e41

DiegoPino reviewed Aug 22, 2022

View reviewed changes

config/schema/format_strawberryfield.schema.yml Show resolved Hide resolved

patdunlavey added 2 commits August 22, 2022 14:47

ISSUE-221 --- fix for condition.plugin.ado_type_condition. 'array' is…

5e9379c

… not a valid mapping field, so changed to 'sequence'

ISSUE-221 --- make boolean everywhere

f5ba1fa

DiegoPino approved these changes Aug 22, 2022

View reviewed changes

DiegoPino changed the base branch from 1.0.0 to 1.1.0 August 24, 2022 00:56

aksm approved these changes Aug 24, 2022

View reviewed changes

DiegoPino reviewed Aug 24, 2022

View reviewed changes

src/Plugin/Condition/AdoType.php Outdated Show resolved Hide resolved

ISSUE-221 --- removed unused 'use Drupal\webform\Entity\WebformOptions;

2ba0d20

'

DiegoPino merged commit 4000321 into esmero:1.1.0 Aug 27, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ISSUE-221 --- ADO Type condition plugin. #222

ISSUE-221 --- ADO Type condition plugin. #222

patdunlavey commented Aug 16, 2022

aksm commented Aug 16, 2022

aksm commented Aug 17, 2022

DiegoPino Aug 22, 2022

patdunlavey Aug 22, 2022

DiegoPino Aug 22, 2022

DiegoPino Aug 22, 2022 •

edited

Loading

patdunlavey Aug 22, 2022

DiegoPino Aug 22, 2022

DiegoPino commented Aug 22, 2022

DiegoPino commented Aug 22, 2022

patdunlavey commented Aug 22, 2022

DiegoPino commented Aug 22, 2022 via email

patdunlavey commented Aug 22, 2022

DiegoPino left a comment

DiegoPino Aug 22, 2022

patdunlavey Aug 22, 2022

patdunlavey Aug 22, 2022

patdunlavey Aug 22, 2022

DiegoPino commented Aug 22, 2022

DiegoPino commented Aug 22, 2022

patdunlavey commented Aug 22, 2022

DiegoPino Aug 22, 2022

patdunlavey Aug 22, 2022

DiegoPino left a comment

DiegoPino commented Aug 26, 2022

patdunlavey commented Aug 27, 2022

	$ado_types = [];
	if ($sbf_fields = $this->strawberryfieldUtility->bearsStrawberryfield($entity)) {
	foreach ($sbf_fields as $field_name) {
	/* @var \Drupal\strawberryfield\Plugin\Field\FieldType\StrawberryFieldItem $field */
	$field = $entity->get($field_name);
	if (!$field->isEmpty()) {
	foreach ($field->getIterator() as $delta => $itemfield) {
	/** @var \Drupal\strawberryfield\Plugin\Field\FieldType\StrawberryFieldItem $itemfield */
	$flat_values = (array) $itemfield->provideFlatten();
	if (isset($flat_values['type'])) {
	$ado_types = array_merge($ado_types, (array) $flat_values['type']);
	}
	}
	}
	}
	}

ISSUE-221 --- ADO Type condition plugin. #222

ISSUE-221 --- ADO Type condition plugin. #222

Conversation

patdunlavey commented Aug 16, 2022

aksm commented Aug 16, 2022

aksm commented Aug 17, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoPino Aug 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoPino commented Aug 22, 2022

DiegoPino commented Aug 22, 2022

patdunlavey commented Aug 22, 2022

DiegoPino commented Aug 22, 2022 via email

patdunlavey commented Aug 22, 2022

DiegoPino left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoPino commented Aug 22, 2022

DiegoPino commented Aug 22, 2022

patdunlavey commented Aug 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DiegoPino left a comment

Choose a reason for hiding this comment

DiegoPino commented Aug 26, 2022

patdunlavey commented Aug 27, 2022

DiegoPino Aug 22, 2022 •

edited

Loading