Use ConfigEntity for features bundles

I'll give this some thought. We never thought about sharing individual bundles. Is there a reason your multiple developers just wouldn't share all the bundles for the project? Are you really changing a bunch of bundle assignment plugin settings that need to be shared?

A bundle consists of both a namespace as well as assignment plugin config settings. The assignment plugin stuff is only needed if you plan to create *new* features within that bundle. So unless you were actually creating new modules for panopoly or oa, you wouldn't care about those bundles. Those bundles will get created automatically because of the "bundle" key in the module.info.yml file for modules that are part of those distros.

So again, I don't see that your devs would need different bundles across your projects. You'd have some company-specific or project-specific bundles that you'd share, and you'd include any other auto-detected bundles with their defaults. Or just put your company and project bundles into the features.bundles yml file for your site and let the others just get auto-detected. The features.bundles.yml file is just the initial config for your site and not the "active" config that CMI uses.

Honestly, this is one of those cases where you should probably just be using CMI to deploy your bundle configuration between your different environments, rather than storing the features.bundle.yml into it's own feature "module". The features.bundle file isn't part of any actual functionality that you might be creating in your features modules (like a Photo Gallery feature, etc). You shouldn't be thinking of using Features as CMI like it was in Drupal 7. Use CMI for deploying configuration and use Features for making modules of specific functionality (Features!)

Sounds good. This change could also facilitate distro contributions in that a distro will be able to export and ship with its bundle settings (in e.g. a distroname_devel module) so that anyone wanting to contribute can access a standard set of plugin configuration for the distro.

Presumably we would remove our existing use of the variables features.assignment.yml and features.settings.yml as well as features.bundles.yml and replace all three with a single default bundle entity that we ship with. Then admins can clone that bundle to create their custom ones.

Sketching in implementation notes.

Raising to major. We should complete this prior to a beta, since it will break backward compatibility.

Yep, this would change the API, so it's a beta blocker.

Started to give it a try.

Just a start of a test.

1. +++ b/config/install/features.bundle.default.yml
   @@ -0,0 +1,84 @@
   +    enabled: false
   

   Shouldn't all these default plugins have "enabled: true"?

2. +++ b/config/install/features.bundle.default.yml
   @@ -0,0 +1,84 @@
   +    weight: '-2'
   

   Shouldn't this be an integer value instead of string?

Overall this looks pretty good. And it's amazing how much cleaner it makes the code. At least we already had Bundles as an object!

