And I should mention that this module receives a ton of block lists through the scenarios. And that works extremely well, by default there is no need to subscribe to additional lists.
This is not a bug. The version is currently implemented as defined by CrowdSec. They wanted us to use the Drupal module's version as the user_agent_version. Changing that to their CAPI version may probably work, but we don't know what the unintended side-effects will be.
Please open a support issue with CrowdSec first to clarify the situation with them. And once we know what they require upstream, we can then make changes for this module.
Changes will be made against the latest version always. And once completed, they can then probably be back ported if needed and possible.
Is the related issue added in #20 probably a duplicate of a fairly old issue at 🐛 Problem with action path for embedded forms Needs work ?
We could still build notifications into the scheduled pipelines. When we run then daily or weekly, and the results are outside acceptable ranges, we could send alerts to whom ever should be notified.
To reduce the noise but still have so the data, we could exclude this tests from MR pipelines and only run them in a schedule. Would that probably solve it?
This is looking good, thanks for your contribution @ipumpkin
What's missing is the update to the config schema, but everything else is OK.
Forward-ported into core in 📌 Merging Gin as Admin theme Active
Forward-ported into core in 📌 Merging Gin as Admin theme Active
Forward-ported into core in 📌 Merging Gin as Admin theme Active
Forward-ported into core in 📌 Merging Gin as Admin theme Active
Forward-ported into core in 📌 Merging Gin as Admin theme Active
Forward-ported into core in 📌 Merging Gin as Admin theme Active
That's a good point regarding React and getting other JS components actually injected and fired up automatically. I don't know the definite answer to that, I can only confirm that I've just written an ECA modeler in React which can get injected as an overlay after the main page request, and it gets loaded by ajax and started by using Drupal behaviours. So that part works, but whether that's in any way related on how Canvas brings things together, I don't know.
Ah, I guess there is a chatbot in the admin interface (that's working with Klaro) and there is another one in Canvas, and that's not working yet. Is that intended, that there are 2 different chatbots?
Anyway, when I apply the change from the MR, the chatbot in Canvas still doesn't ask for consent. So, there must be something else that'S required.
Hmm, that's not it. I've deleted that field again, and re-created it with the same definition as in the entity, but that still shows an error on the status page. I've then compared the current and original field definition and get a strange difference:
# Current definition
O:37:"Drupal\Core\Field\BaseFieldDefinition":5:{s:13:"*definition";a:10:{s:5:"label";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:6:"Status";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}s:11:"description";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:27:"The status of the consumer.";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}s:7:"display";a:2:{s:4:"form";a:2:{s:7:"options";a:3:{s:4:"type";s:16:"boolean_checkbox";s:8:"settings";a:1:{s:13:"display_label";b:0;}s:6:"weight";i:100;}s:12:"configurable";b:1;}s:4:"view";a:2:{s:7:"options";a:4:{s:4:"type";s:7:"boolean";s:5:"label";s:5:"above";s:6:"weight";i:100;s:8:"settings";a:1:{s:6:"format";s:16:"enabled-disabled";}}s:12:"configurable";b:1;}}s:12:"revisionable";b:1;s:13:"default_value";a:1:{i:0;a:1:{s:5:"value";b:1;}}s:8:"provider";s:9:"consumers";s:10:"field_name";s:6:"status";s:11:"entity_type";s:8:"consumer";s:6:"bundle";N;s:13:"initial_value";N;}s:17:"*itemDefinition";O:51:"Drupal\Core\Field\TypedData\FieldItemDataDefinition":2:{s:13:"*definition";a:2:{s:4:"type";s:18:"field_item:boolean";s:8:"settings";a:2:{s:8:"on_label";s:7:"Enabled";s:9:"off_label";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:3:"Off";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}}}s:18:"*fieldDefinition";r:1;}s:7:"*type";s:7:"boolean";s:9:"*schema";a:4:{s:7:"columns";a:1:{s:5:"value";a:2:{s:4:"type";s:3:"int";s:4:"size";s:4:"tiny";}}s:11:"unique keys";a:0:{}s:7:"indexes";a:0:{}s:12:"foreign keys";a:0:{}}s:10:"*indexes";a:0:{}}
# Original definition
O:37:"Drupal\Core\Field\BaseFieldDefinition":5:{s:13:"*definition";a:10:{s:5:"label";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:6:"Status";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}s:11:"description";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:27:"The status of the consumer.";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}s:7:"display";a:2:{s:4:"form";a:2:{s:7:"options";a:3:{s:4:"type";s:16:"boolean_checkbox";s:8:"settings";a:1:{s:13:"display_label";b:0;}s:6:"weight";i:100;}s:12:"configurable";b:1;}s:4:"view";a:2:{s:7:"options";a:4:{s:4:"type";s:7:"boolean";s:5:"label";s:5:"above";s:6:"weight";i:100;s:8:"settings";a:1:{s:6:"format";s:16:"enabled-disabled";}}s:12:"configurable";b:1;}}s:12:"revisionable";b:1;s:13:"default_value";a:1:{i:0;a:1:{s:5:"value";b:1;}}s:10:"field_name";s:6:"status";s:11:"entity_type";s:8:"consumer";s:8:"provider";s:9:"consumers";s:6:"bundle";N;s:13:"initial_value";N;}s:17:"*itemDefinition";O:51:"Drupal\Core\Field\TypedData\FieldItemDataDefinition":2:{s:13:"*definition";a:2:{s:4:"type";s:18:"field_item:boolean";s:8:"settings";a:2:{s:8:"on_label";s:7:"Enabled";s:9:"off_label";O:48:"Drupal\Core\StringTranslation\TranslatableMarkup":3:{s:9:"*string";s:3:"Off";s:12:"*arguments";a:0:{}s:10:"*options";a:0:{}}}}s:18:"*fieldDefinition";r:1;}s:7:"*type";s:7:"boolean";s:9:"*schema";a:4:{s:7:"columns";a:1:{s:5:"value";a:2:{s:4:"type";s:3:"int";s:4:"size";s:4:"tiny";}}s:11:"unique keys";a:0:{}s:7:"indexes";a:0:{}s:12:"foreign keys";a:0:{}}s:10:"*indexes";a:0:{}}
The only difference is in order of 3 properties:
s:8:"provider";s:9:"consumers";s:10:"field_name";s:6:"status";s:11:"entity_type";s:8:"consumer";
s:10:"field_name";s:6:"status";s:11:"entity_type";s:8:"consumer";s:8:"provider";s:9:"consumers";
I don't know if and how this get influenced by the module, or if there's something wrong in core.
Not sure, because I do see the consent overlay for the chatbot in the left sidebar. I just don't see the entire chatbot anywhere else.
@marcus_johansson I need some help to reproduce this. I've installed DCMS 2.x and applied the AI recipe. When I'm in the backend, I have access to the sidebar with the chatbot, and that comes with consent management already. So, that works as designed, correct?
I assume, the issue you're having is for the public chatbot only. But how do I get that to show up? I don't see that in the frontend, neither as authenticated nor as anonymous user. Is there anything in addition that I need to setup to make this visible?
OK, I found what's going on: in klaro-override.css there is this selector and CSS instructions:
@media (prefers-color-scheme: dark) {
.klaro.klaro-theme-gin {
--white3: var(--gin-color-primary-hover);
--klaro-slider-bg: var(--gin-bg-input);
--klaro-slider-bg-active: var(--gin-color-primary-light-active);
--klaro-slider-bg-required: var(--gin-color-primary-light-active);
.context-notice,
.cookie-notice,
.cm-modal.cm-klaro {
border: 1px solid #fff;
background: var(--gin-bg-layer);
}
.cm-modal.cm-klaro:focus,
.cm-modal.cm-klaro:focus-visible,
.context-notice:focus,
.context-notice:focus-visible,
.cookie-notice:focus,
.cookie-notice:focus-visible {
box-shadow:
var(--klaro-dialog-focus-box-shadow),
0 0 0 10px var(--gin-bg-layer);
}
}
}
This should not be limited to @media (prefers-color-scheme: dark), it needs to take effect generally with Gin. I'll open an MR for this.
Sorry for my late review on this, I've been pretty busy over the holidays. I've now left a couple of comments, but things are looking great, and I think we can merge this very soon.
What still bothers me is the dedicated maestro/activepieces submodule. Imagine somebody is using maestro in such a context but not with ActivePieces, but with n8n or Zapier, or XYZ. They won't be able to do so, unless somebody developed an equivalent integration for webhooks on those platforms dispatched from Maestro. I'd be happy to discuss if we couldn't use the generic approach for webhooks to external platforms, then the modules Maestro and Orchestration would be fully agnostic on what external platform will be leveraged by a client.
That's a difficult one, then. We're fixing something in the code that we can't test automatically that it was broken without the fix. That makes me feel uncomfortable as this may not tell the truth either if the tests pass successfully.
When you're testing with Drupal CMS, is that Drupal core 11.3 as well?
And for your scenario where the workspace and the revision don't behave as expected, could you maybe extract the records from the database about that node and all its fields? Afterwards, doing the same with the fixed ECA version and extracting the same database records, there must be a difference somewhere. Or maybe it's the cache again?
@pameeela, sorry, I can't reproduce this. Is followed your steps to reproduce, and this is what I'm seeing:
Now, not only the background looks as expected, it's also not a modal in my case, it's just a div container overlaying the chatbot. Are we having more differences in our scenarios?
Thanks @alexpott and @acbramley for fixing and testing this.
Regarding the class test I guess he can find that in similar contrib integrations.
Not sure if we find any similar thing for CrowdSec integrations, but this should simply do a class_exists('\Drupal\crowdsec\ScenarioInterface')
@klemendev there are several things available, but not all of the comfort we would like. We just need more resources to make things happen. Here is what's available:
- In Drupal's logs, you can see each blocked request
- When you enroll your Drupal site to an account on CrowdSec, you can see nice statistics there, but that doesn't include all the blocks on the Drupal site
This is unfortunately how the update manager in Drupal works. It doesn't check constraints, but there's nothing we can do about this as module maintainers.
I have left a comment in the MR. In general, this looks already good.
Thanks @mglaman for suggesting this, it's how it should have been from day 1. Fixed now.
jurgenhaas → made their first commit to this issue’s fork.
This got addressed as part of ✨ Add support for 403 subrequests to user.login route Active
This looks like a duplicate of 🐛 Broken file upload, wrong upload validators. Active
jurgenhaas → made their first commit to this issue’s fork.
Considering latest changes and comments in #53, this needs another review. I've just rebased the MR for this.
jurgenhaas → made their first commit to this issue’s fork.
This is not within the scope of this module. It styles the user login, registration, and password reset pages. When those pages get used gets decided by other means, and there are a lot of modules (including core) that have an impact on that.
@jwilson3 I improved the theme negotiator in ✨ Add support for 403 subrequests to user.login route Active and that should also cover all other cases where the current request is not directly identifiable as a gin_login path. Please try if that's true.
Then, I wonder if we shouldn't grab the result from the theme negotiator in _gin_login_gin_is_active() instead of applying separate logic once again.
Found a way to determine the route of the sub-request, this should resolve all similar issues.
jurgenhaas → made their first commit to this issue’s fork.
Thanks @4kant for letting us know. It's funny though since the fix that I referred to in #22 is not yet released. But I'm going to mark this as fixed according to that feedback. If anyone still finds something that needs our attention, please open a new issue or re-open this one if it's within scope.
Could the ECA model still be present in the test environment when the new test runs?
No, even if I just run that single test with --filter ContentUpdateWorkspaceTest::testNodeUpdateInWorkspaceWithoutEcaModel it still runs without any failure when the fix is removed.
Your comment #11 made me wonder if the issue is probably making some of the changes, like e.g. setting the workspace property on the node revision, but maybe misses some other required properties to be updated. That's what we should to find out before we can move this forward.
@freelock I've added another test which doesn't come with a workspace. But still, that doesn't fail either when I manually remove the fix in the update hook of ECA.
I wonder if in your case, where this fails, the workspace gets correctly set to the node, but something else is missing which is not properly set due to the ECA issue? And as we only test for the correct workspace property with the node revision, that may always be set correct, but we should test for something else in addition to this? Would be great if you could check in your database what's stored with the node regarding the workspace property, and maybe even what's missing.
The fix in the related issue just got merged. Please give it a try if this one can be closed now.
Thanks @gcb for fixing all this. I've just merged it and also fixed this is the bpmn_io module v3 in 🐛 Boolean fields flagged with non-supported value Active , because this code was moved out of ECA into that module, where it belongs to. But for ECA 2.1, the fix is correct right here.
jurgenhaas → changed the visibility of the branch 3553506-default-value-v3 to hidden.
This looks like it had to be removed as PhpStan (which wasn't in use for version 1 of ECA) must have complained about an unnecessary if-statement, as the $display = $form_object->getFormDisplay($form_state); can never be NULL according to its function declaration. If that method returns NULL, then there must be a bug elsewhere.
@abarrio as you're experiencing this issue right now, do you have a chance to debug the situation to find out where and why this is happening? Maybe, if a NULL value is allowed here, then we first have to update \Drupal\Core\Entity\ContentEntityFormInterface::getFormDisplay in core. But maybe there's a contrib module that implements this and doesn't follow the declaration, then the bug would be with that module.
Merged. I'll review the issue queue if there's anything else that should be included as well. You should expect a patch release soon.
Just re-opening this as I'm also seeing this on 11.3.2 again. Looks like a regression?
That's an interesting analysis. What strikes me that only very few sites seem to be affected by this. There are more than 1500 sites using ECA 3, and only a couple are reporting this issue. Of course, some may still not be on 11.3, but we know of many that are, and none of them are having this issue.
Anyway, we can give this approach a try. Next step could be that you create an issue fork and provide an MR with the proposed solution. I'm more than happy to review and test this.
Thanks @freelock for the test. I've now extended the unit test to verify that this is working as well. The test passes, that's the good news. However, when I temporarily remove the fix from the update hook, the test still passes. Looks like I'm not yet testing every aspect of the issue. Can you please have a look what's missing?
That's great, thanks for reporting and testing @rex.barkdoll, so we eliminated one of those nasty bugs. Merged this and also back ported to ECA 2.1
I can't reproduce this, all the tamper plugins are there on a fresh installation. And we also upgraded a lot of sites to the latest releases and they all have the tamper plugins available too.
Looks like we introduced a bug in the set field value action in this issue: ✨ ECA Form: set field value in ajax calls Active last year in May. It receives the form element before replacing tokens in the field name. I'm just changing the order in the MR here, please give that a try.
Thanks @godotislate for testing this. I went through all your steps again and I can confirm that this is not an issue with the media field for some strange reason.
But it is in the CKEditor, although the links in the pager are identical, AFAIKT.
Rebased the MR against main and changed the version here.
This sounds like a duplicate of 🐛 Save button missing if the content have a webform block Active .
Merged this into 4.1.x and cherry-picked into 5.0.x. This will not go into the core version of this theme since this needs to be covered by the paragraphs module in the future.
Could you please create an issue fork and provide an MR instead of a patch?
Should this issue be postponed until the related issue in core is being merged? Or is that independent?
I've made the branch for MR 738 visible again because that's the place where this will be worked on. Patch files are not used in this project.
Before we can move this forward, there are 2 things:
- There is a comment in the MR that needs to be addressed
- This needs to be tested with Gin 5 to verify if it's also affected together with Drupal 11.2 and 11.3
jurgenhaas → changed the visibility of the branch 3567522-extra-search-field to active.
Fixed that by increasing specificy for the toggle button to overcome the revert all from the navigation module.
Another patch release to be expected later today.
I've pinged contributors to the related issue in Slack asking for their review.
On one of the sites where I'm seeing this, there are many regular notices that IPs are being blocked, interspersed with occasional 500 errors.
We're seeing those 500 errors from the CrowdSec backend on some sites as well occasionally, like 1 or 2 times a day. On others, we don't. To resolve this, one would have to open issues with CrowdSec trying to sort this out. We haven't had reasons to pursue that yet.
But by contrast, on the lower-traffic sites, there are only "Buffered signal to drupal/4xx-scan" warnings and the 500 errors from yesterday, and nothing since -- so perhaps it's working, but just not getting traffic that would generate a block?
Yeah, telling what's going on for low-traffic sites is difficult. Until you get bots that hammer such sites, then you'll see the positive effect very quickly.
Is a cache-clear required after enabling and configuring?
No
Frustratingly, one of these sites is getting pestered by an attacker scanning for vulnerable .php pages, resulting in dozens of 4xx status codes all issued at the same time, but every request comes from a unique IP address, and is not getting blocked by the whisper feature.
Yeah, that's where IP-based blocking isn't suitable.
That error comes from the CrowdSec PHP library which is trying to fetch block lists from the CrowdSec backend. That's not coming from this module. The request is being submitted correctly, but the CrowdSec server responds with a 500 server error. There is nothing we can do about this. The only question is, does this happen all the time, or just sometimes?
@smustgrave I'm uncertain at this point if my suggested solution is the best approach. We may require some feedback from @catch and @berdir first, as the change got introduced when they migrated the preprocess_pager to OO hooks and they may have some different ideas on how that should be handled.
Thanks for reporting and fixing this @balagan
Icons are not correctly centred horizontally:
Wrong font:
Thank you everybody for reporting, testing, and contributing fixes to this issue. We've decided to merge the current MR and tagged 5.0.11 - not because it fixes everything but it's a good improvement that all sites should benefit from asap.
As for further issues, please report still outstanding issues in 🐛 CSS revert all from navigation module clean-up Active where we plan to continue working on this. Just add comments and details to that issue, I'll take care of updating the issue summary on a regular basis then.
The MR fixes the issue with the media library, but I'm uncertain if that's the correct approach, therefore not writing any tests yet.
Turns out the mini pager pre-processor does not receive the route name argument from the plugin, that's why it needs to remain hard-coded. I've updated the original post accordingly.
We do see the same error in Edge but don't even have CSP installed.
This is a great approach, thanks @mxh
I've updated the hook in ECA 3 to make sure it runs last. And I've added a test to make sure that the revision handling really works. This can manually be tested by removing the code in question, then the tests fail.
@freelock could you please test if the workspace issue is being resolved this way?
If that's the case, we need to write the second test, and also back port this to ECA 2.1 where the reordering of hooks works differently.
Not sure this is a bug if we can't reproduce this. LMK if you find any more infos that may help with that.
@lostcarpark couldn't agree more. As you said, this needs a quick resolution and hence, this approach is probably the right way for a patch release maybe as soon as today?
The proper solution would be to implement OO hooks in smart_trim as well. That would then also bring tokens into a hook class, you don't even need plugins for that.
I wouldn't be surprised if the Event: Response created comes too late. If the response has already been created, Drupal will most likely no longer respect any redirects, except for those scenarios that you described.
In my experience, the Controller found to handle request event is the most appropriate one for all things kernel related.
Marking this as a support request, as it's certainly not a bug. The event comes from Symfony and the redirect action comes from core. So, ECA can't do anything for either of them, it just makes them available in the processing. But the way they work is totally out of ECA's control.
Just ran into this as well, and yes, the function _token_field_label no longer exists in the token module since they implemented OO hooks in the latest release. Functions with a leading underscore like _token_field_label are private and shouldn't ever be used as it is expected that they either change or disappear without notice.
This one is now \Drupal\custom_field\Hook\TokenHooks::tokenFieldLabel, but with a private scope, so it can't be used even with an update code here in smart_trim. To solve this, the method needs to be implemented by smart_trim itself.