[PLAN] 8.x-1.0 Release

Created on 7 October 2020, about 4 years ago
Updated 21 March 2024, 8 months ago

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

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.

🌱 Plan
Status

Fixed

Version

2.0

Component

Code

Created by

πŸ‡ΊπŸ‡ΈUnited States devkinetic

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡¬πŸ‡§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.

  • Status changed to Fixed 9 months ago
  • πŸ‡¬πŸ‡§United Kingdom james.williams

    Just tagging an alpha release now :)

  • Status changed to Needs work 9 months ago
  • πŸ‡¬πŸ‡§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.

  • πŸ‡¬πŸ‡§United Kingdom james.williams
  • πŸ‡¬πŸ‡§United Kingdom james.williams
  • πŸ‡§πŸ‡ͺ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]

  • πŸ‡¬πŸ‡§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
  • πŸ‡¬πŸ‡§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 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.

  • πŸ‡§πŸ‡ͺ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.

Production build 0.71.5 2024