Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes #428 Config Distro Import Ignore Bug #653

Merged
merged 1 commit into from
Apr 9, 2021
Merged

Fixes #428 Config Distro Import Ignore Bug #653

merged 1 commit into from
Apr 9, 2021

Conversation

tadean
Copy link
Contributor

@tadean tadean commented Apr 9, 2021

This PR attempts to provide a workaround for a current bug in the config_sync/config_distro ecosystem. The symptom of this problem is that when distribution updates are imported, any modules that are deselected (eg. you choose not to import their updates) still import anyway.

Description

This PR provides a patch that hooks the storage comparer used in the Config Import process up between the site's sync storage and active storage.

Normally config_distro's import form integrates with the sync storage indirectly, through the new transformation API. The config_sync/config_distro ecosystem has been trying to move towards using the transformation API but it appears that perhaps something in the config_filter layer is being cached or not regenerated during a plugin derivation process.

This needs a little more investigation for a real fix as this is a bandaid fix for something that seems going amiss with config_filter using the transformation API.

Related Issue

#428

How Has This Been Tested?

  • Test locally, not practical in Probo.ci
  • Build site and install Drupal as normal
  • Make edits to two configuration files for Quickstart content, eg. Change the display names of two content types in their .yml files
  • Make these edits in two separate modules, eg. there should be two modules with changes
  • visit /admin/config/development/distro
  • Verify that both changes are listed
  • Uncheck one of the modules
  • Import the distribution update
  • Verify that only the CHECKED update was applied, eg. if you changed two content type names, only the checked one should be changed in your active site.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I've added this Pull Request to the AZ Quickstart Github project and set it to "Review in progress".
  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@tadean tadean added the bug Something isn't working label Apr 9, 2021
@tadean tadean requested a review from a team as a code owner April 9, 2021 16:24
@tadean tadean self-assigned this Apr 9, 2021
@camikazegreen
Copy link
Contributor

camikazegreen commented Apr 9, 2021

Here's the diff of the actual patch for convenience in review:

diff --git a/src/Form/ConfigDistroImportForm.php b/src/Form/ConfigDistroImportForm.php
index aff2f08..9410401 100644
--- a/src/Form/ConfigDistroImportForm.php
+++ b/src/Form/ConfigDistroImportForm.php
@@ -41,11 +41,11 @@ class ConfigDistroImportForm extends ConfigSync {
*/
public function buildForm(array $form, FormStateInterface $form_state) {
$form = parent::buildForm($form, $form_state);

  • // @todo: why is this empty?
  • $storage_comparer = $form_state->get('storage_comparer');
  • if (!isset($storage_comparer)) {
  •  $storage_comparer = new StorageComparer($this->syncStorage, $this->activeStorage, $this->configManager);
    
  • }
  • $storage_comparer = new StorageComparer($this->syncStorage, $this->activeStorage);
  • // Store the comparer for use in the submit.
  • $form_state->set('storage_comparer', $storage_comparer);
  • foreach ($storage_comparer->getAllCollectionNames() as $collection) {
    if (isset($form[$collection])) {
    foreach (array_keys($form[$collection]) as $config_change_type) {

Do we have any precedent for including patches via personal gist?

@mmunro-ltrr mmunro-ltrr self-requested a review April 9, 2021 17:17
Copy link
Member

@mmunro-ltrr mmunro-ltrr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Followed along with a local Lando installation, checked for errors in the Probo build logs.

@joeparsons joeparsons merged commit 3fa1a1d into main Apr 9, 2021
@joeparsons joeparsons deleted the bug/428 branch April 9, 2021 17:26
@joeparsons joeparsons added the backport Changes to be back-ported to previous minor release branch label Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Changes to be back-ported to previous minor release branch bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants