Problem/Motivation

Follow-up to #2080823: Create API to discover config entities' soft dependencies and use this to present a confirm form on module uninstall

There's 2 @todo left in code

$ git grep 2080823
core/modules/field/field.purge.inc:91:    // @todo Revisit after https://www.drupal.org/node/2080823.
core/modules/field/field.purge.inc:119:    // @todo Revisit after https://www.drupal.org/node/2080823.

Steps to reproduce

Proposed resolution

Remove the @todo block of code

Remaining tasks

Review
Commit

User interface changes

N/A

API changes

N/A

Data model changes

N/A

Release notes snippet

N/A

CommentFileSizeAuthor
#20 2343517-20.patch1.62 KBquietone
#13 2343517-10-reroll.patch1.57 KBjoelpittet
#10 2343517-10.patch1.58 KBjoelpittet
#4 2343517-4.patch2.3 KBrteijeiro

Comments

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
xjm’s picture

xjm
she/her
Primary language English
commented
Issue tags: -beta blocker +beta target
xjm’s picture

xjm
she/her
Primary language English
commented
Title: Cleanup @todo after 2080823 » Cleanup @todo referring to the config dependencies API issue
Issue tags: -VDC
rteijeiro’s picture

rteijeiro commented
Status: Active » Needs review
StatusFileSize
new 2343517-4.patch2.3 KB

Not sure if this is what it's expected. Let me know if not.

Status: Needs review » Needs work

The last submitted patch, 4: 2343517-4.patch, failed testing.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Issue summary: View changes
Issue tags: -beta target

Version: 8.0.x-dev » 8.1.x-dev

Drupal 8.0.6 was released on April 6 and is the final bugfix release for the Drupal 8.0.x series. Drupal 8.0.x will not receive any further development aside from security fixes. Drupal 8.1.0-rc1 is now available and sites should prepare to update to 8.1.0.

Bug reports should be targeted against the 8.1.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

mile23’s picture

mile23
Primary language English
Location Seattle, WA
commented
Version: 8.1.x-dev » 8.2.x-dev
Priority: Normal » Major
Related issues: +#2782009: Create a field with the same name as one being purged results in data destruction of the new field

Moving to 8.2.x and Major because it's almost a duplicate of #2782009: Create a field with the same name as one being purged results in data destruction of the new field which is critical.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.6 was released on February 1, 2017 and is the final full bugfix release for the Drupal 8.2.x series. Drupal 8.2.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.3.0 on April 5, 2017. (Drupal 8.3.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.3.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.4.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new 2343517-10.patch1.58 KB

Not sure if this is major, but here's a re-roll of #4 which only affects
core/modules/field/field.purge.inc

Status: Needs review » Needs work

The last submitted patch, 10: 2343517-10.patch, failed testing.

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented

Interesting failure:

fail: [Fatal error] Line 0 of :
[23-Feb-2017 08:09:33 Australia/Sydney] Uncaught PHP Exception Drupal\Component\Plugin\Exception\PluginNotFoundException: "The "block_content" entity type does not exist." at /var/www/html/core/lib/Drupal/Core/Entity/EntityTypeManager.php line 133

joelpittet’s picture

joelpittet
he/him
Primary language English
Location Vancouver
commented
Status: Needs work » Needs review
StatusFileSize
new 2343517-10-reroll.patch1.57 KB

Reroll

Status: Needs review » Needs work

The last submitted patch, 13: 2343517-10-reroll.patch, failed testing.

Version: 8.3.x-dev » 8.4.x-dev

Drupal 8.3.6 was released on August 2, 2017 and is the final full bugfix release for the Drupal 8.3.x series. Drupal 8.3.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.4.0 on October 4, 2017. (Drupal 8.4.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.4.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.5.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.4.x-dev » 8.5.x-dev

Drupal 8.4.4 was released on January 3, 2018 and is the final full bugfix release for the Drupal 8.4.x series. Drupal 8.4.x will not receive any further development aside from critical and security fixes. Sites should prepare to update to 8.5.0 on March 7, 2018. (Drupal 8.5.0-alpha1 is available for testing.)

Bug reports should be targeted against the 8.5.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.6 was released on August 1, 2018 and is the final bugfix release for the Drupal 8.5.x series. Drupal 8.5.x will not receive any further development aside from security fixes. Sites should prepare to update to 8.6.0 on September 5, 2018. (Drupal 8.6.0-rc1 is available for testing.)

Bug reports should be targeted against the 8.6.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.8.x-dev

Drupal 8.6.x will not receive any further development aside from security fixes. Bug reports should be targeted against the 8.8.x-dev branch from now on, and new development or disruptive changes should be targeted against the 8.9.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.7 was released on June 3, 2020 and is the final full bugfix release for the Drupal 8.8.x series. Drupal 8.8.x will not receive any further development aside from security fixes. Sites should prepare to update to Drupal 8.9.0 or Drupal 9.0.0 for ongoing support.

Bug reports should be targeted against the 8.9.x-dev branch from now on, and new development or disruptive changes should be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

quietone’s picture

quietone commented
Version: 8.9.x-dev » 9.4.x-dev
Status: Needs work » Needs review
Issue tags: +Bug Smash Initiative
StatusFileSize
new 2343517-20.patch1.62 KB

catch mentioned that this needed a reroll in #bugsmash.

So, here is the reroll. It is small so there is no interdiff.

dww’s picture

dww
we/he/they
commented
Issue tags: +Needs issue summary update

https://www.drupal.org/pift-ci-job/2222363 has some random fails. Requeued.

Is this really major?

I’m not sure why removing this code is so important. Can someone give this a real summary?

Thanks!
-Derek

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Priority: Major » Normal

Agree this is not major, downgrading - IMO this could even be either a minor bug or a normal task.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Category: Bug report » Task

I agree, this is a task not a bug

quietone’s picture

quietone commented
Issue summary: View changes

Updated the IS. Anything else needed?

And starting tests again.

quietone’s picture

quietone commented

Due to feedback in #bugsmash from darvanen I did some research. I didn't learn much. The code being removed is not tested by any test I could find. I did not find any supporting information in the related issues. Is it possible to have a field without an entity type?

It is only the issue summary that says to remove this.

@andypost, can you comment?

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Needs review » Reviewed & tested by the community

This was originally added in #2081609: field purge should bail out on unknown entity types seemingly as protection against purging a field that was in some way related to a disabled module; previously I guess it would crash here if something that provided the entity type no longer existed as it was disabled. Now we no longer have the concept of disabled modules and you cannot uninstall a module without removing all of its content and fields first, I think this is safe to remove again.

dww’s picture

dww
we/he/they
commented

Yeah, patch looks good. #26 sounds persuasive / reasonable. Happy to see this code and the comments go.

Thanks!
-Derek

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 20: 2343517-20.patch, failed testing. View results

spokje’s picture

spokje
Primary language Dutch
commented
Status: Needs work » Reviewed & tested by the community

Back to RTBC per #26 after #3255836: Test fails due to Composer 2.2 solved the unrelated test failure.

  • catch committed c26a85e on 10.0.x 
    Issue #2343517 by joelpittet, quietone, rteijeiro, andypost: Cleanup @...
  • catch committed ca21c4b on 9.4.x
    Issue #2343517 by joelpittet, quietone, rteijeiro, andypost: Cleanup @...
catch’s picture

catch
he/him
Primary language English
commented
Status: Reviewed & tested by the community » Fixed

Committed/pushed to 10.0.x and cherry-picked to 9.4.x, thanks!

Status: Fixed » Closed (fixed)

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