Problem/Motivation

For use cases including producing Drupal distributions, certain processing is needed to change configuration on export.

Currently we make two such changes in FeaturesManager::addPackageFiles():

        // The UUID is site-specfic, so don't export it.
        if ($entity_type_id = $this->configManager->getEntityTypeIdByName($name)) {
          unset($config['data']['uuid']);
        }
        // User roles include all permissions currently assigned to them. To
        // avoid extraneous additions, reset permissions.
        if ($config['type'] == 'user_role') {
          $config['data']['permissions'] = [];
        }

An additional case is #2599268: Enable filtering of recipients on export of contact forms.

However:

  • The current approach is not extensible; changes require new hard-coding.
  • The current approach isn't configurable; all processing is always applied.

Processing could be implemented through new configurable assignment plugins, since assignment plugins can access and alter the full configuration array, though such a plugin wouldn't actually do any assigning of configuration to packages, so is stretching the use case of assignment plugins. Also, a processor plugin doesn't need to be weighted in relation to assigners, since the processing won't affect assignments.

We already have one assignment plugin that fits these criteria for a processor plugin: 'optional'. It doesn't create any packages or assign any configuration; it just processes the configuration collection.

So we could add a new plugin type, 'processor', and convert the 'optional' assignment plugin into a processor plugin, but this would add considerable complexity and create two different plugin types that are structurally identical. Therefore, probably best to keep this as a new assignment plugin.

Proposed resolution

Convert the current hard-coded processing in FeaturesManager::addPackageFiles() into a new configurable assignment plugin.

Remaining tasks

User interface changes

API changes

Data model changes

Comments

nedjo created an issue. See original summary.

nedjo’s picture

Title: New 'processor' plugin type » Move export processing to a configurable plugin
Issue summary: View changes
nedjo’s picture

Status: Active » Needs review
StatusFileSize
new13.32 KB
new13.32 KB

Attached patch adds a new alter assignment plugin and moves the existing processing for _core, uuid, and user permissions to that plugin. Includes:

  • Configuration form to select which of this filtering to enable.
  • Update for existing features bundles.
  • Test coverage.

To test:

  • Install Features prior to this patch and create a custom features bundle.
  • Apply the patch and run updates to run the update included in this patch.
  • Examine the features bundle you created prior to patching and updating. Ensure that the Alter assignment plugin is enabled. Click the Configure link for the Alter plugin. Ensure all three checkboxes are checked.
  • Create a user role and assign permissions to it.
  • Create a feature, adding the role to it. Generate the feature. Examine the exported configuration (look for it in the config/install or config/optional directory). Expected result: it doesn't have any of the following keys: _core, uuid, permissions.
  • On the form for the feature bundle you created, click the Configure link for the Alter plugin. Uncheck all three checkboxes and save.
  • Create a feature, adding the role to it. Generate the feature. Examine the exported configuration (look for it in the config/install or config/optional directory). Expected result: it has all three of the following keys: _core, uuid, permissions.
nedjo’s picture

StatusFileSize
new0 bytes

Fixing up tests.

nedjo’s picture

StatusFileSize
new13.38 KB

Previous patch empty. This should be the right one.

The last submitted patch, 4: features-alter-2599278-4.patch, failed testing.

Status: Needs review » Needs work

The last submitted patch, 5: features-alter-2599278-5.patch, failed testing.

nedjo’s picture

Status: Needs work » Needs review

The test failure is from #2833258: Test failure from core change, so all tests introduced in this patch are passing.

jsst’s picture

Status: Needs review » Reviewed & tested by the community

I've installed this patch, ran update:execute and exported some features with varying combinations of the three alteration types. Works like a charm! Patch looks good from a technical and coding style perspective as well.

Couldn't verify the passing tests though.

  • nedjo committed c6e4b04 on 8.x-3.x
    Issue #2599278 by nedjo: Move export processing to a configurable plugin
    
nedjo’s picture

Status: Reviewed & tested by the community » Fixed

@jsst: thanks for testing and reporting back.

Applied.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

joshua.boltz’s picture

In what scenarios would it be dangerous to uncheck the "Strip out user permissions" option?

nedjo’s picture

@joshua.boltz

Since this issue was a feature request and is now closed, it works best if you can open a new issue for your support request. Thanks!