Attaching a patch of the latest state of the MR.
The issue fork is now updated with all of the tasks outlined in the issue summary completed. This seems to work nicely in terms of improving usability as well as functionality.
spiderman β created an issue.
@avpaderno: Thanks for taking a look at this- perhaps I was unclear, but the issue was not posts missing that I was expecting to be there, but that one (Dec 11) post not being formatted properly. That being said, I had watched the feed for an update for a couple hours on Wednesday, hoping that a subsequent refresh of the feed would fix the formatting (having reverted the change that triggered it on my end), but it didn't seem to be happening. At any rate, the drupal.org/planet view of that post now looks correct again. I'm guessing it had correct itself by the time you looked at it, and thus my original description probably didn't make much sense!
Thanks again, I'll close this ticket now as there's nothing further to do :)
spiderman β created an issue.
@ressa: Thanks for bumping this! Unfortunately, the Aegir project always seems to get short shrift since it doesn't pay the bills, but I plan to give it some attention again in the coming weeks, and will focus on getting some blog content up so we can add this feed to Planet.
Committed.
Committed.
Fixed.
I agree we can do better than accidentally clobbering config/optional changes on disk, but I think `git reset` is a reasonable workaround for now. I'm committing this patch as-is, we can take up extra parameters and such in the followup.
I've committed and merged the fix on π Cannot access Block, Field UI, Views UI config pages when Config Enforce Devel is enabled Fixed , so closing this one as also fixed.
I've merged the associated MR here, and will cut a new release shortly. Thanks for the fix @Ambient.Impact, and the validation the patch helps, @Blanca.Esqueda!
I wasn't able to reproduce the problem here, but it looks like the code change on the MR here just brings things in line with the adjustment we've made in https://www.drupal.org/project/config_enforce_devel/issues/3444231 π Cannot access Block, Field UI, Views UI config pages when Config Enforce Devel is enabled Fixed
spiderman β made their first commit to this issueβs fork.
@maxilein: Absolutely, I'll expand on the project page! Thanks for asking good questions to help elaborate what's important to mention :)
@maxilien: I did consider field_token_value as an option, but I was concerned about 2 things. First, we needed to have the "token field" act like a field of the appropriate type (eg. Date, Entityreference) rather than just a text field. Second, I felt that the existing infrastructure in Computed Field was better equipped to handle caching/performance, and ensure the duplicated data (we were effectively "caching" field data from a sub-sub-entity in the node entity it was attached to) stayed in sync. The second one is more rough based on feel looking at the 2 codebases, but the different field types piece was already in Computed Field, so I based this on that. Does that clarify?
@maxilein: the field configuration page for each type has a single "Token value" textfield which accepts a Token (core module). When the Computed Token Field's value is "computed", this module's implementation of the computation essentially *renders* that Token (in the context of building the Node, or whatever entity the field is on), and sets the value of the Text/Date/Reference field with the result.
If you're asking about handling multiple values, see β¨ Handle multi-value fields correctly. Postponed , which is intended to handle the situation where our source (and therefore the computed token field that uses it) produces many values (eg. taxonomy terms).
I hope that helps- this module is very "hot off the press" if you will, so I'm happy to encounter other uses, and see what it can (and can't) do. If it seems close to your use-case, I'm happy to accept suggestions, questions, and patches/MRs to make it more useful :)
spiderman β created an issue.
spiderman β made their first commit to this issueβs fork.
spiderman β created an issue.
spiderman β created an issue.
spiderman β created an issue.
I seem to be running across this in a client project as well, where importing a vocabulary that has multiple "Other" terms across a handful of top-level topics. The import taxonomies process (we usually run via drush) seems to all work, but just hangs at the end. We also import a small handful of simple menu links into the "main" menu via Structure Sync.
Applying this patch appears to resolve the hanging at the end of import, and I *don't* see the errors @mparker17 notes from the DrupalCI. This would seem to indicate there's something distinct about the menu fixtures created in the test scenario, but I'm not sure what that would be.
@apaderno Thanks for the quick update- I may have jumped the gun, was just surprised to find this old blog we're re-upping wasn't already on Planet (perhaps it got cleaned up at some point). Anyway, we expect to have more content to come soon, so I'll update here when there's more recent content. Should I move the status to Needs Review at that point?
spiderman β created an issue.
FYI, we've been using the patch/MR here along with the one in β¨ Add option to set theme used for entity_print rendering Needs work , and found managing the two patches at once to be cumbersome. As such, I've created a custom fork that incorporates both, here: https://gitlab.com/consensus.enterprises/drupal/entity_print/-/commits/d...
FYI, we've been using the patch/MR here along with the one in β¨ Support multiple view modes for a given library Needs review , and found managing the two patches at once to be cumbersome. As such, I've created a custom fork that incorporates both, here: https://gitlab.com/consensus.enterprises/drupal/entity_print/-/commits/d...
Having used variations on this patch for some while, the newest project where I'm using migrate_tools doesn't seem to have the bug as described above. We have not installed this patch, so I'm assuming the problem got resolved elsewhere. As such, I'm going to mark this as closed. For reference, we're running migrate_tools 6.0.1 on a Drupal 9 install here :)
Here's a static patch to include in composer.json
@jurgenhaas I see the top-level composer.json in the ECA project declares the drush.services section, but it looks like the sub-modules need to also re-declare this. We're seeing it in the context of enabling modules like eca_base
etc. via drush. We're using Drush 11.5 on a Drupal 9.5.8 site with ECA version 1.1.3.
I have to concur, this looks like it may have ended up being more trouble than just doing the form separation work, but I am fully aware how much easier it can be to see that in hindsight. That said, now that it's done and passing tests, I see no reason *not* to merge it, really. I appreciate that it's nicely segmented off, and if we do end up implementing the "proper" fix (separating forms) on the 2.x branch, at least this gives a workaround for folks who (unfathomably) are still using our 1.x branch and trying to enforce theme settings without throwing 500s :)
Looks great to me!
Ouch! That first screenshot hurts my eyes. Please merge this ASAP ;)
Also: thanks for raising this!
spiderman β created an issue.
Ideally we'd add tests for this, but it's a little tricky to reproduce. We might be able to create a test module that does what ECK does, and triggers a cache-rebuild immediately after importing its own config, which should cause the config_enforce_rebuild
to fire a second time. In that case we can check for the log output from the patch, to ensure that we're successfully avoiding the loop.
Alternately, if we implemented an event subscriber to listen for the ConfigEvents::IMPORT
event and that subscriber triggered a cache rebuild, we might be able to observe the behaviour that way.
Attached is a patch that checks for a lock which the ConfigImporter sets when it's in process of importing config, and returns from config_enforce_rebuild()
early if so.
spiderman β created an issue.
@Ambient.Impact: this looks great to me. Ship it!
@Ambient.Impact: Nice improvement, I think this is good to merge. As I mentioned in
β¨
Add support for ConfigEntityListBuilder config forms
Fixed
, let's also leverage this trait to test for the presence of the config enforce container on a variety of config forms, to cover the various cases ConfigResolver
covers. I think this could reasonably go in on this issue/MR if we repurpose it a little bit, but if you prefer to create a new ticket for it, feel free :)
This looks great to me! I think relying on a core module's (esp the venerable block.module
) implementation here is reasonable, since any changes there should likely result in some kind of change in our module as well. We'd either want to support the new implementation and/or adapt the current one. This also saves us some boilerplate in the test code, which is a nice upside :)
I'd love to see a follow-on ticket to add tests for the other types of config ConfigResolver
supports, namely "simple" config, basic Config Entities, and Plugin-based Config Entities. With the new Trait introduced in [#3353064+], along with the example set here, these should be trivial to implement, but increase the value of our test suite quite a bit.
Thanks @Ambient.Impact!
That's great, thanks!
I'm not sure why we're adapting CED to match the refactored patch on config_devel. Can we not make that patch behave as it had been previously? It's been working for us as-is...
If we can show that Multiselect 2.0 still passes tests, then I think that makes sense. Perhaps if we rebase this MR on the newer main branch that removes the D10 compatibility patch which was causing the pipeline to fail, we can see that here?
I've attached a static patch of the MR as it stands right now, for inclusion in composer.json.
I've uploaded a static patch file based on the current state of the MR, for inclusion in composer.json as needed.
I've pushed a patch on the issue fork that implements the getEditableConfigNames method on both classes.
spiderman β created an issue.
spiderman β created an issue.
@raystuart Thanks for the MR! I'm sorry I didn't recognize your patch was the same as what I was doing, or I would have just merged that. In any case, we ended up in the same place. Appreciate your input :)
I was finding a fresh build failing due to the D10 compatibility fix no longer applying, so I've pushed a commit on `1.0.x` to remove it. We may want to roll a new release on this basis, just to avoid composer failures downstream of us.
Thanks @ergonlogic, I believe you're right. I've removed the patch from composer.json in the issue fork, we'll see what DrupalCI says.
spiderman β created an issue.
@joachim Apologies if the issue doesn't make it clear, but our use-case here is for Config Enforce β and Config Enforce Devel β modules, which keep their own registry of what configs to auto-import and auto-export. Rather than re-implement the functionality provided by Config Devel, we figured adding these hooks would more generically achieve the goal.
I'm happy to answer other questions about this, but fwiw we've been using this patch to allow Config Enforce to mature over the last couple years, and it's working very well for us.
I agree, it's a tricky question. I was imagining a X icon in the "floaty window" itself, when it's present. The harder part seems to be how you bring it back- where does the button go? One option could be that it just disappears, but the "floaty window" reappears the next time you reload the page- this could probably go a long way to improving the UX for a developer. Most of the time, when I find this thing in the way, it's temporary- I need to see the rest of the config page I'm on better, but once those settings are back, I don't mind the CED form returning.
Alternately, maybe we can collapse down to a micro-version of the floaty window (the CE logo?), to minimize its overlap with anything else on the page. Again, we're looking for a quick and dirty stopgap here more than anything, until we can invest the time to refactor the floaty window into its own off-canvas dialog (and separate form).
spiderman β created an issue.