Problem/Motivation
The initial D8 port is not built within the webform D8 handler format, it uses hook_insert and various other outdated patterns, there is a lot to do, but nothing is worth doing until we have the proper structure and files.
Proposed resolution
Port all of the functionality into a webform handler and remove the old config patterns
Remaining tasks
Add a storage handler for webform protected downloads entity
Define a webform handler
Port over the settings form
Port over the hook_insert into the handler
Port the token support into the handler
I think that the best way to do this is to only all one handler instance per form and then target the handler id that matches our type id. That should allow us to access the handler settings for that webform.
Implement a proper access for the download route. Add an option to verify access to the submission, e.g. so download links can't just be shared around?
Add in the ability to have a downloads page vs BinaryFileResponse if multiple files are added to the form - now covered by #3425782: Downloads page for multiple files
Sort out saving and deleting files from the handler. What happens to the entities related to that file? (i.e. need to clear out the usage when removing a file from the handler)
Cleanup all of the old code
User interface changes
The existing page will be removed and instead you will use the handlers section built into webform
API changes
There is currently no D8 for this, so this would actually be establishing one.
Data model changes
There is currently no data model for config, and that is what is changing. After this is complete, you will have to add handlers to your webform and reconfigure but only if you have used previous version of the dev module.
Comments
Comment #2
devkinetic commentedComment #3
devkinetic commentedComment #4
devkinetic commentedComment #5
devkinetic commentedComment #6
james.williamsI’m working on this for a client… I’ll be in touch with my work early next week! Would you be interested in sharing maintainership? Feel free to poke around through my profile at my previous contributions elsewhere. I’m new to this module but have been a maintainer of other projects.
Comment #7
redseujac@james.williams
Drupal 10 is out quite some time now (version 10.2.3), while Webform is on version 6.2.2 already.
Maybe the time has come to make this module work on Drupal 10 now so it can also be used in Webform 6.
Thanks and best regards.
Comment #10
james.williamsJust tagging an alpha release now :)
Comment #11
james.williamsOh actually, to be fair, there are still some steps to complete from the original plan:
I've updated the issue summary.
Comment #12
james.williamsComment #14
james.williamsComment #16
redseujacI have tried the alpha.
It works, but I have a problem with my confirmation message and the Protected download link webform submission token.
When a I complete the webform submission, in my message I do see the link in place of the token, but I cannot click it to download the protected file directly after. Clicking the link to download directly the file was possible in the previous version.
Maybe I'm missing something.
The syntax in the message looks like:
Click on: [webform_submission:protected_download_url]Comment #18
james.williams@redseujac Thanks for taking a look at the alpha so soon! I've added a few more commits to the 8.x-1.x dev branch since then, which are building up nicely towards a next alpha. It might be worth you working against the latest dev code.
I changed the tokens to only ever output the download URL, instead of actual linked text. This is primarily because the links in emails didn't work at all, so I believe it's better for the token to just be the URL. The token can then be put into a link. So for example, you might want to adjust your confirmation message to use
[webform_submission:protected_download_url]as the href attribute of a link, instead of being directly in text. If you've got the usual WYSIWYG enabled for the webform confirmation, that probably means selecting the text you wish to link, pressing the link button icon, and entering[webform_submission:protected_download_url]into the URL box of the popup dialog. Of course you could also make the clickable text use the token too, so the configured HTML would end up something like:Comment #19
james.williamsMeanwhile, I've spun out the suggestion to allow a separate listing page of multiple downloads to #3425782: Downloads page for multiple files, which was the last remaining point in the issue summary here.
So I'm going to tag a new alpha shortly and call this ticket complete!
@redseujac Please do continue to share any feedback & testing results you have from the latest module version. I'm using the new alpha in a client project, but hearing of other use cases & perspectives is very helpful, thanks!
Comment #20
redseujacI have installed the updated alpha 2.
As to now it's working properly. The link for downloading the protected file is OK as you explained in #19.
Thank you very much for both the new module (working well in Drupal 10 and most recent Webform) and the explanation for linking.
Maybe the alpha can become definitive soon.
Comment #21
james.williamsThanks, I really appreciate that feedback :-)
Having completed this little burst of work, I shall now let the alpha live for a while before tagging anything more stable/definitive. I do find often sitting back to give time for issues to be discovered or ideas to arrive is usually worthwhile. I would prefer to avoid jumping ahead too quickly!
Comment #22
redseujacOf course. I understand that perfectly.
Still one small thing about the link for downloading the protected file with the token included in the confirmation message.
In the "old" version there was no need to use HTML directly or throuhg the CKEditor link button for the Token text title. Indeed in the settings of the Weborm Protected Downloads module there was an option "Token text title" and the default was "Download file". So you just had to accept that or enter another text. Thus it was sufficient to enter the token in the confirmation message. For myself it looked like:
Click on: [webform_submission:protected_download_url].and for the user it looked like "Click on: Download file" where "Download file" contained the link to download automatically.Now there is no option for the Token text title, although that was quite easy to use. Maybe a suggestion for a next version?
Comment #23
james.williams@redseujac yes, I appreciate this new version is tricky to use. Do you use the tokens in confirmation emails? I found that the old approach produced completely broken links. That's why I changed the tokens to just simply be the URL without HTML, as they can always be embedded within HTML where necessary, whilst keeping plain links (for emails) working. Unfortunately I didn't see a way to identify when the output 'should' be HTML (like in a confirmation message) vs when it should not be (like in an email). A solution could be to have two different tokens, one for the HTML link, and another just to be the URL.
For what it's worth, I'm not convinced that setting the token text in the handler configuration was a good approach, because it's often worth having different wording for different contexts, whether that be in an email vs in an on-page message, or between different languages. The translatability of the old 'Token text title' was already using an unsupported approach as it was. Under the old version, did you ever deal with translating that text so it could appear different for different languages?
Comment #24
redseujacNo, I do not use the token in confirmation mails, only in the confirmation message. I repeat I did never use confirmation emails.
It's perfectly possible to have the text translated via the form settings > translation. I have indeed a website with two languages: Dutch and French.
Comment #25
james.williamsYes, translating the old token text setting was possible, but only as 'interface' translation, whereas this should use 'configuration' translation. This meant a few problems:
1) If you changed the original token text setting to something that hadn't been translated before, it would have to be translated again.
2) The text would always be translated to exactly the same string if it was translated for anywhere else in the interface, whereas Drupal usually segments translating these kinds of things. (That may well not have been an issue for your site/language(s), but it could be for those of others.)
3) Translations of it could not be reliably imported or auto-detected for exporting
This module now just leans on the existing translatability of webform's content settings for confirmation messages or emails rather than trying to make its own special case.
By the way, sorry about the multiple notifications you'll have received for the open issues on the old https://github.com/timlovrecic/Webform-Protected-Downloads repo; I'm trying to ensure potential users are signposted to this active version of the project! It looks like only the issue/repo owners can actually close those issues.
Comment #26
redseujacI do not insist for that "Token text title" option, because it's working well as it is now.
Just one observation on translation: actually - as it is now - I have still to add a translation for the Token text title I'm using in the HTML construction (as in #18).
In Dutch I have now in the Confirmation message:
Klikken op: <a href="[webform_submission:protected_download_url]">Bestand downloaden</a>.In English this means:
Click on: <a href="[webform_submission:protected_download_url]">Download file</a>..So I have to create a French translation for the string "Dowbload file" in the HTML construction.
This is done through "myWebForm" (name of my form) > Translate, where all the strings of my Webform can be translated in e.g. French in my case. See uploaded image "Confirmation_message_translation.png"
Comment #27
redseujacComment #28
devkinetic commented@james.williams Thanks for picking up the torch for this, glad my initial port is getting some love! This looks great so far. Couple of bigger picture things:
Comment #29
james.williams@devkinetic Glad to help ... especially because I had a client needing this ;-)
I don't believe there's any actual 'need' to move to the semver format, it's just a minor opportunity - but also a hurdle for the few that are already using a 8.x-1.x version. But OK, as & when there is change in the new branch that's worth making a release for, we can do so. I've pushed a 2.x branch, though I'm in no hurry to cut a new release from it. For now I've just pushed some minor improvements that phpstan flagged up. Some of what it flags up is unnecessary or going beyond Drupal's standards anyway; I'm reluctant to spend time implementing those. If you're interested in setting up CI stuff to automate checks from phpstan/phpcs/similar, do go ahead!