Process text format filters for WYSIWYG fields

I'm attaching a patch for the current state of the MR as of #3 for use in Composer-based patching workflows.

If you're using cweagans/composer-patches, then you can install it by adding the following to your composer.json or composer.patches.json file depending on your configuration:

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-23/patternkit-14534373--wysiwyg-field-processing--5.patch"
      }
    }
  }
}

I got some reports that errors like the following were being logged when using patterns that had references:

Exception: Unknown schema type "" at cta.link.text in Drupal\patternkit\PatternFieldProcessorPluginManager->getPropertySchema() (line 147 of modules/contrib/patternkit/src/PatternFieldProcessorPluginManager.php).

I was able to reproduce this and have added updates to both reproduce this in tests and resolve it. The schema traversal in PatternFieldProcessorPluginManager now recognizes references and attempts to load the referenced schema. I've updated the MR, and I'm now attaching an updated patch for testing via Composer:

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-25/patternkit-14534373--wysiwyg-field-processing--7.patch"
      }
    }
  }
}

There is still some more improvement that could be done to better formalize and test the schema traversal as well as improving exception handling throughout that process, but this should get the basic functionality working.

After reviewing and testing with some others, we've identified the following issues that currently exist with the implementation in this ticket:

• WYSIWYG HTML fields may be first processed with the WYSIWYG processor turning the value into a render array, then when processed by the Token processor the array is not handled correctly and sent back as a string "Array"
• Traversal of schema references may not be happening as completely as expected resulting in processors not running on properties in referenced schemas

I'll be moving forward with writing tests for each of these issues then working to resolve them.

I've just pushed up some more work to fix the issues noted in #8. This work adds tests to confirm the issues with traversing the schema through references, and fixes them.

I, however, wasn't able to reproduce the issue with the WYSIWYG processor running before the Token processor and outputting an "Array" string instead, but I added notes to add future validation of return values from processors being applied and changed the WYSIWYG processor to always return a rendered out string.

This work is once again ready for review and testing.

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-26/patternkit-14534373--wysiwyg-field-processing--9.patch"
      }
    }
  }
}

Tested the work, is working great. Thanks for making this happened. One thing I've notice is that the `$tokenized[$token]` sometimes does not exist and throws a warning message

Notice: Undefined index: [url-nid:12345;title:'test | story | title test'] in Drupal\patternkit\Plugin\PatternFieldProcessor\TokenProcessor->apply() (line 83 of modules/contrib/patternkit/src/Plugin/PatternFieldProcessor/TokenProcessor.php).

I think we can check for that via

if (empty($tokenized[$token])) {
          continue;
}

Thanks for the testing, feedback, and recommendation @johnle! In the work I just pushed up, I've added both a test to reproduce the issue you noted and the check for the token replacement value as you described to fix the issue. As a result of this fix, the unprocessed token value remains visible in the final output. If that's not desired, we should re-evaluate the solution for it to account for that. It's worth noting, however, that I'm planning to rewrite the token replacement logic altogether to follow core usage more closely in a follow-up issue and we may consider revising the intended behavior for it there.

This should fix the issues that have been noted so far and make this work ready for review again, but there are a couple of other tweaks I'm considering adding before this gets merged in:

• Add custom exceptions to be thrown during schema traversal in PatternFieldProcessorPluginManager
• Pass in the property value as well to a field processor plugin's applies method to allow another level of logic in determining if a processor applies to properties being processed

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-27/patternkit-14534373--wysiwyg-field-processing--11.patch"
      }
    }
  }
}

I've just pushed up the changes for the items listed in #11. With that, I consider this work feature-complete for this issue, so we'll move forward with code review and community testing before anything gets merged in.

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-27/patternkit-14534373--wysiwyg-field-processing--12.patch"
      }
    }
  }
}

I've just pushed up another round of updates addressing various code review items that were posted as well as:

• Capture exceptions while traversing schemas and log them instead of letting them fail fatally. (In a future update, it might be nice to make this a configurable option or parameter.)
• Fail gracefully and output a placeholder text for a block that couldn't load the pattern successfully. (Previously it failed fatally.)

{
  ...
  "extra": {
    "patches": {
      "drupal/patternkit": {
        "#3281936: Process text format filters for WYSIWYG fields": "https://www.drupal.org/files/issues/2022-05-31/patternkit-14534373--wysiwyg-field-processing--13.patch"
      }
    }
  }
}

Further testing has highlighted issues in the schema traversal logic when encountering array property values like the following:

        "breakpoints": {
            "title": "Breakpoints",
            "type": "array",
            "options": {
                "hidden": true
            },
            "items": {
                "anyOf": [
                    {
                        "title": "",
                        "type": "string",
                        "enum": ["", "xxs", "xs", "sm", "md", "lg"]
                    }
                ]
            }
        },

This is also an issue in other array cases where the values in the anyOf may contain references as well. In either case, the error that is output looks like this:

Unrecognized schema at "items.1.settings"

Next Steps

1. Add tests to reproduce errors when traversing array properties.
2. Add tests to reproduce errors when traversing array properties containing references.
3. Expand the logic in PatternFieldProcessorPluginManager to handle traversal of array properties when encountered.
4. Repost with updates for further testing and review.

I've just pushed up a huge chunk of work refactoring the schema traversal process to use the external swaggest/json-schema library to handle schema validation and a lot of the traversal/inspection process.

This should make the schema traversal and processing much more robust than it was previously, and it should specifically fix the previously noted issues with array traversal. To address this, a new set of iterator classes and helpers have been added in src/Schema to handle the schema traversal in a more modular way. To keep the underlying implementation and use of the new library abstracted, the new services that have been created to support this have been marked as private.

Since this change now includes changes to the composer.json file including a new dependency, installation as a patch is no longer the most straightforward method of applying these changes for testing. To this end, the best way to install these changes locally is to specify the GitLab fork as a new repository in your composer.json file and install the specific branch in question. The related changes to your composer.json file are demonstrated below:

Notes:

• The custom package definition must be added above the standard packages.drupal.org composer reference. This ensures the package reference for "drupal/patternkit" is loaded from the fork first.
• You'll need to change the required version of the patternkit module to dev-3281936-process-text-format.
• After making these changes, you may install the fork with the command composer update drupal/patternkit.

{
    ...
    "repositories": [
        {
            "type": "package",
            "package": {
                "name": "drupal/patternkit",
                "type": "drupal-module",
                "version": "dev-3281936-process-text-format",
                "source": {
                    "url": "https://git.drupalcode.org/issue/patternkit-3281936.git",
                    "type": "git",
                    "reference": "3281936-process-text-format"
                }
            }
        },
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        },
        ...
    ],
    ...
    "require": {
        ...
        "drupal/patternkit": "dev-3281936-process-text-format",
        ...
    },
    ...
}

All remaining issues that have been reported to me have been addressed and manual testing reported to me seems to be successful. If we can get an approval on this and code review, then this should be good to merge in next week barring any unforeseen issues.

Merged into 9.1.x for inclusion in Beta 4.