- Issue created by @redseujac
- ๐ง๐ชBelgium redseujac
OK, but there is still another issue:
modules/contrib/webform_protected_downloads/src/Plugin/WebformHandler/WebformPro
tectedDownloadsHandler.php:
+-------------+------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| status | line | message |
+-------------+------+-------------------------------------------------------------------------------------------------------------------------------------------------------------------+
| Now
fix | 257 | Call to deprecated function file_validate_extensions().
Deprecated in drupal:10.2.0 and is removed from
drupal:11.0.0. Use the 'file.validator' service instead. - ๐ฌ๐งUnited Kingdom james.williams
Sure, thanks - I wasn't rejecting any progress here by adding the related ticket! Contributions welcome :)
- ๐ง๐ชBelgium redseujac
I understand, but maybe this module could already be made D11 compatible without waiting for the Weborm update.
Indeed, I'm afraid that the latter could take some (considerable) time to complete.
- ๐ง๐ชBelgium redseujac
@james.williams
Could you please fix the issue indicated in #3 concerning the deprecated function file_validate_extensions()?
I am working actually with Drupal 11.0.1 and installed the Webform module 6.3.x-dev which I made compatible with D11.
I really would like to test the module in that environment, but I don't know how to fix WebformProtectedDownloadsHandler.php.
Could you provide me with a patch?
- Status changed to Needs review
2 months ago 5:38am 2 September 2024 - ๐ฎ๐ณIndia sarwan_verma
Hi,
I have resolved the issue with compatibility for Drupal 11, specifically replacing the file_validate_extensions function. I have also attached the patch.
Please review and verify.
Thank you!
- Status changed to Needs work
2 months ago 8:49am 2 September 2024 - ๐ฌ๐งUnited Kingdom james.williams
Thanks! I think the file.validator service was only introduced in 10.2; so we'd need changes before we can claim compatibility below that. Let's use
\Drupal\Component\Utility\DeprecationHelper
to resolve that please - here's an example which wraps a deprecated call tofile_icon_class()
: https://git.drupalcode.org/project/media_entity_download/-/blob/8.x-2.3/... Matt Glaman wrote a blog post explaining this too. - ๐ง๐ชBelgium redseujac
@sarwan-verma
Thanks for the patch.
I have applied the patch, but I get errors when working with te module.
I'm not able to upload and save a file. I'm trying to upload a .zip file. When I think the operation is finished andI click save, the file is not saved and I get an error message in /admin/reports/dblog lik this:
Drupal\Component\Plugin\Exception\PluginNotFoundException: The "file_validate_extensions" plugin does not exist. Valid plugin IDs for Drupal\Core\Validation\ConstraintManager are: Callback, Blank, NotBlank, Email, Choice, Image, BlockContentEntityChanged, CKEditor5Element, CKEditor5MediaAndFilterSettingsInSync, CKEditor5EnabledConfigurablePlugins, CKEditor5FundamentalCompatibility, SourceEditingPreventSelfXssConstraint, SourceEditingRedundantTags, StyleSensibleElement, CKEditor5ToolbarItemConditionsMet, CKEditor5ToolbarItem, CKEditor5ToolbarItemDependencyConstraint, UniqueLabelInList, CommentName, DateTimeFormat, FileExtension, FileExtensionSecure, FileImageDimensions, FileIsImage, FileNameLength, FileSizeLimit, FileUriUnique, FileValidation, LinkAccess, LinkExternalProtocols, LinkNotExistingInternal, LinkType, MenuTreeHierarchy, MenuSettings, PathAlias, TaxonomyHierarchy, ProtectedUserField, UserMailRequired, UserMailUnique, UserName, UserNameUnique, ContentTranslationSynchronizedFields, ConfigExists, LangcodeRequiredIfTranslatableValues, RequiredConfigDependencies, Bundle, EntityChanged, EntityHasField, EntityType, EntityUntranslatableFields, ImmutableProperties, ReferenceAccess, ValidReference, ExtensionExists, ExtensionName, UniquePathAlias, ValidPath, PluginExists, AllowedValues, ComplexData, Count, CountryCode, EntityBundleExists, FullyValidatable, Null, Length, NotNull, PrimitiveType, Range, Regex, UniqueField, Uuid, ValidKeys in Drupal\Core\Plugin\DefaultPluginManager->doGetDefinition() (line 53 of /data/sites/web/daproverbnbe/www/core/lib/Drupal/Component/Plugin/Discovery/DiscoveryTrait.php).
Any help is welcome.
- ๐ง๐ชBelgium redseujac
I took a closer look and I guess that the issue from my post #3 has not been fixed.
In the following snippet there is still a call to deprecated function file_validate_extensions().
$form['protected']['protected_file'] = [
'#name' => 'protected_file',
'#type' => 'managed_file',
'#title' => $this->t('Choose file for protected download'),
'#multiple' => FALSE,
'#upload_validators' => [
'file_validate_extensions' => [$form_state->getValue('protected_file_allowed_extensions', $this->getSetting('protected_file_allowed_extensions'))],
],
'#upload_location' => 'private://webform_protected_downloads/' . PlainTextOutput::renderFromHtml($this->replaceTokens('[date:custom:Y]-[date:custom:m]')),
'#default_value' => $this->getSetting('protected_file'),
];That's line 217.
- Status changed to Needs review
2 months ago 3:50pm 2 September 2024 - ๐ฌ๐งUnited Kingdom james.williams
I've opened MR !2 starting from the patch from comment 7. I've fixed some inconsistent whitespace, used the DeprecationHelper as I suggested earlier, and included the form element pointed out in comment 10. I haven't tested this at all yet, but it's the kind of change I would expect to work. I should get more time to test this out in the coming days, but feel free to try it in the meantime and let me know how you get on.
- ๐ง๐ชBelgium redseujac
I don't understand quite well.
You wrote in @12:
and included the form element pointed out in comment 10
Well, that snippet in comment 10 is NOT A FIX. It's an element containing a deprecated function file_validate_extensions(), that should be fixed as explained in my comment 3.
Obviously I am missing something. Maybe @sarwan-verma could help.
- ๐ฌ๐งUnited Kingdom james.williams
Well, that snippet in comment 10 is NOT A FIX. It's an element containing a deprecated function file_validate_extensions(), that should be fixed as explained in my comment 3.
Yes - I meant I've included a fix for that element which was highlighted in comment 10 - see the merge request: https://git.drupalcode.org/project/webform_protected_downloads/-/merge_r...
No dev version was released yesterday. The work is in the merge request. Merge requests are preferred over patches now.
- ๐ฌ๐งUnited Kingdom james.williams
I've just realised we also had some discussion around this in #3426444-10: Resolve issue reported by PHPStan and PHPCS โ , so just connecting these tickets.
- ๐ง๐ชBelgium redseujac
@james williams
I read your comment 14.I am really sorry, but I am not a developer and I don't understand the fix in the merge request.
Could you please provide me here with a simple patch as in comment 7, the way I can continue testing the module as a simple user?
- ๐ฌ๐งUnited Kingdom james.williams
@redseujac Sure - there are guidelines at https://www.drupal.org/docs/develop/git/using-gitlab-to-contribute-to-dr... โ about how to get hold of a patch file from a merge request. It's worth getting to grips with this process if you're used to using patches, as drupal.org is heading towards using merge requests for code changes instead of direct patches entirely.
Thank you for being willing to test this work in progress! Please understand that the work is in this form (which I recognise is developer-centric), because it's not yet completed testing. Hopefully the guidance in that link above helps bridge the gap for you as a non-developer?
- ๐ง๐ชBelgium redseujac
OK, I have read the guidelines and applied the patch from the merge request.
No luck.
When I try to add the handler to my form, I get the following warning and error in admin\reports\dblog
Warning: Undefined array key "#upload_validators" in Drupal\webform_protected_downloads\Plugin\WebformHandler\WebformProtectedDownloadsHandler->buildConfigurationForm() (line 224 of /data/sites/web/daproverbnbe/www/modules/contrib/webform_protected_downloads/src/Plugin/WebformHandler/WebformProtectedDownloadsHandler.php)
TypeError: Unsupported operand types: null + array in Drupal\webform_protected_downloads\Plugin\WebformHandler\WebformProtectedDownloadsHandler->buildConfigurationForm() (line 224 of /data/sites/web/daproverbnbe/www/modules/contrib/webform_protected_downloads/src/Plugin/WebformHandler/WebformProtectedDownloadsHandler.php).
Can you please fix this?
- ๐ฌ๐งUnited Kingdom james.williams
Thanks for the bug report! I've updated the merge request with what I believe to be a fix. I'm well aware this is still otherwise untested code. But you can get an updated patch for the updated merge request to test with again if you wish.
- ๐ฌ๐งUnited Kingdom james.williams
I've now completed my own testing, which did include some more necessary fixes. But I'm confident this is now fixed. Note though that there's a MR including this but also with further code quality improvements at https://git.drupalcode.org/project/webform_protected_downloads/-/merge_r... which combines the work done in ๐ Resolve issue reported by PHPStan and PHPCS Needs review with this. @redseujac would you be up for testing that?
- ๐ง๐ชBelgium redseujac
james.williams wrote in comment 20:
@redseujac would you be up for testing that?
I have tested the most recent MR build and I'm glad to tell you that it is working properly now in my environment D11.0.1 and the Webform module 6.3.x-dev which I made compatible with D11.
I am very pleased.
Thank you very much.
- ๐ง๐ชBelgium redseujac
I have tested more deeply today.
I have a problem.
I have deleted the uploaded file from within the handler.
When I trie to upload again it fails, with the message:
"Oops, something went wrong. Check your browser's developer console for more details."I looked in admin\reports\dblog and found nothing about it.
Maybe the upload runs out of time, I don't know.
I wiil stop now and test again tomorrow.
- ๐ฌ๐งUnited Kingdom james.williams
Ok, thanks for the report! That message indicates something could be logged in the browser's console; you can enable the developer tools and view its console tab to see this. https://www.coursera.support/s/article/learner-000001653-How-to-open-the... might help you view this. The message would only show up at the point of running into this error, so if you've navigated away, it won't still be there if you return to it. But maybe you can put the file back and try to repeat what you did earlier to see if it happens again?
- ๐ง๐ชBelgium redseujac
I have tried again and now it works. I have been able to upload the protected file again.
I guess the problem was caused by a local temporary slow internet connetion with a really slow upload speed.
Hopefully it keeps working so you could continue working towards a stable version release of this module.
- ๐ง๐ชBelgium redseujac
I have deleted the protected file again and was able to upload it again. So this is working :)
However one remark: after deleting the file via the handler, saving the handler, running Cron an flushing all caches, the protected file is still present in the protected private file location. Is this normal or is it a bug?
- ๐ฌ๐งUnited Kingdom james.williams
@redseujac Files would only ever be deleted on cron once they are older than the 'Delete temporary files after' setting on /admin/config/media/file-system - assuming that's even set to something other than 'Never'. Does that explain why your files are still around? Do they get deleted by cron after that amount of time has passed, which I guess it may now have?
- ๐ง๐ชBelgium redseujac
@james.williams
OK. I missed that one.
'Delete temporary files after' : 6 hours = setting on /admin/config/media/file-system
So I will test again and let you know the result.Is it possible to release a new dev version? Now in admin\reports\dblog I get each time a warning about my webform protected dowloads module. Not compatible.
- ๐ง๐ชBelgium redseujac
@redseujac wrote in comment 27:
OK. I missed that one.
'Delete temporary files after' : 6 hours = setting on /admin/config/media/file-system
So I will test again and let you know the result.Well the file didn't get removed after those 6 hours, though it is used in "0" places, but its status is marked as "permanent". I guess that's the reason. I don't know why that file isn't marked as "temporary".
So i have to remove the file manually.
- ๐ฌ๐งUnited Kingdom james.williams
@redseujac Please could you confirm which merge request you've tested please? Was it MR !2 from this ticket; or MR !3 from ๐ Resolve issue reported by PHPStan and PHPCS Needs review ?
I'm not in a rush to make a new release that includes this, because webform itself doesn't have a D11 compatible release yet either. I would rather give more time to hear confirmation from a wider set of people that each of these tickets' work is fine (especially from my co-maintainer in that other ticket). If we don't hear anything more by the time webform provides a D11 compatible release, I'll potentially make the release then though.
- ๐ง๐ชBelgium redseujac
james.williams commented in #29
@redseujac Please could you confirm which merge request you've tested please? Was it MR !2 from this ticket; or MR !3 from #3426444: Resolve issue reported by PHPStan and PHPCS ?
I'm not sure anymore: I think MR !3, which combined the two according to your comment in #20.
To be sure I will compare the code from MR !3 with the code I'm using actually. If there are differences I'll aplly the full modifications and improvements from MR !3 and let you know my findings.
However look at my comment in #21.
- ๐ง๐ชBelgium redseujac
@james.williams
I have uninstalled and removed the module and I installed and enabled it again with all the changes and improvements of MR !3.So actually it's certain that I'm testing MR !3.
So far I didn't have any issues.
In any case I will continue using the MR !3 untill you will release a stable version of the module.
- Status changed to Fixed
2 months ago 1:24pm 12 September 2024 - ๐ฌ๐งUnited Kingdom james.williams
Having given it a bit of time, and heard no reason not to, I've merged that MR so this is now fixed. I'll create a new D11-compatible release shortly.
- ๐ง๐ชBelgium redseujac
@james.williams
Cannot install alpha3 with composer because your root composer.json contains following:"require": { "drupal/core": "^9.4 || ^10 || ^11", "drupal/token": "^1.0", "drupal/webform": "^6.2", "php": ">=7.4" }
Better would be "drupal/webform": ">=6.2" for users like me that are using another (higher) version than webform ^6.2.
- ๐ฌ๐งUnited Kingdom james.williams
Hmm, thatโs strange - what versions of webfrom and core are you using please? The ^6.2 constraint should be respecting semantic versioning so should allow 6.3.*, for example.
I note that thereโs no official release of webform itself that is marked as compatible with Drupal 11 by the way. Are you sure thatโs not the problem? - ๐ง๐ชBelgium redseujac
You may be right.
I am using webform 6.3.x-dev and made it compatible with D11 by editing the webform.info.yml tike this:
core_version_requirement: ^10.3 // ^11
. So I added there ^11. - ๐ง๐ชBelgium redseujac
This is the error messag when I try to install alpha 3 with composer:
Your requirements could not be resolved to an installable set of packages. Problem 1 - drupal/webform_protected_downloads 1.0.0-alpha3 requires drupal/webform ^6.2 -> satisfiable by drupal/webform[6.2.0, ..., 6.2.7]. - drupal/webform_protected_downloads[1.0.0-alpha1, ..., 1.0.0-alpha2] require drupal/core ^9 || ^10 -> found drupal/core[9.0.0, ..., 9.5.11, 10.0.0, ..., 10.3.5] but the package is fixed to 11.0.4 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command. - drupal/webform[6.2.0, ..., 6.2.6] require drupal/core ^9.4 || ^10 -> found drupal/core[9.4.0, ..., 9.5.11, 10.0.0, ..., 10.3.5] but the package is fixed to 11.0.4 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command. - drupal/webform[6.2.5, ..., 6.2.7] require drupal/core ^10.1 -> found drupal/core[10.1.0, ..., 10.3.5] but the package is fixed to 11.0.4 (lock file version) by a partial update and that version does not match. Make sure you list it as an argument for the update command. - Root composer.json requires drupal/webform_protected_downloads ^1.0@alpha -> satisfiable by drupal/webform_protected_downloads[1.0.0-alpha1, 1.0.0-alpha2, 1.0.0-alpha3]. Use the option --with-all-dependencies (-W) to allow upgrades, downgrades and removals for packages currently locked to specific versions. Installation failed, reverting ./composer.json and ./composer.lock to their original content.
- ๐ฌ๐งUnited Kingdom james.williams
Yeah, so that message is basically saying: You're using webform_protected_downloads, which needs webform, which isn't compatible with Drupal 11. Composer doesn't read your files on disk for the compatibility data, it's taking that from drupal.org. So even if your local webform.info.yml says it's compatible with Drupal 11, that's only for Drupal's benefit, not composer's. If you want to use a Drupal 11 compatible version of webform, you need to inform composer how to do that. You can use https://github.com/mglaman/composer-drupal-lenient to let composer ignore the Drupal 11 compatibility issue, and then https://github.com/cweagans/composer-patches to add a patch to webform, such as one from the work in โจ Drupal 11 compatibility fixes for webform Needs work .
- ๐ง๐ชBelgium redseujac
I see. Thank you.
That's too complicated for me. I will install the new version of Website Protected Downloads manually, leaving composer alone :)
Automatically closed - issue fixed for 2 weeks with no activity.