Allow to configure entity types to add path base fields (so that Pathauto can apply for them)

Forgot about default config and schema. This passes for me.

There's not much to see, just a bunch of checkboxes.

See also the referenced file_entity issue about what it means for a module that integrates with pathauto.

@todo:
* move the cache clears into a config save event listener, so it also works for config deployments and the file_entity patch can then be simplified too.
* Write a test just like the one in file_entity, with an entity_test entity.

It would, but then you'd get lots of options on the add pattern form that really don't make sense, like shortcut and menu links. But yes, now you get those options on the settings page.

We'd have to change for what we expose plugins, currently that only happens for entity types that have a path field.

It would make the code to check for which entity types the field should be added more complicated, same for the code that needs to detect a cache and clear caches. A bit worried about adding more dynamics and complexity there, but we can try that if you'd prefer that.

+++ b/src/Plugin/pathauto/AliasType/Broken.php
@@ -0,0 +1,23 @@
+    return $this->t('This block is broken or missing. You may be missing content or you might need to enable the original module.');

close, but it's not a block :)

Just return $this->t('Broken type').

+++ b/src/Form/PathautoSettingsForm.php
@@ -19,6 +25,53 @@ use Drupal\Core\Form\FormStateInterface;
   /**
+   * Case should be left as is in the generated path.
+   */
+  const CASE_LEAVE_ASIS = 0;
+
+  /**
+   * Case should be lowercased in the generated path.
+   */
+  const CASE_LOWER = 1;

not sure what this is, I think a copy & paste mistake from me. remove it.

Now we need tests for ($type = pick one that is not enabled by default. you might need to enable a module, e.g. block_content, comment, ..):

1. Ensure that $type is not available as an type when adding a pattern.
2. Ensure that Broken is not available there.
3. Enable the type.
4. Now create a pattern with that type.
5. Create an entity of that type, ensure the alias is generated.
6. Make sure the type can not be disabled now.
7. Delete $type from the setting through the API., call clearCachedDefinitions() on plugin manager.
8. visit the overview page, must not be broken, check for the broken type string.
9. Make sure that you can delete the pattern.
10. Enable $type again, disable it again while there is no pattern.

Yes because user is now configurable and you have to enable it. Just go to the settings page and do that. We could add an update function to automatically make user selected by default.

The settings page shows content entity types with a canonical link template, only those are supported at the moment.

1. +++ b/config/install/pathauto.settings.yml
   @@ -1,3 +1,5 @@
   +entity_types:
   +  user: true
   

   Lets add an update function that ensures that this is also set for existing sites to avoid the confusing broken type message when they update.

   You just have to get the editable config, set user as enabled and then save.

2. +++ b/config/schema/pathauto.schema.yml
   @@ -1,6 +1,11 @@
      type: config_object
      mapping:
   +    entity_types:
   +      label: Enabled entity types
   +      type: sequence
   +      sequence:
   +        type: boolean
   

   Lets make the key a bit more self-explaining and name it enabled_entity_types.

   I'm also wondering if we should simplify the structure to a list of entity types instead of using them as key with a dummy boolean value. The default config would then look like this:

   enabled_entity_types:
   - user

3. +++ b/pathauto.module
   @@ -143,9 +143,8 @@ function pathauto_field_widget_info_alter(&$widgets) {
   -  if ($entity_type->id() === 'user') {
   +  $config = \Drupal::config('pathauto.settings');
   +  if ($config->get('entity_types.' . $entity_type->id())) {
   

   it would mean that this would get a bit more complicated as we'd need an in_array() check.

4. +++ b/src/Form/PathautoAdminDelete.php
   @@ -73,15 +73,18 @@ class PathautoAdminDelete extends FormBase {
   +      if ($id !== 'broken') {
   +        // Broken shouldn't show on the aliases list.
   

   what about the bulk update form, doesn't that need the same?

   I would recommend that we hadd a getVisibleDefinitions() method to the alias type manager and use that, then you don't have to change anything here.

   Could also be a getVisibleOptions() and just return key => label.

Almost there I think.

1. +++ b/src/AliasTypeManager.php
   @@ -60,4 +60,21 @@ class AliasTypeManager extends DefaultPluginManager implements FallbackPluginMan
   +   * @return mixed[]
   +   *   An array of plugin definitions (empty array if no definitions were
   +   *   found). Keys are plugin IDs.
   

   we always return an array, so the type is just array. I think this is mixed in getDefinitions() because that can also return objects for some plugin types.

2. +++ b/src/AliasTypeManager.php
   @@ -60,4 +60,21 @@ class AliasTypeManager extends DefaultPluginManager implements FallbackPluginMan
   +  public function getVisibleDefinitions() {
   +    $definitions = $this->getCachedDefinitions();
   +    if (!isset($definitions)) {
   +      $definitions = $this->findDefinitions();
   +      $this->setCachedDefinitions($definitions);
   +    }
   +    unset($definitions['broken']);
   +    return $definitions;
   

   No need to repeat the implementation of getDefinitions() here. Just call $definitions = $this->getDefinitions()

3. +++ b/src/Form/PathautoAdminDelete.php
   @@ -69,22 +69,19 @@ class PathautoAdminDelete extends FormBase {
   +    // Next, iterate over all visible alias types
   +    $definitions = $this->aliasTypeManager->getVisibleDefinitions();
   

   Missing . at the end of the comment.

4. +++ b/src/Form/PathautoSettingsForm.php
   @@ -246,21 +246,23 @@ class PathautoSettingsForm extends ConfigFormBase {
            foreach ($value as $entity_type_id => $enabled) {
   -          if (!$enabled || !empty($form['entity_types'][$entity_type_id]['#disabled'])) {
   -            unset($value[$entity_type_id]);
   +          if ($enabled) {
   +            $enabled_entity_types[] = $entity_type_id;
              }
            }
   

   This means we will now store node in the list, even though that is not actually a type that can be enabled or disabled.

   There used to be only one disabled condition (not defined by us), now there are two. This means instead of just removing this check, you need to instead do the same check as in the disabled check about, with the provider.

   Make sure you test this, by making sure that node is not saved in the config when submitting this form.

One last thing:

Add a comment about that fairly complex condition and explain why we're doing it like that.

Works for me, will commit this soon.

@davereid: Not ignoring your idea to enable it automatically, but I think this is a good first step, we could still change that, but I think there would be some challenges when doing that (basically a cross-dependency between config entities and plugins that they use, which exist based on the config entities that exist).

Thinking about that, I think what would make sense is extend the descrption on the type selection and say that additional types can be enabled on the settings page, with a link that points there. Lets do that, setting back to needs work hopefully one final time for that ;)

Yes.

Great, committed.