This is complete!
For the record, we made a detailed checklist on a GitLab issue, pasting the contents here to show the steps we went through.
- [x] Push 2.0.x branch from drupal.org config_enforce to this repo
- [x] Push 2.0.x branch from drupal.org config_enforce_devel to https://gitlab.com/consensus.enterprises/drupal/config_enforce_devel
- [x] Review (possibly merge) MRs on config_enforce_devel on drupal.org
- [x] Close https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/15 with a comment, the changes are small and trivial to reimplement, and cannot be merged as-is
- [x] Review https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/23
- [x] Consider merging https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/24 into 2.0.x
- [x] Close https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/25 and the associated issue, we will create target modules in the SimpleTest site instead
- [x] Review https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/26 - possibly it's merging the previous two into one MR, but doesn't seem to apply, possibly can be closed
- [x] Close https://git.drupalcode.org/project/config_enforce_devel/-/merge_requests/27 and the associated issue, or move to CE and repurpose to use the SimpleTest sites/ directory approach instead
- [x] Subtree ~~merge~~ *add* CED into CE/.config_enforce_devel
- [x] Add `monorepo` branches to both projects
- [x] ~~Prepare CED by moving the codebase into the desired subdirectory:~~ This is no longer relevant, since we're using a `git subtree add`/`git subtree push` approach.
```
git clone git@gitlab.com:consensus.enterprises/drupal/config_enforce_devel.git --branch monorepo
cd config_enforce_devel
git filter-repo --to-subdirectory-filter .config_enforce_devel --refs monorepo
```
- [x] ~~Do the subtree merge:~~ Replaced by the `git subtree add` below
```
cd ..
git clone git@gitlab.com:consensus.enterprises/drupal/config_enforce.git --branch=monorepo
cd config_enforce
git remote add ced ../config_enforce_devel
git fetch ced --no-tags
git merge --allow-unrelated-histories ced/monorepo
git remote remove ced
```
- [x] Do the subtree add:
```
git subtree add --prefix=.config_enforce_devel/ git@gitlab.com:consensus.enterprises/drupal/config_enforce_devel.git 2.0.x
```
- [x] Test that a `git subtree push` will still look reasonable of the 2.0.x branch of CED
- [x] Check that we can push to drupal.org repo for config_enforce (because we've re-written the history)
- [x] Move .local-dev and .ddev to top level
- [x] Update .gitattributes at top level to include
- [x] Update CE README to explain the presence of CED in a subdir
- [x] Review to see if there are any other changes/removals that make sense at this point
- [x] Move CED docs to project root
- [x] Sort out publishing of docs site in CI (https://www.drupal.org/project/config_enforce/issues/3559151)
- [x] Exclude Drumkit components via `.gitattributes`
- [x] Update .local-dev and .ddev to function with the new structure
- [x] Check that the path to the CED module is not nested within CE, but its own separate path (that we have configured as a composer path repo)
- [x] Test/come up with the release process manually in our sandbox environment
- [x] Push changes in CE to CED (from within CE): `git subtree push --prefix=.config_enforce_devel/ git@gitlab.com:consensus.enterprises/drupal/config_enforce_devel.git 2.0.x`
- [x] Push tags to both projects (CE & CED) - Note that there doesn't appear to be a way to push tags directly to CED via `git subtree push`
- [x] Cut releases so we can examine the resulting tarballs
- [x] Add Drumkit targets to automate the release process:
- [x] Pass in or prompt for a tag name
- [x] Prompt could display the latest tag for reference (because you most often want to do something like alpha-n+1, or later a.b.n+1 or similar)
- [x] Subtree push to config_enforce_devel as part of the release process
- [x] Prompt/confirm before pushing to git.drupalcode.org
- [x] Test that the security promises that we have made are still being kept when releasing on git.drupalcode.org (specifically, no CED packaged with CE, and no .local-dev or .ddev either)
We are planning to remove the method that ✨ Make it possible to set default target module machine name and path before creating Needs review updates, and aren't adopting 📌 Add test trait to create randomly named, non-conflicting default target modules Needs review , so I'm going to close this as outdated.
The larger goal is ✨ Prompt the user to create a new target module if none are found Active from a UI/UX perspective.
We are also working on making our tests more robust in terms of not making unnecessary changes to the filesystem (leaving git in a dirty state, specifically).
In ✨ Allow configs to be enforced by default Active , we took a different approach of installing the temporary target module into the SimpleTest site directory, so that it automatically gets cleaned up on teardown.
I'm going to close this, and we can always revisit if need be. Thanks!
Closing, the core bug is fixed, so we can refactor TargetModuleInstaller in 📌 Refactor TargetModuleInstaller Active .
Thanks!
I'm not sure when this changed, but at least on 2.0.x I can't reproduce this bug.
Looking at the code, I think the relevant method here is Drupal\config_enforce_devel\EnforcedConfigCollection::deleteEnforcedConfigs(), which doesn't seem to touch the actual config, just the enforcement settings as I would expect.
I'm going to mark this as fixed.
This seems to work as expected on 2.0.x, both with the enforce by default (pressing Save elements on the form), and via the off-canvas/separate config enforce settings form ( ✨ Improve usability of Config Enforce Devel's UI Postponed ).
Looking at Drupal\restui\Form\RestUIForm at a glance it seems like we would have to bend over backwards to incorporate this into our ConfigResolver. Ideally this is something that could be resolved upstream with an update to RestUIForm::getEditableConfigNames().
Demoting since there is a workaround as @spiderman pointed out. I also am switching this to be a feature request, there is a lot of contrib out there and a near-endless supply of bugs if we treat all config forms that don't expose their config as bugs.
Drupal\auto_entitylabel\Form\AutoEntityLabelForm::getEditableConfigNames() only returns 'auto_entitylabel.settings'. The same class has a getConfigName() method (also protected) that returns the config name with the entity type and bundle.
It seems to me that the ideal here would be to fix this upstream by updating the getEditableConfigNames method, and having that add a call to getConfigName().
Re-tested on 2.0.x and the behaviour hasn't changed.
As for resolving this, in the context of ConfigResolver, checking get_class_methods($this->formObject) gives us somewhat promising leads:
getEntityType(). This returns a string, the machine name of the entity type. We should be able to check if the entity type is a config entity.
getMachineName(). This returns a string, I don't expect this to return a value when creating a new page/page variant, but should be able to play nicely with enforce by default's submit handler and get a config enforce indicator when editing an existing config entity.
initValues() This returns an array, including a 'page' key that contains the Page config entity. From there I think we could call getVariants(), or getConfigDependencyName() to get the config name of the page itself.
Overall I do see a path forward, just not a super solid one. Assuming the APIs here can be trusted to be stable, then initValues() seems to give us what we want, and we should be able to carefully incorporate changes into our ConfigResolver to allow getting config names of page manager's page and page variant configs.
I'm going to file this as a feature request, since there is a workaround of being able to use "Generate from active storage".
I tested this on 2.0.x and can confirm the same behaviour as originally reported.
The issue is different from
✨
Drupal 10.2+ config forms that use the new #config_target are not detected by Config Enforce
Active
, but I can see that getEditableConfigNames() is not implemented anywhere. This seems to be a special ctools-ish thing.
I'm happy to report this is fixed in 2.0.x, I cannot reproduce. I believe this was fixed by ✨ Improve usability of Config Enforce Devel's UI Postponed .
Both errors are fixed in 2.0.x because of ✨ Improve usability of Config Enforce Devel's UI Postponed .
While testing I found separate issues with each module:
The xmlsitemap settings for individual entity/bundle settings (for example /admin/config/search/xmlsitemap/settings/node/page) only show the global xmlsitemap.settings config, but that is a separate issue, and seems to be because xmlsitemap's XmlSitemapLinkBundleSettingsForm only returns xmlsitemap.settings. I don't think we should special-case every contrib module, so that would be one to use the "Generate from active storage" feature.
Pathauto doesn't seem to have this issue, but does have a separate bug which I've made an issue for: 🐛 Enforce by default submit handler conflicts with pathauto new pattern form Active
This is fixed in 2.0.x, I re-tested and the only quirk is that the config files are only written after you save from the field storage settings form - but I think this is because the config actually doesn't exist until you press save.
I'm going to mark this as fixed. The specific issues that fixed this are likely some combination of ✨ Improve usability of Config Enforce Devel's UI Postponed and 🐛 Offer to enforce field storage configs Needs review .
This one seems closely related, and could be the solution we need to solve this bug.
The primary use case and reason for existence of this module is to ensure certain configs remain in place (including features like making their associated config form elements disabled), not to provide a way to remove/uninstall config.
Normally I would set "Postponed (maintainer needs more info)" and ask for clarification or more about your use case, but given the lateness of this reply, I'm just going to close this as outdated. If you can provide more clarification we may be able to help, but I do suspect this is not the module you want for your particular use case.
In certain environments we may need to reckon with file permissions as well, linking this related issue.
For what it's worth, this use case (migration YAML files) is why I took to just adding those files into the migrations/ directory as plugins, rather than going through the config system. If I recall correctly, this approach does lose some of the Migrate Plus fanciness where you can extend other migrations and such, but I prefer the overall simplicity, and not having to wrestle with the config system potentially mangling these files.
I bring this up, because I'm not sure what the use cases for comments in config files are, aside from migrate.
See also https://www.drupal.org/docs/drupal-apis/migrate-api/migrate-api-overview... → (Migration as configuration, Migration as plugins).
Wondering if we could solve this in a similar way as 🐛 Offer to enforce field storage configs Needs review .
Thank you for the efforts here folks. No further work is planned for the 1.0.x series other than security fixes and some deprecations and such.
2.0.x is under active development and we plan to fix the phpcs issues there, but this will be taken on by the module maintainers so that it can be done efficiently without a lot of rework needed, and without disrupting the work that the module maintainers are doing across many fronts.
Forgot that I had already created ✨ Prompt the user to create a new target module if none are found Active last week, so re-purposing this to be its follow-up instead, and focus on the enforcing CED settings part.
Moving to Config Enforce Devel, Config Enforce only contains the read-only version.
Here is the follow-up issue: 📌 Refactor out FormHelperTrait Needs review
Here is the follow-up issue: 📌 Refactor away from $this->form(), $this->form_state and friends Active
Marked as internal on both branches, closing and will make a follow-up for further work.
Marked as internal on both branches, closing and will make a follow-up for further work.
Merged to 2.0.x, and I am going to mark this trait as internal in 2.0.x and 1.0.x rather than deprecating individual methods.
Committed to 2.0.x, moving to 1.0.x. I think marking these entire traits as @internal on both branches might be an appropriate thing to do rather than individual deprecations.
Updating to reflect another one-off we can move, and I'm going to make these changes now that ✨ Allow configs to be enforced by default Active has been refactored to not touch this class very much.
This is no longer an issue on 2.0.x (we have released a few alphas so far for testing). If you need to work around this issue on 1.0.x and do not want to upgrade, please use the patch.
I went through method-by-method to determine the fate of each, I've added that to the issue summary.
star-szr → changed the visibility of the branch 3556975-clean-uprefactor-develformhelpertrait to hidden.