Make entity URL substitutions pluggable to support a wider variety of use cases.

I really like the idea here, but I'm not sure about the substitution weight and the way the manager is deciding which substitution to use. I might be missing something here but lets say that there are multiple substitutions that is applicable for an entity. How can a user choose which one to use? I think this will be the case for files with file entity.

• Link to the file entity
• Direct link to the file in the file entity.

In previews version of Linkit (Linkit 7.x-3.x) users was able to select "url_type" for the file entity url.

• Direct file link
• Download file link
• Entity view page

Maybe this is a corner case, but when dealing with file entities, there is no 90% use case as I see it, based on these three options.
However, we could make this a matcher setting.

All matchers is bound to a specific entity type, and we could therefor list available substitution plugins to choose from for each matcher plugin instance setting form. Basically use the SubstitutionManager::getPluginFromEntity() but return a List<Substitution> instead of a single Substitution.

Re: manager, I think it makes sense for the plugin manage to decide which plugin is appropriate for a given entity

I see your point here, but are there any disadvantages of letting the users decide this themselves?

Awesome. I will take a look at this over the weekend.

Thanks a lot Sam for all your work, I really appreciate this, and I really what this in the 5.x release. I just wanna make sure this is solid. I have done some mistakes in the previous versions that was very tricky to solve after the release. That is why I'm kind of picky with this.

I like the approach in the #14 patch. There are some minor coding standard issues, but nothing big.

1. +++ b/linkit.install
   @@ -122,3 +125,35 @@ function linkit_update_8500() {
   +function linkit_update_8501() {
   

   This update hook needs a test.

2. +++ b/linkit.install
   @@ -122,3 +125,35 @@ function linkit_update_8500() {
   +  foreach (FilterFormat::loadMultiple() as $id => $filter) {
   ...
   +  foreach (\Drupal\linkit\Entity\Profile::loadMultiple() as $profile) {
   

   I'm not sure this will work 100%.
   The API only uses the \Drupal::configFactory(); to load configuration.
   https://www.drupal.org/node/2535454

3. +++ b/linkit.install
   @@ -122,3 +125,35 @@ function linkit_update_8500() {
   +      $filter->save();
   ...
   +        $profile->save();
   

   Make sure to set $has_trusted_data to TRUE when you save configuration, so that the schema will not be checked. The reason this is important is that a later update might update the config schema, and you don't know when running updates if someone delayed and ran this update later when the schema was different.

One thing though that crossed my mind is that now we kind of force a lot of values into the href field in the dialog. It is entity type, entity id, and substitution id, and I feel like this bloat the field to mush. Also, the settings page for the matchers is already kind of bloated. Maybe a redesign should be made here. However, these should be separate issues. Just to be clear, there is nothing wrong with the patch, this is just general thoughts.

Summary:

• Fix the coding standard issues
• Add substitution_type" to the default profile.
• Fix the update hook and add a test to it.

Then I think we are ready to commit.

Here is a patch that

• fixes the coding standard issues
• adds substitution_type" to the default profile
• fixes the update hook
• adds a test for the 8501 update hook

The coding standard report still gives one error.

FILE: .../modules/linkit/src/Annotation/Substitution.php
----------------------------------------------------------------------
FOUND 1 ERROR AFFECTING 1 LINE
----------------------------------------------------------------------
 33 | ERROR | Class property $entity_types should use lowerCamel
    |       | naming without underscores
----------------------------------------------------------------------

Core does have this issue as well, but I would like to see if we can remove it from Linkit.

As said on IRC, I'm not that familiar with the 3 way merge.

Here is my comments, I have used my patch as the "base".

1. +++ b/config/optional/linkit.linkit_profile.default.yml
   @@ -15,4 +15,5 @@ matchers:
   +      include_unpublished: false
   +      substitution_type: canonical
   \ No newline at end of file
   

   Mine adds a newline. Think we should use mine here.

2. +++ b/linkit.install
   @@ -5,6 +5,9 @@
   +use Drupal\filter\Entity\FilterFormat;
   +use Drupal\linkit\Entity\Profile;
   

   Not sure these are needed now. If not, they should be removed. I have removed them in mine.

3. +++ b/linkit.install
   @@ -122,3 +125,34 @@ function linkit_update_8500() {
   +    $filter = $config_factory->getEditable($id);
   +    if ($allowed_html = $filter->get('filters.filter_html.settings.allowed_html')) {
   +      $allowed_html = str_replace('data-entity-uuid', 'data-entity-uuid data-entity-substitution', $allowed_html);
   +      $filter->set('filters.filter_html.settings.allowed_html', $allowed_html);
   +      $filter->save(TRUE);
   +    }
   

   This is better then mine. We should use this.

4. +++ b/linkit.install
   @@ -122,3 +125,34 @@ function linkit_update_8500() {
   +    $profile = $config_factory->getEditable($id);
   +    foreach ($profile->get('matchers') as $key => $matcher) {
   +      $settings = $profile->get('matchers.' . $key . '.settings');
   +      $settings['substitution_type'] = $matcher['id'] === 'entity:file' ? 'file' : 'canonical';
   +      $profile->set('matchers.' . $key . '.settings', $settings);
   +    }
   

   This is better then mine. We should use this.

5. +++ b/src/Plugin/Linkit/Substitution/Canonical.php
   @@ -0,0 +1,33 @@
   +  /**
   +   * {@inheritdoc}
   +   */
   +  public static function isApplicable(EntityInterface $entity) {
   +    return TRUE;
   +  }
   

   I have removed this from mine. This method is no longer used.

6. +++ b/src/SubstitutionManager.php
   @@ -0,0 +1,43 @@
   +  public function filterPluginDefinitions($definitions, $entity_id) {
   ...
   +  public function getApplicablePluginsOptionList($entity_id) {
   

   New: Rename this to $entity_type_id. Same for the interface and documentation.

This looks great, couple of minor observations

1. +++ b/config/optional/linkit.linkit_profile.default.yml
   @@ -15,4 +15,5 @@ matchers:
   \ No newline at end of file
   ...
   \ No newline at end of file
   

   whitespace issues

2. +++ b/src/Plugin/Linkit/Matcher/EntityMatcher.php
   @@ -205,6 +217,15 @@ class EntityMatcher extends ConfigurableMatcherBase {
   +    $form['substitution_type'] = [
   +      '#title' => $this->t('Substitution Type'),
   +      '#type' => 'select',
   +      '#default_value' => $this->configuration['substitution_type'],
   +      '#options' => $this->substitutionManager->getApplicablePluginsOptionList($this->targetType),
   +      '#description' => $this->t('Configure how the selected entity should be transformed into a URL for insertion.'),
   +      '#weight' => -49,
   

   We code hide this if count($options) === 1 using #access

@larowlan: 2. Good point.

@Sam152: Will you be able to do the 3 way merge?

Awesome! I made some small fixes. (New lines and doc)

woops wrong patch file.

Another try