ekes → created an issue.
Reviewed and spotted one last nullable type. For the rest looks good.
Note this does not change the function signature see my note about this on https://github.com/commerceguys/addressing/pull/221#issuecomment-2582855457 so shouldn't require more than a patch release.
Will require https://www.drupal.org/project/date_recur/issues/3498936 🐛 DateTime Range Views Data was converted to a class, hook removed Active
Passes PHPUnit.
I don't thing the cspell, phpstan, phpcs are related to this change?
Was just coming here to do something just like this. Patch adds nullable to all declarations that have a NULL default, and shuts up our 8.4 test errors \o/
I'm just trying to recreate this. Are there some steps I could follow?
Shall open a new issue for any remaining/new.
\Drupal\Core\Render\Element\FormElement::* is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\Render\Element\FormElementBase::* instead. is address module.
The "Drupal\entity_browser\Entity\EntityBrowser::*" method will require a new "string *" argument in the next major version of its interface "Drupal\entity_browser\EntityBrowserInterface", not defining it is deprecated. is Entity Browser module.
Deprecation we should probably catch
1) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
The "Drupal\entity_browser\Entity\EntityBrowser::setWidgetSelector()" method will require a new "string $display" argument in the next major version of its interface "Drupal\entity_browser\EntityBrowserInterface", not defining it is deprecated.
Triggered by:
* Drupal\Tests\geo_entity\Functional\GeoBundleCreationTest::testMediaTypeCreationForm
/var/www/html/web/modules/contrib/geo_entity/tests/src/Functional/GeoBundleCreationTest.php:74
* Drupal\Tests\geo_entity\Functional\GeoOverviewAccessTest::testOverviewPageAccess
/var/www/html/web/modules/contrib/geo_entity/tests/src/Functional/GeoOverviewAccessTest.php:106
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
2) /var/www/html/vendor/symfony/error-handler/DebugClassLoader.php:341
The "Drupal\entity_browser\Entity\EntityBrowser::setSelectionDisplay()" method will require a new "string $display" argument in the next major version of its interface "Drupal\entity_browser\EntityBrowserInterface", not defining it is deprecated.
Triggered by:
* Drupal\Tests\geo_entity\Functional\GeoBundleCreationTest::testMediaTypeCreationForm
/var/www/html/web/modules/contrib/geo_entity/tests/src/Functional/GeoBundleCreationTest.php:74
* Drupal\Tests\geo_entity\Functional\GeoOverviewAccessTest::testOverviewPageAccess
/var/www/html/web/modules/contrib/geo_entity/tests/src/Functional/GeoOverviewAccessTest.php:106
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
3) /var/www/html/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:52
\Drupal\Core\Render\Element\FormElement::processGroup() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\Render\Element\FormElementBase::processGroup() instead. See https://www.drupal.org/node/3436275
Triggered by:
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud (2 times)
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
4) /var/www/html/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:52
\Drupal\Core\Render\Element\FormElement::valueCallback() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\Render\Element\FormElementBase::valueCallback() instead. See https://www.drupal.org/node/3436275
Triggered by:
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud (2 times)
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
5) /var/www/html/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:52
\Drupal\Core\Render\Element\FormElement::preRenderGroup() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Use \Drupal\Core\Render\Element\FormElementBase::preRenderGroup() instead. See https://www.drupal.org/node/3436275
Triggered by:
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
6) /var/www/html/web/core/lib/Drupal/Core/Test/HttpClientMiddleware/TestHttpClientMiddleware.php:52
Renderer::renderPlain() is deprecated in drupal:10.3.0 and is removed from drupal:12.0.0. Instead, you should use ::renderInIsolation(). See https://www.drupal.org/node/3407994
Triggered by:
* Drupal\Tests\geo_entity_address\Functional\AddressFormsTest::testCrud
/var/www/html/web/modules/contrib/geo_entity/modules/geo_entity_address/tests/Functional/AddressFormsTest.php:86
finn lewis → credited ekes → .
finn lewis → credited ekes → .
amateescu → credited ekes → .
Thanks for filing this and for the fix.
Is there a scenario where old URLs should be supported too? In other words, should be change the regex to the new format or should we just accept the new format as well?
I think I was planning that (?:\/episodes) but then didn't follow through ?. As seen in the in the update from Buzzsprout themselves it's only the new format for their content. In our case however, there will be entities with the old pattern that have it in the field with the old URL, no point in updating that.
OK a bit more than the minimum change to the regex. Add the test and user feedback too.
This patch is specific to configuring for Buzzsprout. Which is actually really handy for me, as that's what I was doing :-) But it should probably be an example addition to the readme, rather than a replacement for the generic description that there is. I'd guess more specific examples might be good, I've not tried if there are other specific differences.
> Out of memory issue reported in #32 still occurs with Leaflet 10.2.20 using a Search API view. Attached patch fixes it.
So far as I can see this stops loading the field data from the Index Item Item::fieldsExtracted is basically only set once Item::getFields has retrieved them from the backend. So unless something else is calling this before leaflet does, the data will be falling back to load it from the entity. Which is what some use cases are trying to avoid. This will usually work, and you won't notice unless putting break points in. It does not work for some other cases, where the field is on a referenced entity.
If I'm correct that @idebr case means, either something else has added the extracted data (not search_api or search_api_solr code that I see), or loading it (probably also retrieving it from the entity) within Item::getFields takes more memory than directly doing a getValue on the field is probably a Search API issue, not one for Leaflet to solve.
It was an unrelated issue, not caused by this one. Behaviour here is correct.
Noticed some possible issues with workflows publishing. Going to put this on needs work until clarified.
I just went searching for this feature as I yet again forgot that the default theme is used for 'render item' which means I can spend as long as I like configuring the view mode, but I still need to also add a template for that view mode or the default one (which will have some heavy customisation) will kick in and override everything I did in the UI config.
I guess I'm going to quickly make templates again. But for next time:
I'm thinking this option would be useful specifically on the RenderedItem processor. So the option to choose theme would make sense with the choice of which view mode to use. Just once? Not for every view mode?
Hopefully clarified.
Serivce name typo should also be fixed. Tests pass I think.
Update 📌 Allow CRUD operations for Redirect entities in a workspace Needs review works.
Adding a patch version of the MR for a patch ready snapshot.
> When content gets published to the default workspace, the redirect_path_update hook works as before and handles the generating of the redirect.
This was making sense to me, why would I need redirects in stage environments. That was until I realized we were using https://www.drupal.org/project/pathauto/issues/3283769#comment-15541961 🐛 Allow path generation inside of a workspace Needs work which stops the re-generation of a pathauto alias when the workspace is published (which makes sense, especially because in experiments with paths that include `[node:entity_reference_field:entity:url:relative]/[node:title]` the order of publishing could cause the alias of the parent not to be available in the live space before the reference entity alias was regenerated hence causing the incorrect alias to be generated).
So related issues (as I'm looking) are: 📌 Allow CRUD operations for Redirect entities in a workspace Needs review and #3187274: Redirect deletion in workspaces →
In the MR !9062 now. Which is passing tests.
The MR https://git.drupalcode.org/project/drupal/-/merge_requests/9062#note_352712 against 11.x on #3179199 also fixes this.
In creating the branch https://git.drupalcode.org/issue/drupal-3179199/-/compare/11.x...3179199... against 11.x I ended up with the same code as @acrazyanimal in #9 it truly does fix both #3179199 and #3132022 but rewrote the tests against the present state of the class and found that to make them clear they should be separated I think. The tests are in the first commit, so can easily be run to confirm they do fail with the present code.
@sam152
> Has the scope of this issue grown to include #3132022: Content moderation check does not take into account entity IDs being used by two content entities? We should either try to limit the scope of this issue and tackle the problems separately, or update the issue summary to describe all the scenarios that are being tested and the bugs that are being fixed.
@acrazyanimal
> Thanks @Sam152. Yes I it does look like it covers the scenarios from the issue you linked above. Reviewing the comments in that issue's thread it looks like Alexj12 also increased the scope of that issue to cover the original issue here lol. I'm not sure what to do really. My inclination is to redefine this issue as an all encompassing issue since the tests are overlapping. I will think it over and possibly reduce this and contribute to the other issue.
Looking I'm starting to get convinced https://www.drupal.org/project/drupal/issues/3132022#comment-15708738 🐛 Content moderation check does not take into account entity IDs being used by two content entities Needs work that the query there is just going to throw too many false positives, for this, and the referenced issue, and for the other example I mention. So it might needs tweaking, not just filtering out revision ids.
Looking at this for the first time, having encountered it on a site in development, and it seems that this can not only happen if there as explained in the OP:
A non-workflow non-revisionable entity with a revision id that is the same as another different entity type revisionable entity (previous not default) revision id that is in a non-published state (the path_alias and node example)
but also if there is:
Any (including workflow revisionable) entity with a current default state revision id (published) that is the same as another different entity type revisionable entity (previous one not the current default) revision id that is in a non-published state.
eg: A taxonomy term ID 1, Revision 1 Published; and Node 1, Revision 2 Published (but also a Revision 1 not published).
The $tracked_revisions https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/conte... returned are going to be $tracked_revisions = [ 'taxonomy_term' => [1], 'node' => [2] ] which becomes $tracked_revision_ids = [1, 2] when queried in the table https://git.drupalcode.org/project/drupal/-/blob/11.x/core/modules/conte... will return 1 because it is catching the tracked, but no longer relevant node revision.
Looking at the base test class ContentModerationStateTest and WorkspacesContentModerationStateTest extend it, only one entity type is tested at a time for this. So it feels like it might be clearer to make a new test that reflects workspaces behaviour more, where several entities of different types are being published at the same time - obviously with some clashing revisions id's included.
ekes → created an issue. See original summary → .
valthebald → credited ekes → .
Think that covered them. The additional Unit test for the Negotiator is left, but might just duplicate what is in the Access Policy test.
Switched to use GroupSitesNegotiator naming, added setting the CacheMetadata. Made this optional. These tested in the Kernel test.
Then changed the AccessPolicy to use the new Negotiator. Updated the Unit tests to reflect the changed internals, and added equivalent Unit tests for the GroupSitesNegotiator.
Logging these notes about how where we're requiring the group from context in our code base at present. Just posting here, really notes for myself about the cacheable metadata. In the interim I'd made a .module function to get the group, we're calling this, so looking at where we're using it, some are legacy, but:
Decide which page to redirect to on login
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
I don't think we mind the cacheable metadata, but we do have it from $form_state.
Adding a relationship entity for content made outside the group forms on a group sites domain
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
and also legacy versions of doing that, which we can remove
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
Both cases, we've got the entity, so can pass the cacheable metadata from it, but don't think it makes a difference?
Something clever with the Taxonomy Term form
https://github.com/localgovdrupal/localgov_microsites_group/blob/4.x/mod...
which probably will benefit from having cacheable metadata.
Finally some workaround
https://github.com/localgovdrupal/localgov_microsites_group/blob/98c4e17...
where it looks like the original cache tags aren't yet correct. We can get cacheable metadata from the entity, but it's not going to make a difference here.
I can recreate the error, although it only requires resubmitting form to fix. It seems only related to specifically: address field, subfield `Address:administrative_area (exposed: country set via exposed filter)` which is a dependent field shown based on the value of the country exposed field. I'm not going to spend longer on the error if as it seems on a clean install it's not actually preventing operation. But if I stumble on it again within that site I'll post a follow-up.
Isn't it the schema that is wrong, or rather missing? Those look like they should be integers not strings. I see there is for example viewsreference.enabled_settings.limit which is set for integer. It will be that it also needs one for when it's used *view:?
> don't have the why on this part,
I can investigate further if absolutely necessary. But I think it is going to be because: (a) as mentioned, even if you index a field it is not used in the results from Search API (except in very specific Solr configurations); (b) the values that are used are from the loaded entity; (c) this does not yet include related entities, and in this case the geofield is on an entity referenced from the base entity.
> The issue occurs only when we use Search API Index based View.
Regarding the memory usage, as I see it here. I've looked at in the past, and did supply a patch or two that helped bring it down. But what I noted was:
First with Search API either DB or Solr unless you have done quite some work you will be doing an entity load for every row returned (so every point). You can do the work with Solr to make it use the field data and not do an entity load. This will reduce your memory usage significantly. There is a patch in Search API module I started to do the same with the db backend, but it's only a start, and requires much more work.
The second point where there is quite some memory usage is the same if you use views sql or search api, and probably doesn't come into play if you're talking just 1000 points or so. But I'll mention it here as I looked at this too at the same time. When constructing the points for display on the map there is a lot of configuration and the whole loop over all the points and creation of the output is quite intensive. This I don't see any way round, because doing anything else would loose the features of the module, being able to configure the output such. With flexibility comes some complexity.
Just a heads-up if anyone else notices this. I've not had time to investigate myself but we have a report that this patch broke a Search API based view https://github.com/localgovdrupal/localgov_directories/issues/378
Added Kernel tests to describe the functionality of the patch in https://git.drupalcode.org/issue/date_recur-3010184/-/compare/3.5.x...3.... The branch does not (yet) look at the views integration. I'll be honest I'm still a bit confused that it indexes the default version, and the latest version; and then only the default and latest. I guess this is where looking at the views integration comes in.
Additional explanatory note https://git.drupalcode.org/issue/date_recur-3010184/-/compare/3.5.x...3.... The existing tests passed with field values of cardinality 2, but storage of 1, because they were working on the inserted entity. But on loading (the default revision) from storage, of course, the second value was not loaded as it was not stored. Basically presently should you save an entity with a date recur field with more values than its cardinality it does calculate occurrences for the invalid deltas.
ekes → changed the visibility of the branch 3010184-create-occurrences-for to hidden.
BramDriesen → credited ekes → .
157 merged.
Many thanks.
https://github.com/localgovdrupal/localgov_guides/pull/155#issuecomment-...
> Is this a dupe of #145 ?
> If so if you could review that we can get it merged. If not maybe the two should be combined to add what ever got missed in the first?
I'm going to put this question here, as documentation, I couldn't find it anywhere else.
What is the minimum set of permissions for a solr role that will work out-of-the-box with the module? For example including the upload config. I see by default:
security-edit
security-read
config-edit
config-read
collection-admin-edit
core-admin-edit
core-admin-read
read
collection-admin-read
update
health
metrics-read
filestore-read
package-read
package-edit
schema-edit
schema-read
zk-read
all
I assume several of these that are needed could also be constrained to a collection.
@kerasai Did you continue to use this? Can you confirm that the 3.x compatible version works with 2.x as well? I'll change the dependency if so.
Aside: After all the work to make domain_group 3.x when group_sites came along we've decided it's worth switching to use it. Maintained by the Group module maintainer.
Fixed in 1.1.0-beta2 Thanks both.
Would still be nice to get some more tests over the new function that have been added.
Fixed in 1.1.0-beta2
ekes → created an issue. See original summary → .
(FWIW) Possibly not dissimilar use case, but we have the users with an admin role being able to log into additional admin domain which I ended up fixing in admin mode https://github.com/localgovdrupal/localgov_microsites_group/blob/4.x/src... The whole domain is then customised to make doing admin tasks easier.
DDEV is offered in addition to Lando. The project mentioned `composer create localgovdrupal/localgov-project` includes https://github.com/localgovdrupal/localgov_project/tree/3.x/.ddev
Assuming this is in the section that renames the local tasks which is the reason for the comment https://git.drupalcode.org/project/date_occur/-/blob/0.1.x/date_occur_ui... It's also partly the reason all that code is separate in the _ui module. It should fail gracefully however, and not explode. So the fixes there have been helpful. Not changing the local tasks names in this case isn't essential.
That said the parameter bag that it is being retrieved from is stored key => value, it seems convention that the parameter of the primary entity on the route will be keyed with the entity_type_id, and so far I've not seen an example for an entity route implementation that will fail on this. If there is one maybe can look to see if it can be made more robust.
Seems in moving the field storage id into the route parameters I'd not taking into account it was creating multiple routes (with different param values) but the same path. So opting (almost) for A putting back just the field machine name into the URL fixes this https://git.drupalcode.org/project/date_occur/-/commit/26b531bf11cb13bed...
Merged, along with a fix for 🐛 occurrence page crashes if there are more than one occurrence field on an entity type Active , should be safe to have multiple occurrence fields on different bundles of the same entity type.
Many thanks.
Ah! I thought you meant more than one field on the same entity type.
I'll look back at the original repo where I removed the field name from the path see what comment I made, and see if it makes sense to put it back for working out the field to load.
snowballstem.org is
The Snowball compiler translates a Snowball program into source code in another language - currently Ada, ISO C, C#, Go, Java, Javascript, Object Pascal, Python and Rust are supported.
So not PHP, and not the PHP stemmer this module uses https://github.com/wamania/php-stemmer
There are two options I can see.
- Convert the snowballstem algorithm into PHP and submit it to the PHP module.
- Or if people are going to use it: you can use the snowballstem library (which if I see the explanation it needs compiling and placing the .so file in you path) via PHP with https://github.com/amaccis/php-stemmer It would be possible to update this module to alternatively use that in addition
In the beginning, but not implemented, was the idea to also allow having augmenter entities. Rather than close, overrride and replacing in the index as is implemented, these would another entity type that just has the fields to add/replace data on the original. So the possibility is still there in the original architecture, but wasn't something that was needed for the project the module was originally written for.
Amusingly I think I did have the $field_id in the path at some point and refactored it out to simplify things. I think there might be other complexities involved with having multiple fields on the same entity. Is there a particular use use case for having more than on?
The
comment #7 →
is correct, and really if these are calculated it should result in them being calculated before the entity save when using the short $entity->field->value method for setting.
I've opened a new issue, rather than re-opening this rather old one, also as it's a bit more involved. It includes that it is recalculating it when it needn't be, and with a failing test in a branch here 🐛 Computed values Active
I'd say this is blocked on 🐛 Computed values Active
Pushed an update to the test in the branch which will fail because all the computed values are loaded from the db as NULL.
It uses a reflection class to get the field values from the entity as they came from the EntityLoad without doing any field loading.
// Before accessing the entity field and incidentally trigger a calculation.
$relection = new \ReflectionClass($entity);
$property = $relection->getProperty('values');
$property->setAccessible(TRUE);
// Retrieve the values as they came from storage.
$values = $property->getValue($entity);
The first test added passes as the $entity->geofield_field->value does set a wkt value to the value property and this is stored.
// Check stored set value.
$this->assertEquals($value, $values['geofield_field']['x-default'][0]['value']);
But the following tests all fail because while the property values have been loaded from the database, they are all NULL.
// Check stored computed values.
$geom = \Drupal::service('geofield.geophp')->load($value);
$centroid = $geom->getCentroid();
$bounding = $geom->getBBox();
$computed = [];
$computed['geo_type'] = $geom->geometryType();
$computed['lon'] = $centroid->getX();
$computed['lat'] = $centroid->getY();
$computed['left'] = $bounding['minx'];
$computed['top'] = $bounding['maxy'];
$computed['right'] = $bounding['maxx'];
$computed['bottom'] = $bounding['miny'];
$computed['geohash'] = $geom->out('geohash');
foreach ($computed as $index => $computed_value) {
$this->assertEquals($computed_value, $values['geofield_field']['x-default'][0][$index]);
}