Thanks all for this change! I think indeed this is the safest way to do it. There are 2 ways of interpreting the use of multiple plugins together and we should go for the conservative approach.
I started to work on this, but the code of this form is not only outdated, it doesn't work at all and probably hasn't worked for years. Even if it would work this feature does not seem to be very useful. There is a single button that will start a batch process to resave every single media entity. There is no explanation what is going to happen, no way to control which media entities are going to be resaved. It does not even have a confirmation form. And when the button is pressed nothing happens.
I did some digging and this was introduced in 2020 alongside the PDF Thumbnails feature. This makes me think this was probably just intended to run a single time and should have been an update hook instead.
The form is touched at least once per year during maintenance work (to fix coding standards etc) even though it doesn't work. I think this has no value at all and keeping it around will just keep wasting our time. And since nobody has complained about I guess nobody is using it.
I will rescope this issue to remove the form instead of updating it.
I missed that there is now a third client. Added the fix for this one also, sorry for keeping you busy!
pfrenssen → changed the visibility of the branch 3560221-rokka-metadata-entity-type to hidden.
pfrenssen → changed the visibility of the branch 3560221-the-rokka-metadata to hidden.
@medha kumari, I don't see how your MR is related to this issue, or if it is addressing anything at all, this is just some random change? Are you a bot?
Thanks for the suggestion, both MRs are updated.
Our dynamic use of third party settings doesn't seem to be possible to define in a config schema. The current implementation tries to do it like this:
'*.type.*.third_party.revision_manager':
type: mapping
label: 'Per-bundle revision manager plugin settings'
But this is not correctly parsed by the config schema resolver. But using something like node.type.*.third_party.revision_manager works fine.
This ticket seems to have been discussion the problem years ago, even before third party settings existed: 💬 How do you add schema for every config bundle entity type Closed: works as designed , but it was closed due to lack of movement.
For now, maybe we should just provide the schema for the core entity types that we support?
I updated the schema but it looks like the module is currently not storing the configuration values with correct types. Instead we are passing through whatever we get from the form state. For example the entity type enabled status should be a boolean but is stored as an integer 0 or 1. Also the values for plugin settings (age and amount) which are declared as integers are stored as strings.
We should make sure the types are correctly cast before storing them, and we probably need to add an update hook to correct the values in existing installations.
The module is still used in Group 3.x. It has been removed in 📌 Convert our access policies to use the Access Policy API Active but we will have to wait until Group 4.x is stable before we can remove it.
Opened a MR against 2.x. I did not test this out manually, the porting was quite straightforward.
In the meantime 3.x is out. I suggest we continue on the agreed path, let's document the current reality in the interface without breaking B/C.
Opening a second MR for 3.x.
pfrenssen → made their first commit to this issue’s fork.
Added the MR, I applied the patch from @chandraraj and fixed an unrelated coding standards error that popped up in the latest version of Coder. Setting back to RTBC because I did not make a change to the fix, just made it available as an MR.
Let's make a merge request so this can be merged by the maintainers.
pfrenssen → made their first commit to this issue’s fork.
I created a new MR because the previous one is outdated now that 📌 Abstract info-collection RedisController to deal with special environments Active is in.
Thanks to the improvements made in that ticket, the fix is now very trivial.
pfrenssen → changed the visibility of the branch 3173988-azureaws-take-into to hidden.
Reopening. I updated to Redis 8.0 on Upsun (Platform SH) and I am now getting this error. It seems this is a security recommendation from Redis when running in a hosted environment, to prevent customers from reconfiguring Redis (ref. https://redis.io/docs/latest/operate/oss_and_stack/management/security/#...).
Thanks for the response. I missed this filter when looking at the code, but this seems to filter out not only forward revisions, but also historic unpublished revisions. Certain content types can go through cycles of being published and unpublished and there can be a large number of old unpublished revisions.
Here is an example of a job posting on our website which is published when the job position is open, and then unpublished when the position is filled:
I think it would be probably a good idea to start with creating some tests with content that has gone through a more complex revision history with periods of publication, periods of unpublication, and a number of forward revisions. We can then assess how well the current plugins are handling these situations.
I see the MR has been simplified to just having an additional method to allow to delete images manually on Rokka. The delete hook is no longer there. That makes this quite safe to commit, we don't have to worry about users that might want to keep the images on Rokka after they are deleted on the Drupal side.
pfrenssen → made their first commit to this issue’s fork.
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.