field.storage YAML and FieldType plugin cannot coexist in same module because of FieldUninstallValidator constraint

Good catch!

> Any field storage definition that needs to be deleted should include the module name (example_module) that defines the FieldType plugin an enforced module dependency with the following YAML code:

I don't think that's a good API change. Automatic config dependency calculation is very useful, and we shouldn't be removing part of it.

The Commerce Recurring module is affected by this bug (See #2924965: [PP-1] Module does not uninstall cleanly (dependency issue)). It cannot be uninstalled because of this bug...

Any news on this?

Would be curious as well where we are at with this?

Unassigning the issue given that Plankje is clearly not working on that anymore.
This may lead it to be picked up by other contributor.

Update please.

Any news about this issue?

Trying to reroll for last versions.

If the patches are rerolled they need to reviewed or am I wrong? So changing issue status.

@dercheffe
you're absolutely right.
Thank you!

Any news/updates about this?

I'm willing to pay USD$25 to get this bug squashed and committed to core. Any takers?

1. +++ b/core/modules/field/src/FieldUninstallValidator.php
   @@ -53,11 +53,21 @@ public function validate($module) {
   +      $hasDeleted = FALSE;
   

   The variable name should be in snake_case.

2. +++ b/core/modules/field/src/FieldUninstallValidator.php
   @@ -53,11 +53,21 @@ public function validate($module) {
   +        if ($field_storage->isDeleted()) {
   +          $hasDeleted = TRUE;
   +          continue;
            }
   

   This logic needs a comment to explain what's being done.

I have tested the patch #18 and it fixed this issue.

I have also made the changes you suggested and attached a new patch. Please review.

Looks like xamount has uploaded the interdiff as patch.

ahh my bad, good catch! sorry about that.

Hi, has anyone successfully uninstalled "Commerce Recurring Framework" ?

@zenimagine Yes I have. Here are the steps:

Step 1: Apply the patch from this issue queue

Step 2: In the commerce_recurring module (8.x) inside of config/install you have to update ALL the field.storage... yml files to the API changes suggested at the top of this page.

For e.g. in commerce_recurring/config/install/field.storage.commerce_order.billing_period.yml

before:

langcode: en
status: true
dependencies:
  module:
    - commerce_order
    - commerce_recurring
id: commerce_order.billing_period
field_name: billing_period
entity_type: commerce_order
type: commerce_billing_period
settings: {  }
module: commerce_recurring
locked: true
cardinality: 1
translatable: false
indexes: {  }
persist_with_no_fields: false
custom_storage: false

after:

langcode: en
status: true
dependencies:
  module:
    - commerce_order
    - commerce_recurring
  enforced:
      module:
        - commerce_order
        - commerce_recurring
id: commerce_order.billing_period
field_name: billing_period
entity_type: commerce_order
type: commerce_billing_period
settings: {  }
module: commerce_recurring
locked: true
cardinality: 1
translatable: false
indexes: {  }
persist_with_no_fields: false
custom_storage: false

Step 3: After you have updated all the field.storage yml files inside of config/install then you need to reimport the config with a command such as drush cim -y --partial --source=modules/contrib/commerce_recurring/config/install;

Step 4: clear cache

Step 5: Then try to uninstall commerce_recurring

I confirm that this has worked for me. If someone can review the latest patch in this issue queue and we can get it committed into core, I will gladly write the patch for #2924965: [PP-1] Module does not uninstall cleanly (dependency issue)

Thanks for the explanations, I will wait for a patch because it seems very complicated. It's been 2 years since I made the mistake of installing this module on my production site and could not uninstall it. I can't wait to get rid of it.

I am going to set the status to RTBC. Feel free to change it back to "Needs Review".

Needs tests or I am afraid that it won't be committed.

See #10.

xamount's advice in #34 looked promising for me after following the instructions carefully. Until, that is, I reached step 4, at which point I got this after issuing the drush cache:rebuild (and also the error "the website encountered an unexpected error. Please try again later." on the site):

In FieldStorageConfigStorage.php line 174:
                                                                                                                 
  Unable to determine class for field type 'commerce_plugin_item:commerce_subscription_type' found in the 'fiel  
  d.storage.commerce_product_variation.subscription_type' configuration                                                                                                                                                           

In DiscoveryTrait.php line 53:
                                                                                                                 
  The "commerce_plugin_item:commerce_subscription_type" plugin does not exist. Valid plugin IDs for Drupal\Core  
  \Field\FieldTypePluginManager are: address_zone, address_country, address, comment, commerce_remote_id, comme  
  rce_plugin_item:commerce_condition, commerce_plugin_item:commerce_promotion_offer, commerce_plugin_item:comme  
  rce_license_type, commerce_plugin_item:recurring_period, commerce_adjustment, commerce_price, datetime, entit  
  y_reference_revisions, feeds_item, file, file_uri, fontawesome_icon, image, interval, layout_section, link, m  
  etatag, list_string, list_float, list_integer, path, redirect_source, state, text_long, text_with_summary, te  
  xt, viewsreference, webform, xfloat, xinteger, xdecimal, xquantity_stock, email, uuid, string_long, changed,   
  password, float, timestamp, language, integer, boolean, decimal, uri, map, string, entity_reference, created   

So I guess it's not possible, unless I did something wrong (or have a peculiar setup).

Adding test cases with simple entity and field plugin.
For the testing i've included configuration which has "enforced" key in it.

enforced:
    module:
      - entity_uninstall_validator

Fixing test case failures related to the field storage configuration yml file.
I've updated test-only file as well.

Bump:

Given this affects commerce_recurring directly, and there is a patch is there any scope for a resolution to this soon?

This was a Bug Smash random triage issue. I was wondering if this was works as designed but darvanen disagreed. Updated the remaining tasks to indicate this Needs review.

The Needs Review Queue Bot tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

Consult the Drupal Contributor Guide to find step-by-step guides for working with issues.

@rivimey #46 following up on this as it relates to commerce_recurring.

Has anyone used the patch in #41 successfully in this use case?

Even after the patch in #41 Commerce Recurring fails upon uninstall for me:

commerce_recurring: The <em class="placeholder">Billing period</em> field type is used in the following fields: commerce_order.billing_period, commerce_order_item.billing_period

I haven't had much luck uninstalling Commerce Core either.