Sorry about this, I am still getting used to this new procedure, I indeed added you as a contributor but forgot to press save. I will try to do better next time!
This is a problem in the D11 patch, let's fix it there. Marking as a duplicate of 📌 Automated Drupal 11 compatibility fixes for commerce_price_rule Needs review .
Sorry for the long delay, I don't work on a project any more that uses this module and it fell off my radar. I have merged it. Thanks all and especially @joseph.olstad for reaching out personally.
I am a maintainer and I support Joseph's application. I do not have the privileges to add members to the maintainer team though.
RTBC+1
Nice find, thanks a lot! I solved the merge conflict and set it to auto-merge.
pfrenssen → made their first commit to this issue’s fork.
Thanks a lot for working on this! Merged both in 2.0.x and 3.0.x.
pfrenssen → made their first commit to this issue’s fork.
pfrenssen → made their first commit to this issue’s fork.
The version range is too broad, we should only declare compatibility with versions that have been tested and are known to work. Using open ended version ranges will allow the module to be installed on any future version and that will cause sites to break.
Reviewed the patch, it looks good. Only the info file needs to be updated, no other necessary changes are detected by Upgrade Status.
pfrenssen → changed the visibility of the branch 3430062-automated-drupal-11 to hidden.
The TransactionDataEvent has been deprecated in favor of the PaymentIntentEvent but both events seem to function in different ways. There is no direct upgrade path between the methods in both events.
It would be great if we could have a change record with before/after examples that showcase how to migrate from the old event to the new event.
Also, can we have some information on what this is postponed on?
pfrenssen → changed the visibility of the branch 3431472-automated-drupal-11 to hidden.
pfrenssen → changed the visibility of the branch project-update-bot-only to hidden.
pfrenssen → changed the visibility of the branch 3363802-drupal-10-comatability to hidden.
Deleting the cache files is OK for non-persisted bins but not for persisted bins. We should move them to the new folders or the persisted data will no longer be available to Drupal after updating the module.
Applied the patch from #5 and removed an unneeded change. The fix works to prevent the error from the issue summary. I reviewed the patch and it looks good. It is OK to set an empty value on the new field, because the module can handle a missing value. There is a check for it in BehaviorSettingsManager::getEntityBehaviorSettings().
There are multiple different errors being confused here. Let's focus this only on the error mentioned in the issue summary. Comments #2 #6 #8 #10 are different error messages and are not in scope of this issue.
I like the idea of using an md5 hash of the cid, this will more evenly spread out the cache files over many different folders. I am wondering if we should provide an update path to move the existing files into the folder for cache bins that have the persisted option set?
Great that you have been able to work around the problem. I will for now close this issue as fixed. Some more ideas:
- To save on disk space, you could try to employ compression, see ✨ Serializer should be configurable Fixed .
- Make sure cron is running, the system cron job will trigger garbage collection on cache bins, which should delete invalidated cache entries.
- Using file cache for all the cache bins is probably not the best idea. You can decide for each cache bin which cache backend is the most suitable. For example you can store session data in Redis, page cache on the file system, render cache in the database, etc.
I have been going through the settings and while there are hundreds of settings, I think the vast majority are not useful at all for a decoupled frontend. Probably some settings can be useful in some rare cases, but most of them are intended for administration purposes or to control backend processes.
I think the page URLs are probably the ones that are most useful, then a site builder can choose on which path the webform is shown in the frontend. The messages for open / closed forms are also useful, but we don't yet expose this status, so I split this off to a separate issue #3552668: Report whether a form is open or closed → . The rest of the settings seem rather useless for the majority of decoupled frontends.
If a setting that you need is not exposed, feel free to open a feature request!
I reviewed the MR but it looks like the problem was already fixed. I checked the git log and indeed it was fixed recently in 🐛 JavaScript error in toolbar.js when using paragraphs_edit frontend editing Active . Marking as a duplicate.
Since this is also affecting older versions of Drupal, it could be interesting to backport this to 11.2.x and 10.x. See for example 🐛 document.querySelector is null Active which reports this issue affecting 10.1.x.
It is totally OK to use AI agents to help with repetitive tasks like adding missing documentation, but please review every single line of AI generated code carefully before committing it. The LLMs can make a right mess of things. I just spent my entire evening fixing hundreds of lines by hand.
Anyway apart from this, the sheer amount of work that was done is incredible, thanks a lot @jeremy1606 for the effort!
This still needs another review before it can be committed.
Cherry-picked the fix into the MR for 📌 Automated Drupal 11 compatibility fixes for currency Needs review . Closing as duplicate.
I did a review of the MR. There is still some work left.
This is a valid problem, but since this is Drupal 11 specific, it would be better to roll this into 📌 Automated Drupal 11 compatibility fixes for currency Needs review .
pfrenssen → made their first commit to this issue’s fork.
I did an iteration on it, but there is still an issue with this. It looks like there is a difference between [node:field_link_2:path] and [node:field_link_2:url:path]:
[node:field_link_2:path]seems to return the relative path including the base URL (/web/foo/bar).[node:field_link_2:url:path]seems to return the relative path without the base URL (/foo/bar).
This is interesting and yes I agree 255 characters will not be enough for all possible use cases.
I found a discussion about URL lengths on Stack Overflow: https://stackoverflow.com/questions/417142/what-is-the-maximum-length-of...
In short, it highly depends on the use case. In general there is some consensus in this discussion on using 2048 characters as a limit, but that's possibly outdated because this seems to have been mainly a limit of Internet Explorer. Some clients apparently can handle URLs up to 2GB, but that would be a tad excessive.
Needs to be rerolled now that 📌 Convert hooks to OOP Active is in.
Hiding patches, drupal.org is moving to merge requests. See Using Gitlab to contribute to Drupal → .
pfrenssen → changed the visibility of the branch 2995680-error-on-call to hidden.
Created new MR against 2.0.x. It looks like 2.1.x has not been worked on for a long time, and 2.0.x is more recent. I can rebase against 2.1.x if needed.
#11 is right, I received a bug report with the following error logged:
Error: Call to a member function first() on null in Drupal\address\Plugin\views\field\Subdivision->render() (line 91 of modules/contrib/address/src/Plugin/views/field/Subdivision.php).
Let's also account for that. I will update the MR.
This issue is also affecting contrib, e.g. Entity Reference Revisions: #3549453: Exception when cancelling user account → .
I did some very limited testing, but it looks like propagating the syncing state from the host entity to the referenced entity is enough to prevent the exception from being thrown.
In any case this is postponed until 🐛 Revision user incorrectly appears as anonymous user when node author is cancelled Needs work is in.
Addressed #127 and the remarks on the MR. Extended the test coverage to include a forward draft revision, which covers the change in ContentEntityBase.
Updated code to work with latest 11.x. There were conflicts due to 📌 Remove NodeHooks1 Active and 📌 Create a new NodeRepository for the code from Drupal\node\NodeStorage Active .
None of the remarks have been addressed. Now that 📌 Create a new NodeRepository for the code from Drupal\node\NodeStorage Active is in it is expected that there will be deprecation warnings.
pfrenssen → made their first commit to this issue’s fork.
The Webform module stores the default values in YAML format, so they are structured data. We can expose it as JSON encoded strings so the GraphQL API consumer can figure out what to do with it.
Indeed some types are not great, but passing what is stored is already a good start.
I couldn't find any token replacement happening with the default values. But I might have overlooked it.
Awesome improvements, thanks a lot for the contribution! Looking good to me, will merge when tests are green.
Thanks for this! A lot to unpack, assigning to me for review.
pfrenssen → made their first commit to this issue’s fork.
Very nice work! I removed a duplicate exposure of the `alignItems` property. This is ready to be merged. Thanks a lot for the contribution!
I aligned both competing branches, this is ready to go. Thanks a lot for the contribution!
Hey I saw you are working on this, but I am already done with the changes. I'm sorry it was not clear, I had assigned the issue to me to indicate I am taking care of it.
Thanks a lot for working on this! I have reviewed the code, it's looking great so far. I left some comments on the MR. We also need a test. I can take care of that.