I think the message added by the logger is confusing. I ran into this issue when trying to deploy updates to a site. Somehow a couple of roles had permissions left that no longer existed. (Workflow permissions from a content type that was removed). The updates were trying to add new permissions that did exist to the role, but the extant permissions were triggering this error.
In my case, silently removing the permissions that no longer exist is what I'd like to happen. The decision to remove those permissions has already been made. The fact that they're still assigned to a role is an error, and one that we can easily correct for people. Logging an error saying "Adding non-existent permissions to a role is not allowed." is confusing, as nothing was trying to add non-existent permissions.
I've gone with patch #11 and it's worked well for me.
Excellent - thankyou!
That call to sleep() actually came over in some code I copied from OpenSearchBackendTest. I've swopped it out in both that test and the new SpellCheckTest for a call to refresh the index. The tests both pass, and now they run faster, which is nice.
I've added a kernel test to cover the spell check feature. How's that looking now?
Thanks for the review kim.pepper.
I've added unit tests for the new classes SpellCheckBuilder and SpellCheckResultParser. While doing that I realised that the builder was getting passed data it never used, so I've cut down the params to SpellCheckBuilder::setSpellCheckQuery(). I also found a bug in SpellCheckResultParser::parseSpellCheckResult(), where suggestions that appeared early in the response from OpenSearch would be ignored in favour of later ones with the same score. I've replaced the logic there with code that won't ignore any suggestions, and ranks by both score and frequency to find the best ones.
I'm going to work on a functional test now.
The tests are green now (except for some code style failures in other classes unrelated to this change).
I've tried out the code in this PR locally, and it's working well for me.
Great - I'm happy to add tests for any changes. It's something we aim to do for all the code in LGD too.
The only other change to the module that we're looking at right now is the existing issue #3025156. One of our councils has run into the same issue on two of their sites, and found that the patch you put in that issue resolves the issue on one of them but not the other. I'm going to work with them to find a solution that works for both of them.
Good call. Here's a updated patch with a unit test for the new option.
I've granted you permission. Go for it :)
Uploading a patch that implements a setting to keep existing IDs when filtering.
#3222058 already exists to get rid of the dependency on fakeobjects (and therefore the dependency on CK4).
This needs work as the text isn't helpful. It's just a copy of the text from the project page on d.o. It even includes installation instructions.
I've tested this. With this patch applied (and all the patch does is remove the core key from the info.yml, and add D10 to core_version_requirement) this module works well on D10.
I've attached a screenshot of the testing I did for proof.
You're right, this will work on D8 from 8.7 onwards, when the extension.list.module service was added. (I read the change notice incorrectly and thought that was added in 9.3).
While we could specify ^8.7 in the core_version_requirement key, that was introduced in 8.7.7 and the old core key isn't there, so the module already states it compatibility correctly.
I'll set this back to RTBC.
Closing this as it looks like all this work's been done elsewhere.
Closing as answered.
Duplicate of #3296307, which I note that you also made. Please don't make duplicate issues.
-core_version_requirement: ^8 || ^9
+core_version_requirement: ^8 || ^9 || ^10
Shouldn't D8 be removed from here? And a constraint on whichever point release of D9 added the services this patch uses added?
This issue isn't wrong as such, but Drupal 8 has been EOL for a long time now, and any future release of this module that might include this change is unlikely to work with it at all.
This issue duplicates #3343180, which is further along.
I have also sorted some cards.
Multiple field values aren't working for me. I'm using version 3.2.0 of the module.
See the attached screenshots - I've created an event with one recurrence weekly on a Wednesday, and another on a different time on Saturday. When viewing the event, only the Wednesday occurrences are displayed.
Patch to fix. This adds a check to see if this.href is a string before trying to use it like one. It's an object on the links in the SVG data.