Problem/Motivation

hook_install() cannot be used for content imports in modules because the module cannot rely on default config to be there if they are installed during config import.

Proposed resolution

Add a fallback config import event.

Remaining tasks

Finalise the approach.
Create a patch.

User interface changes

None

API changes

None

Data model changes

None

CommentFileSizeAuthor
#19 2914213-19.patch11.27 KBjibran
#19 interdiff-b69470.txt9.54 KBjibran
#19 conflict-88af9f.txt2.28 KBjibran
#3 don_t_create_content_in-2914213-3.patch5.59 KBjibran
#3 interdiff-bb7148.txt450 bytesjibran
#2 don_t_create_content_in-2914213-2.patch5.59 KBjibran
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jibran created an issue. See original summary.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Status: Active » Needs review
FileSize
don_t_create_content_in-2914213-2.patch5.59 KB 
+    $modules = $event->getConfigImporter()->getExtensionChangelist('module', 'install');
+    if (in_array('standard', $modules, TRUE)) {

We can't do this until #2788777: Allow a site-specific profile to be installed from existing config is in.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
FileSize
interdiff-bb7148.txt450 bytes
don_t_create_content_in-2914213-3.patch5.59 KB

Fixed c/p issue.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Title: Don't create content in install hooks » Don't create content in install hooks if module is installed during config import
Alumei’s picture

Alumei CreditAttribution: Alumei commented
Status: Needs review » Active

But isn't the problem either way, that the module has no assurance for the expected config to be present.
If the config that the module relies on in the subscriber is not present after config import, because It was deleted before the export, this would still fail, right?

Alumei’s picture

Alumei CreditAttribution: Alumei commented
Status: Active » Needs review

sry, was not my intention

Alumei’s picture

Alumei CreditAttribution: Alumei commented

How is this different from #2901418: Add hook_post_config_import_NAME after config import? To me it looks like both issues provide similar solutions.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

If the config that the module relies on in the subscriber is not present after config import, because It was deleted before the export, this would still fail, right?

We can add the check here whether the config is present or not.

How is this different from #2901418: Add hook_post_config_import_NAME after config import? To me it looks like both issues provide similar solutions.

This one will be fired after the module is installed and that one will tackle the situation after hook_update_N has run.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and commented

There's yet another problem. We have no idea whether the content is valid. It is possible to have added a required field that during config import will be created before the content is created :(

gambry’s picture

gambry
Primary language Italian
Location Milan
CreditAttribution: gambry at manifesto commented

Wasn't this issue meant to be a documentation task, according to #2788777: Allow a site-specific profile to be installed from existing config #114?

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
CreditAttribution: alexpott as a volunteer and commented

@gambry well we can do both. But I'm not sure this solution is correct and I know that the extra documentation is warranted.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

We have no idea whether the content is valid. It is possible to have added a required field that during config import will be created before the content is created

Any suggestions for moving forward?

gambry’s picture

gambry
Primary language Italian
Location Milan
CreditAttribution: gambry at manifesto commented

This is just me talking aloud:

We have no idea whether the content is valid. It is possible to have added a required field that during config import will be created before the content is created

I'm not sure there is a simple way to go around this issue.

When a module with default config is installed NOT as part of config import, then implementing either #3 or #2901418: Add hook_post_config_import_NAME after config import fixes what this issue is about.
But as soon as its default config becomes part of the sync storage and the module is installed as part of the config import... everything can happen! And #3 or #2901418: Add hook_post_config_import_NAME after config import implementations may have invalid content.

And there is no way for the module shipping the original config to know what's going on. It may be a new required field, it may also be a pre-save function blocking the CRUD process under certain conditions, failing spectacularly.

Suggestion:
Why don't we make a clear separation between these two scenarios and we create a custom hook to run only after default configuration is imported during standard install process?
Then #2901418: Add hook_post_config_import_NAME after config import will cover most of the other scenarios.
A module ABC "extending" default configuration of module DEF may require to re-implement the logic of its DEF_default_config_imported() hook (or call it directly) within ABC_post_config_import_NAME() if - for example - they require that content to exists.

My only doubt is if the scenario when ABC and DEF modules are installed through config_installer profile behaves for them like the standard install or the config import process.

Alumei’s picture

Alumei CreditAttribution: Alumei commented

I think this sound reasonable,

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

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now 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.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now 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.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
Related issues: +#2901418: Add hook_post_config_import_NAME after config import

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

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented
FileSize
conflict-88af9f.txt2.28 KB
interdiff-b69470.txt9.54 KB
2914213-19.patch11.27 KB

Here is a reroll of #3 but it also added content registry service to make sure we keep track of created content. Thoughts?

Status: Needs review » Needs work

The last submitted patch, 19: 2914213-19.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

jibran’s picture

jibran
he/him
Primary language Urdu
Location Sydney, Australia
CreditAttribution: jibran as a volunteer commented

TBH, I have mixed feeling about this approach. We made a deliberate effort to remove all the uuid form the config this is going against that but then these are content uuids not config.

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

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). 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.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now 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.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
CreditAttribution: larowlan at PreviousNext commented
Category: Bug report » Feature request
Status: Needs work » Postponed (maintainer needs more info)
Issue tags: +Bug Smash Initiative

On the basis of the proposed resolution this sounds like a feature request

Now drush supports deploy hooks is this something we want to continue with?

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 10.1.x-dev » 11.x-dev

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

smustgrave’s picture

smustgrave CreditAttribution: smustgrave at Mobomo commented
Status: Postponed (maintainer needs more info) » Closed (outdated)

If still a valid request please reopen updating issue summary for D10 and up

Thanks everyone that worked on it!