Problem/Motivation

The webform module gives some problems, especially if the user is anonymous.

Steps to reproduce

In a webform field use [current-user:email|current-page:query:url_param]. If is anonymous, it deletes the whole token.

CommentFileSizeAuthor
#2 3406685-support-for-webform.patch3.88 KBjmaties

Issue fork token_or-3406685

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

jmaties created an issue. See original summary.

jmaties’s picture

jmaties commented
StatusFileSize
new 3406685-support-for-webform.patch3.88 KB
jmaties’s picture

jmaties commented
jmaties’s picture

jmaties commented
Issue summary: View changes
jmaties’s picture

jmaties commented
Issue summary: View changes
jmaties’s picture

jmaties commented
Issue summary: View changes
socialnicheguru’s picture

socialnicheguru commented
Status: Active » Needs review
nikolay shapovalov’s picture

nikolay shapovalov commented
Status: Needs review » Needs work

Thanks for patch Javier, can you please convert it to MR?
To have lint check for your changes.

jmaties opened merge request !17

pianomansam’s picture

pianomansam commented

@jmaties thanks for your work on this. It looks like this is still in "Needs work" but I wanted to give some quick feedback.

First, it looks like you have duplicated the entire replace method. It would be great if this was more surgical, only making the changes necessary for this functionality to work, and then handing it off to the parent. The reason for this is that the underlying replace method in WebformTokenManager could change at any time. If/when it does, those changes won't be reflected in our version. Thus increasing maintenance and the chance something could break.

Second, to be accepted, we'll need tests added to your MR. Initially, we'll need tests that fail and demonstrate the issue. Then we'll need passing tests that demonstrate it is resolved.

jmaties’s picture

jmaties commented

The problem occurs not only with the user annonymous, but also with any parameter.

I tried [current-page:query:param1|current-page:query:param2] and if the first parameter is false it fails.

jmaties’s picture

jmaties commented

I forgot it was a submodule :)

jmaties’s picture

jmaties commented
Status: Needs work » Needs review
pianomansam’s picture

pianomansam commented
Status: Needs review » Needs work

While I see tests for the submodule, it doesn't look like they are being run by the continuous integration.

jmaties’s picture

jmaties commented

I don't know why this happens...

pianomansam’s picture

pianomansam commented

@jmaties While I'm trying to get the testing rig working with the submodule, it looks like TokenOrWebformFunctionalTestBase is throwing a deprecation error in Drupal 9:

Calling Drupal\Tests\WebAssert::elementTextContains with more than three arguments is deprecated in drupal:9.1.0 and will throw an \InvalidArgumentException in drupal:10.0.0. See https://www.drupal.org/node/3162537
1x in TokenOrWebformFunctionalTestBase::testGetFirstParam from Drupal\Tests\token_or_webform\Functional
1x in TokenOrWebformFunctionalTestBase::testNotGetParam from Drupal\Tests\token_or_webform\Functional

jmaties’s picture

jmaties commented

@pianomansam thank's, done!!

pianomansam’s picture

pianomansam commented
Status: Needs work » Fixed

I got tests to work, so I expanded them to make sure they handled both current-user and current-page tokens. Sure enough, they caught an issue with current-user tokens. That is now resolved and I'm content with the amount of test coverage, so I'm merging this into dev.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.