🇫🇮Finland @mErilainen

mErilainen → created an issue.

mErilainen → created an issue.

Define the Conditions class as a Drupal service

- Transformers class will use Conditions in its constructor
- Change itemsToSchema() to non-static
- Call the apply() with $this->conditions->apply($schema, $items); in itemsToSchema()

mErilainen → created an issue.

And apparently that broke the rendering, so I will just fake the selection "All" as "One" which works with the if-then-else logic.

I had incorrect operator machine name, changed "one" to "xor". Also setting default value didn't work, so I removed that.

This will just affect the Drupal UI so that only "oneOf" operator can be selected. The if-then-else (with "not" in the mix) doesn't seem to work easily with "allOf" or anyOf" operators.
Marking as "Needs review" because it would be nice to know if this approach makes sense.

This changes the message to be an error displayed on the form element. See the attachment how it looks like.

mErilainen → made their first commit to this issue’s fork.

mErilainen → made their first commit to this issue’s fork.

The patch works, after giving the permission for anonymous user I can use it now as anonymous.

I'm not sure if there would be another way than setting the #cache max-age to 0, because it will kill the page cache and for example varnish will not work, making a site vulnerable for DDOS attacks.

Changing the patch author to Nathan B.

Apparently the empty default values just mess with form validation, so the first patch should be enough. Marking this RTBC.
I'm not sure how this should be done, so I'll just upload the first patch again in this comment.

This version will replace most of the conditions from if-then to if-not-else because the if-then else logic seems to require some default values in the schema for the elements. The if-not-else seems to work better and makes validation easier when there are no default values in the elements.

This patch takes more drastic approach and removed the use of the non-draft dependencies keyword completely. "Values is" trigger is also handled in the if-then-else structure which seems to work nicely too.
It also changes the getDependencyKey() function so that it will not throw an exception, because it seems to break Webform UI if there is a container element anywhere in a webform. This part needs still more work, because currently it allows to add any element into a condition and many of them do not work correctly.

Fix for "Notice: Only variable references should be returned by reference".

Yes the v0.8.4 requirement comes from another project and it will probably not work with the older v0.2.1. The problem is actually in the use composer caret ^ symbol, because:
"For pre-1.0 versions it also acts with safety in mind and treats ^0.3 as >=0.3.0 <0.4.0 and ^0.0.3 as >=0.0.3 <0.0.4"
https://getcomposer.org/doc/articles/versions.md#caret-version-range-
So I suppose it would be better to use the tilde ~ instead, so ~v0.2?
"Another way of looking at it is that using ~ specifies a minimum version, but allows the last digit specified to go up."
https://getcomposer.org/doc/articles/versions.md#tilde-version-range-
Here is a new patch with that suggestion.

This iteration of the patch adds support for multiple triggers per state and moves the handling of the "filled" trigger from the prepareDependencySchema() to the alterSchemaProperties() so that multiple "filled" triggers can be used per state.
The webformElementStatesAfterBuild() has not been touched yet because I could not get it working on my local.

It starts to feel like major refactoring could be in place, but there is still a lot of handling of the "Value is" trigger which I don't want to touch and might not need to be changed in the end.

Yet slightly improved version which uses if-not-then properties instead of if-else. Seems to work better without default values in the schema too.

Here is an improved version of the patch which also works with the "empty" trigger, at least to a certain degree.
Having the patch from ✨ Default value not available in response Needs review helps in some cases.

This patch sets the default value to empty string "" which works better with conditions and seems to also work with numeric fields.

Here is another patch which also sets the default value to "false" for checkbox elements when the default value is not set. This seems to work better with react-jsonschema-form and conditions.

Here is a patch which will add support for the checked trigger. It works with both "visible" and "required" states.
This approach alters the schema properties and uses the if & then properties instead of using the "dependencies" keyword which is not supported by the latest JSON Schema if I understood correctly.

mErilainen → created an issue.

I suddenly got an error while working on a test form:
TypeError: reset(): Argument #1 ($array) must be of type array, string given in reset() (line 254 of /app/drupal/web/modules/contrib/webform_jsonschema/src/Submission.php)
I updated the patch to check that the $dependencies is an array.

Here is a patch which will hopefully apply to 6.2.x.

I tried to apply the patch, but it would not apply to 6.2.2 nor 6.2.x-dev.

After applying the changes manually, I tested it with a filename which has spaces "test picture with spaces.png".
It worked if I don't select the "Sanitize file name" setting in the element configuration.
If I do select it, I cannot open the file from the results listing, instead I get the following error:
The requested URL "https://my-cms.lndo.site/system/files/webform/test%2520picture%2520with%2520spaces_0.png" was not found on this server.

