- π¬π§United Kingdom james.williams
Iβ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.
- π§πͺBelgium 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.
-
james.williams β
committed 0b990a9e on 8.x-1.x
Issue #3175606: D10 compatible version. Fully transitioned to handler...
-
james.williams β
committed 0b990a9e on 8.x-1.x
-
james.williams β
committed c3ad8f4f on 8.x-1.x
Issue #3175606: Fixed links in emails. Token should only include the...
-
james.williams β
committed c3ad8f4f on 8.x-1.x
- Status changed to Fixed
9 months ago 2:23pm 4 March 2024 - Status changed to Needs work
9 months ago 2:43pm 4 March 2024 - π¬π§United Kingdom james.williams
Oh actually, to be fair, there are still some steps to complete from the original plan:
- Implement a proper access for the download route (what counts as proper access? I would quite like to 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
- Cleanup all of the old code
I've updated the issue summary.
-
james.williams β
committed 97c5cbf6 on 8.x-1.x
Issue #3175606: Fixed error handling, remove removed file usage records...
-
james.williams β
committed 97c5cbf6 on 8.x-1.x
-
james.williams β
committed 04f38598 on 8.x-1.x
Issue #3175606: Improve file extension setting experience. * Use...
-
james.williams β
committed 04f38598 on 8.x-1.x
- π§πͺBelgium redseujac
I 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]
-
james.williams β
committed 15f87098 on 8.x-1.x
Issue #3175606: Verify submission access/ownership for download links....
-
james.williams β
committed 15f87098 on 8.x-1.x
- π¬π§United Kingdom 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:<a href="[webform_submission:protected_download_url]">[webform_submission:protected_download_url]</a>
- Status changed to Fixed
8 months ago 2:39pm 5 March 2024 - π¬π§United Kingdom james.williams
Meanwhile, I've spun out the suggestion to allow a separate listing page of multiple downloads to β¨ Downloads page for multiple files Active , 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!
- π§πͺBelgium redseujac
I 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.
- π¬π§United Kingdom james.williams
Thanks, 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!
- π§πͺBelgium redseujac
I would prefer to avoid jumping ahead too quickly!
Of 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].
Now there is no option for the Token text title, although that was quite easy to use. Maybe a suggestion for a next version?
- π¬π§United Kingdom 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?
- π§πͺBelgium redseujac
No, 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.
- π¬π§United Kingdom james.williams
Yes, 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 exportingThis 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.
- π§πͺBelgium redseujac
I 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"
- πΊπΈUnited States devkinetic
@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:
- We should cut a new branch using the new semvar format, deprecating the 8.x- prefix. Work there from now on.
- The new work needs to validated through coder/phpstan/phpcs.
- The next release should comprise of the above, and the 8.x branch should be set as unsupported.
- π¬π§United Kingdom 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!
Automatically closed - issue fixed for 2 weeks with no activity.