- Issue created by @jmaties
- Status changed to Needs review
11 months ago 11:03am 7 December 2023 - Status changed to Needs work
11 months ago 11:42am 7 December 2023 - π·πΊRussia zniki.ru
Thanks for patch Javier, can you please convert it to MR?
To have lint check for your changes. - πΊπΈUnited States pianomansam
@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 underlyingreplace
method inWebformTokenManager
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.
- πͺπΈSpain jmaties
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.
- Status changed to Needs review
11 months ago 6:59pm 9 December 2023 - Status changed to Needs work
11 months ago 3:54pm 11 December 2023 - πΊπΈUnited States pianomansam
While I see tests for the submodule, it doesn't look like they are being run by the continuous integration.
- πΊπΈUnited States pianomansam
@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 -
pianomansam β
committed a55eee87 on 2.x authored by
jmaties β
Issue #3406685 by jmaties, pianomansam: Support for webform
-
pianomansam β
committed a55eee87 on 2.x authored by
jmaties β
- Status changed to Fixed
11 months ago 5:27pm 12 December 2023 - πΊπΈUnited States pianomansam
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.
Automatically closed - issue fixed for 2 weeks with no activity.