Code looks good. Tested manually, it solves the problem. Thanks!
With the fix the test is passing! Ready for review :)
Started the MR with a failing test which indicates the problem: https://git.drupalcode.org/project/graphql_core_schema/-/jobs/8168101
There was 1 failure:
1) Drupal\Tests\graphql_core_schema\Kernel\CoreComposableConfig\GetBundleTypeNameTest::testGetBundleTypeName with data set "entity type without bundles" ('user', 'user', [[[true]]], 'User')
Failed asserting that two strings are equal.
--- Expected
+++ Actual
@@ @@
-'User'
+'UserUser'
Now let's add the fix.
pfrenssen → made their first commit to this issue’s fork.
Opened an issue in the upstream client library to add support for recent PHP versions: https://github.com/rokka-io/rokka-client-php/issues/82
PHPStan is failing due to Drupal 7 being EOL and the D7 migration being deprecated. This is handled in #3566967: Remove Drupal 6/7 migration path → .
It looks like these warnings are in the upstream project rokka/client. I did see we have a PHP 8.5 compatibility warning.
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?
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.