Problem/Motivation

The Pardot URL is not validated. Usually these URLs are copy/pasted and could have an space at the end, which will make the whole Pardot submission not work due to that.

Steps to reproduce

Add a Pardot handler and paste a URL with a space at the end. Then submit and run cron and you'll see that it fails.

Proposed resolution

Validate the URL but also fix the obvious without even warning the user, like trimming spaces from the input.

Remaining tasks

Provide a patch. Incoming :-)

Comments

fjgarlin created an issue. See original summary.

fjgarlin’s picture

Status: Active » Needs review
StatusFileSize
new867 bytes

Patch provided.

travis-bradbury’s picture

StatusFileSize
new9.04 KB

Thanks fjgarlin. I like it, but I thought it'd be nice to have a test before committing it.

This patch changes the error message from "URL is invalid." to "The Pardot post url must be a valid URL.", which I think gives a bit more direction about what to do about the problem, as opposed to just saying that there is a problem. It also adds tests, but they don't work yet.

I'm getting this:

There was 1 error:

1) Drupal\Tests\webform_pardot\Functional\ConfigurationTest::testAddHandler
Drupal\Component\Plugin\Exception\PluginNotFoundException: Plugin ID 'pardot_submission' was not found.

/app/web/core/lib/Drupal/Core/Plugin/DefaultLazyPluginCollection.php:79
/app/web/modules/contrib/webform/src/Plugin/WebformHandlerPluginCollection.php:35
/app/web/core/lib/Drupal/Component/Plugin/LazyPluginCollection.php:80
/app/web/modules/contrib/webform/src/Entity/Webform.php:2538
/app/web/modules/contrib/webform_pardot/tests/src/Functional/ConfigurationTest.php:82

which is referring to the getHandler part of

    /** @var \Drupal\webform\Entity\Webform $webform */
    $webform = Webform::load($webformId);
    $handler = $webform->getHandler($handlerPluginId);
    $configuration = $handler->getConfiguration();
    self::assertTrue(isset($configuration['pardot_url']) && $configuration['pardot_url'] === 'https://example.com/', 'Whitespace is trimmed from the Pardot URL.');

I'm not sure what its problem is. Maybe the plugins are discovered and cached prior to the new plugin getting installed?

Maybe we should just commit it without that assertion, but it'd be annoying to have all that and not include an assertion that the configuration value is transformed correctly.

Status: Needs review » Needs work

The last submitted patch, 3: 3204411-3.patch, failed testing. View results

travis-bradbury’s picture

Status: Needs work » Needs review
StatusFileSize
new8.99 KB

The tests failed for a different reason than what I described in #3: the name of a webform configuration doesn't match its ID. Not sure why that didn't happen to me locally, but this should fix, and leave us with the issue described by #3.

Status: Needs review » Needs work

The last submitted patch, 5: 3204411-5.patch, failed testing. View results

fjgarlin’s picture

Status: Needs work » Needs review
StatusFileSize
new4.09 KB

I gave it a go at testing the validation. I got these test to run, it's mostly based from your but extending the WebformBrowserTestBase class.

Patch attached.

  • fjgarlin authored 305f6f7 on 8.x-1.x
    Issue #3204411 by fjgarlin, tbradbury: Validate Pardot URL
    
travis-bradbury’s picture

Status: Needs review » Fixed

Awesome, thanks a lot fjgarlin.

Status: Fixed » Closed (fixed)

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