Problem/Motivation

After #3439769: Remove all the pre-10.3.0 updates hooks we will have a lot of hook_ENTITY_TYPE_presave() implementations and ViewsConfigUpdater methods which were added to both support update paths, and to 'update' config being imported from modules or distributions.

We should remove all of those in 11.x, and enforce that modules have to re-export any shipped config in 10.3.x in order to be compatible with 11.x

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Release notes snippet

Issue fork drupal-3442162

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

catch created an issue. See original summary.

andypost’s picture

andypost
he/him
Primary language Russian
commented
Parent issue: » #3416928: [Meta] Remove updates added up until 10.3.0 from Drupal 11
catch’s picture

catch
he/him
Primary language English
commented
Title: Remove redudundat hook_ENTITY_TYPE_presave() and ViewsConfigUpdater methods » Remove redundant hook_ENTITY_TYPE_presave() and ViewsConfigUpdater methods

pradhumanjain2311 made their first commit to this issue’s fork.

catch opened merge request !7640

catch’s picture

catch
he/him
Primary language English
commented
Status: Active » Needs review
catch’s picture

catch
he/him
Primary language English
commented

This is choking on the ckeditor4 config in ckeditor5 module, we might need to split that out to its own issue since it looks non-trivial to untangle the test coverage.

catch’s picture

catch
he/him
Primary language English
commented

ckeditor5 and editor are going to be more complicated, so I opened #3442395: ckeditor5 and editor module test config exports/stubs rely on hook_editor_presave() bc layers.

What this is showing, is that we need to be stricter about requiring deprecation triggering in hook_entity_presave() implementations when they're not running in updates, then all these tests would have failed when the config validation was added (i.e. before commit, requiring the config to be fixed first) instead of now.

smustgrave’s picture

smustgrave commented
Status: Needs review » Needs work

Appears to have some functional test failures.

catch’s picture

catch
he/him
Primary language English
commented
Status: Needs work » Needs review

We have a general problem that issues adding config entity update paths haven't always remembered to update shipped config, or to issue deprecations for non-upgrade-path-config-fixing in presave. Updated that config but going to need to figure out a way to make this harder to forget for everyone so that we can cleanly remove updates next time.

smustgrave’s picture

smustgrave commented

Should that be done here? Or good to mark?

catch’s picture

catch
he/him
Primary language English
commented

#2913850: Trigger E_USER_DEPRECATED from config entity presave bc layers is probably the place to work on that.

smustgrave’s picture

smustgrave commented
Status: Needs review » Reviewed & tested by the community

Sweet, looking at the MR and doing a search for hook_ENTITY_TYPE_presave(), the instances that are remaining in
ckeditor5, comment, user, and some tests seem fine as they don't appear to be doing $config_updater stuff.

So I believe this is good to go.

quietone’s picture

quietone commented
Parent issue: #3416928: [Meta] Remove updates added up until 10.3.0 from Drupal 11 » #3439769: Remove all the pre-10.3.0 updates hooks
Related issues: -#3439769: Remove all the pre-10.3.0 updates hooks +#3416928: [Meta] Remove updates added up until 10.3.0 from Drupal 11
smustgrave’s picture

smustgrave commented
Related issues: +#3295574: [meta] Remove deprecated classes, methods, procedural functions and code paths outside of deprecated modules on the Drupal 11 branch

  • quietone committed fecffbff on 11.x 
    Issue #3442162 by catch, smustgrave: Remove redundant...
quietone’s picture

quietone commented
Status: Reviewed & tested by the community » Fixed

Removing no longer needed presave and ViewsConfigUpdater code is new to me. Therefor, I carefully reviewed these changes checking each one was correct and any related post_update hooks were removed etc. I didn't find any other fixtures that needed to be removed either.

Committed fecffbf and pushed to 11.x. Thanks!

Status: Fixed » Closed (fixed)

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

quietone closed merge request !7640