Here is a patch.

mErilainen → created an issue.

Can you provide steps how to reproduce? I don't get any errors if I add checkboxes element to a webform.

Your patch can also generate a warning about "Undefined index" if the none of the keys in $row['selector']['#options'][$selector] are set.

Tested and works.

Here is a patch.

mErilainen → created an issue.

mErilainen → created an issue.

I don't understand the content of the merge request, it doesn't change the library version but instead adds a "self-dependency" to the module itself in composer.json?
https://git.drupalcode.org/project/openai_image_for_drupal/-/merge_reque...

Here is a patch to fix the situation. This will also upgrade the openai-php/client version to ^0.8.4.

mErilainen → created an issue.

mErilainen → created an issue.

There has been this issue #2992777: Implement a migration path → going on "forever".

Having trouble with multilingual sites and the proposed fix? This fix in Lando drupal10 recipe should work better:

# Fighting with Styles? This little gem is amazing.
# location ~ ^/sites/.*/files/imagecache/ { # For Drupal <= 6
location ~ ^(/[a-z\-]+)?/sites/.*/files/(css|js|styles)/ { # For Drupal >= 7
    try_files $uri @rewrite;
}

This module seems abandoned, last commit is over three years ago https://git.drupalcode.org/project/ckeditor_iframe/-/commits/8.x-2.x

There seems to be a Drupal 10 compatible release 2.1.0 https://www.drupal.org/project/link_class/releases/2.1.0 →
Can this be closed?

Looks good.

RTBC it is.

This is not right, right?

Note! If you do not want to share the configuration of the development modules, you can use the Drupal core functionality from version 10+ onward ...

Here https://www.drupal.org/node/3079028 → it says:

Up to Drupal 8.8.0 developers had to rely on contrib solutions such as Config Split ...

Drush 10 is required though.

Looks good to me.

Looks good to me, after patching Upgrade status saysNo problems found.
I will mark this as RTBC as the changes are simple replacements which Drupal Rector could do.

But does Drupal core check anything else than missing files defined by the configuration fast_404.paths: '/\.(?:txt|png|gif|jpe?g|css|js|ico|swf|flv|cgi|bat|pl|dll|exe|asp)$/i'?

From Fast 404 module:

* The second is to check whether or not the URL exists in Drupal by checking
* with the menu router, aliases and redirects. If the page does not exist, we
* will serve a Fast 404 error and exit.

Yep, I didn't check the code, I just saw the problem reported by upgrade_status, sorry. This makes it harder to see which projects are actually ready for D10 because I need to manually go through each problem reported by upgrade_status and see if they are false negatives or real problems.

mErilainen → created an issue.

The upgrade status module is still complaining about this in some places, but not all. A bug in https://www.drupal.org/project/upgrade_status → perhaps?

There is also a call to deprecated tmgmt_redirect_queue_dequeue() function in the same function.

Looks good.

Now the module seems to give an error when saving a provider:

The website encountered an unexpected error. Please try again later.
TypeError: in_array(): Argument #2 ($haystack) must be of type array, null given in in_array() (line 163 of modules/contrib/tmgmt_ilangl/src/ILangLTranslatorUi.php).

And the data looks like this:

in_array('ilangl', NULL) (Line: 163)
Drupal\tmgmt_ilangl\ILangLTranslatorUi->validateConfigurationForm(Array, Object) (Line: 237)
Drupal\tmgmt\Form\TranslatorForm->validateForm(Array, Object)

Seems that the values are saved to the Drupal state and it's possible to select the provider which the form fetches, so translations still work correctly.

This looks better now. Maybe it's a bit drastic change to move everything to Drupal state, because the only thing which might change in the remote is the service_id and services values. Now all other configuration needs to be added manually to each environment.

Looks good to me. This will drop Drupal 8 compatibility, but it is not supported anymore anyway.

What about problems like
Call to deprecated function file_create_url(). Deprecated in drupal:9.3.0 and is removed from drupal:10.0.0. Use the appropriate method on \Drupal\Core\File\FileUrlGeneratorInterface instead.

What about deprecated code? There are at least two problems which upgrade status reports:

Relying on entity queries to check access by default is deprecated in drupal:9.2.0 and an error will be thrown from drupal:10.0.0. Call \Drupal\Core\Entity\Query\QueryInterface::accessCheck() with TRUE or FALSE to specify whether access should be checked.

in SearchLinks.php and ExtraLinks.php

mErilainen → created an issue.

That works.

mErilainen → created an issue.