So when this is applied we get errors on the site, even after doing a "drush cr". Specifically, it doesn't look like it's loading the default bundle. The code:

      foreach (\Drupal::entityTypeManager()->getStorage('features_bundle')->loadMultiple() as $machine_name => $bundle) {

doesn't return any bundles.

Ahh, I have to uninstall Features and re-enable it. Should we bother with an update hook or just assume people using alpha versions can handle this?

Now, once Features is re-enabled, the error loading the site goes away, but on the Features page it no longer shows any auto-detected packages, even though I set the "enabled" flags to true. So my guess is there is still a missing refactor for handling the enabled plugins.

Thank you for providing some good feedback!

Ahh, I have to uninstall Features and re-enable it. Should we bother with an update hook or just assume people using alpha versions can handle this?

Yeah I guess this would be the easiest, good idea. Yeah I had the exact same problem locally and just could not bother dealing with it and reinstalled.

Now, once Features is re-enabled, the error loading the site goes away, but on the Features page it no longer shows any auto-detected packages, even though I set the "enabled" flags to true. So my guess is there is still a missing refactor for handling the enabled plugins.

Will look into it and maybe wr
ite some basic UI test coverage for it? Can't be bad to do that ...

OK, here is a new patch for people to try. Sorry I didn't do an interdiff.

Sorry, missed a file. Here is a better patch.

+++ b/config/schema/features.schema.yml
@@ -0,0 +1,31 @@
+    weights:
+      type: mapping
+      mapping:
+        type: int

Not sure "weights" is needed in the schema since each individual plugin has a weight propery

Looking good!

Testing the patch, I'm getting errors in creating a new bundle. Steps:

• Install Features UI.
• Navigate to admin/config/development/configuration/features/bundle. Bundle select says "--New--", but machine name is preset at "default".
• Enter a name for a new bundle and click "Save configuration". Message "Package assignment configuration saved." appears, but no new bundle is available for select.

If instead I manually change the machine name from "default" to "test", I'm able to create a new bundle. However:

• The bundle select doesn't include an option to return to the original default bundle.
• If I click "Remove bundle", I get
  

  Fatal error: Call to a member function getMachineName() on a non-object in /var/www/drupal80/modules/features/src/FeaturesAssigner.php on line 247

Stepping back briefly, on this form the functionality we're trying to retain is:

• Edit the default bundle and its assignment plugin configuration.
• Create a new bundle.
• Edit an existing bundle other than the default.

Suggested changes to achieve this:

• Treat the default bundle just like any other bundle in this form except that it can't be deleted. Specifically , add the default bundle to the "Bundle" select. However, if the default is selected, don't display the "Remove bundle" button. Probably we should add "Default" as the label (name) of the bundle, plus some help text for this page to explain bundles and what the default bundle is.
• If a user selects "--New--" for bundle, the form reloads empty.

I'm working on making the needed updates to Drupal\features_ui\Form\AssignmentConfigureForm.

Treat the default bundle just like any other bundle in this form except that it can't be deleted. Specifically , add the default bundle to the "Bundle" select. However, if the default is selected, don't display the "Remove bundle" button. Probably we should add "Default" as the label (name) of the bundle, plus some help text for this page to explain bundles and what the default bundle is.

That sounds like an actual better way to communicate what is going on, than what currently the UI implements. The concept of the default was really not clear for me,
when I first played around with it.

Here's an update addressing the issues in #2.

I've manually run through the basics of bundle creation and configuration and not found remaining issues.

Notes on a few of the changes:

1. +++ b/modules/features_ui/src/Form/AssignmentBaseForm.php
   @@ -55,10 +55,9 @@ class AssignmentBaseForm extends AssignmentFormBase {
   +    // Merge in types selections.
   +    $settings = $this->currentBundle->getAssignmentSettings(self::METHOD_ID);
   +    $settings['types'] = $form_state->getValue('types');
        $this->currentBundle->setAssignmentSettings(self::METHOD_ID, $settings)->save();
   

   In assignment plugin configuration form submit handlers, merge in changes to assignment plugin settings so as not to lose status and weight settings.

2. +++ b/modules/features_ui/src/Form/AssignmentConfigureForm.php
   @@ -23,6 +25,11 @@ use Symfony\Component\DependencyInjection\ContainerInterface;
   +  const NEW_BUNDLE_SELECT_VALUE = 'new';
   

   Add constant AssignmentConfigureForm::NEW_BUNDLE_SELECT_VALUE and use this to designate the case where we're adding a new bundle.

3. +++ b/modules/features_ui/src/Form/AssignmentConfigureForm.php
   @@ -174,19 +208,41 @@ class AssignmentConfigureForm extends FormBase {
   +    if ($bundle_name == self::NEW_BUNDLE_SELECT_VALUE) {
   +      $default_values = [
   +        'bundle_select' => self::NEW_BUNDLE_SELECT_VALUE,
   +        'name' => '',
   +        'machine_name' => '',
   +        'description' => '',
   +        'is_profile' => FALSE,
   +        'profile_name' => '',
   +      ];
   +    }
   +    else {
   +      $default_values = [
   +        'bundle_select' => $current_bundle->getMachineName(),
   +        'name' => $current_bundle->getName(),
   +        'machine_name' => $current_bundle->getMachineName(),
   +        'description' => $current_bundle->getDescription(),
   +        'is_profile' => $current_bundle->isProfile(),
   +        'profile_name' => $current_bundle->getProfileName(),
   +      ];
   +    }
   

   Change default values for bundle form such that they are empty for a new bundle. Also alter form generation logic to conditionally handle default bundle--for example, disable bundle label (name) form element for default bundle.

4. +++ b/src/FeaturesAssigner.php
   @@ -21,8 +22,6 @@ use Drupal\Core\StringTranslation\StringTranslationTrait;
   -  const DEFAULTBUNDLE = 'default';
   

   Move the default bundle constant from FeaturesAssigner to FeaturesBundleInterface and use it whenever we are referring to the default bundle.

5. +++ b/src/FeaturesAssigner.php
   @@ -282,11 +281,10 @@ class FeaturesAssigner implements FeaturesAssignerInterface {
   +  public function createBundleFromDefault($machine_name, $name = NULL, $description = NULL, $is_profile = FALSE, $profile_name = NULL) {
   +    // Clone the default bundle to get its default configuration.
   +    $bundle = clone $this->getBundle(FeaturesBundleInterface::DEFAULT_BUNDLE);
   +
   

   Rename FeaturesAssignerInterface::createBundle() to ::createBundleFromDefault() and begin by cloning the default bundle. This ensures we start with the default settings, including assignment plugin configuration.

To clear the way for other issues, I've gone ahead and committed this. Thanks all! Any fixes can be done in follow-up issues.

I've moved further refactoring to potential followup in #2621658: FeaturesBundle should implement EntityWithPluginCollectionInterface.

The need for help text on bundles is already captured in #2490818: Add help to clarify purpose and usage of bundles.

Awesome, thank you!