We need to port the feeds tamper trim plugin into a tamper plugin for Drupal 8.

CommentFileSizeAuthor
#7 interdiff-2960790-6-7.txt2.56 KBericgsmith
#7 tamper-trim-plugin-2960790-7.patch4.66 KBericgsmith
#6 interdiff_2-6.txt865 bytesjamesdixon
#6 tamper-trim-plugin-2960790-6.patch4.6 KBjamesdixon
#3 tamper-trim-plugin-2960790-2.patch4.57 KBjamesdixon
#2 tamper-trim-plugin-2960790-1.patch4.75 KBjamesdixon
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

jamesdixon created an issue. See original summary.

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Status: Active » Needs work
FileSize
tamper-trim-plugin-2960790-1.patch4.75 KB

So I took a crack at the trim plugin and tests. The trouble I'm having is when I try to run the tests, it's not generating the test xml in /sites/default/files/simpletest and I'm getting some error in the watchdog. I'm new to writing tests in D8.

I'm sure I've made a mistake in my code somewhere, or need to clear the testing cache or something. Any suggestions on getting around this would be greatly appreciated.

Notice: Undefined index: #error in Drupal\simpletest\Form\SimpletestResultsForm::addResultForm() (line 326 of /var/www/html/drupal/core/modules/simpletest/src/Form/SimpletestResultsForm.php) #0 /var/www/html/drupal/core/includes/bootstrap.inc(584): _drupal_error_handler_real(8, 'Undefined index...', '/var/www/html/d...', 326, Array) #1 /var/www/html/drupal/core/modules/simpletest/src/Form/SimpletestResultsForm.php(326): _drupal_error_handler(8, 'Undefined index...', '/var/www/html/d...', 326, Array) #2 /var/www/html/drupal/core/modules/simpletest/src/Form/SimpletestResultsForm.php(122): Drupal\simpletest\Form\SimpletestResultsForm::addResultForm(Array, Array, Object(Drupal\Core\StringTranslation\TranslationManager)) #3 [internal function]: Drupal\simpletest\Form\SimpletestResultsForm->buildForm(Array, Object(Drupal\Core\Form\FormState), '4') #4 /var/www/html/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(514): call_user_func_array(Array, Array) #5 /var/www/html/drupal/core/lib/Drupal/Core/Form/FormBuilder.php(271): Drupal\Core\Form\FormBuilder->retrieveForm('simpletest_resu...', Object(Drupal\Core\Form\FormState)) #6 /var/www/html/drupal/core/lib/Drupal/Core/Controller/FormController.php(74): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\simpletest\Form\SimpletestResultsForm), Object(Drupal\Core\Form\FormState)) #7 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch)) #8 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array) #9 /var/www/html/drupal/core/lib/Drupal/Core/Render/Renderer.php(582): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #10 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure)) #11 /var/www/html/drupal/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array) #12 [internal function]: Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}() #13 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(151): call_user_func_array(Object(Closure), Array) #14 /var/www/html/drupal/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1) #15 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #16 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #17 /var/www/html/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(99): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #18 /var/www/html/drupal/core/modules/page_cache/src/StackMiddleware/PageCache.php(78): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true) #19 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #20 /var/www/html/drupal/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(50): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #21 /var/www/html/drupal/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #22 /var/www/html/drupal/core/lib/Drupal/Core/DrupalKernel.php(664): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true) #23 /var/www/html/drupal/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request)) #24 {main}.
jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Assigned: jamesdixon » Unassigned
Status: Needs work » Needs review
FileSize
tamper-trim-plugin-2960790-2.patch4.57 KB

Sorry I have my environment setup where php errors aren't being logged to watchdog properly. Thanks for the help with this one megachriz.

Please find the revised patch which passes all 4 tests!

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Issue tags: +Nashville2018
Parent issue: » #2958013: [META] port remaining feeds tamper plugins
ericgsmith’s picture

ericgsmith CreditAttribution: ericgsmith as a volunteer commented
Status: Needs review » Needs work

Hi James,

Thanks for your contribution :) It's awesome to see the patches starting to roll in.

Just a couple minor issues for me then I think were good to go with this.

  1. +++ b/src/Plugin/Tamper/Trim.php
@@ -0,0 +1,99 @@
+ * Plugin implementation for removing text and whitespace from the beginning,

    Doc comment short description must be on a single line, further text should be a separate paragraph. 

  2. +++ b/src/Plugin/Tamper/Trim.php
@@ -0,0 +1,99 @@
+  const SETTING_CHARACTER = '';

    This should be a string value. This is the config key, so should reflect what the configuration is storing. It looks like maybe this is being confused with the default config. 

  3. +++ b/src/Plugin/Tamper/Trim.php
@@ -0,0 +1,99 @@
+  const SETTING_SIDE = 'trim';

    As above, something like 'side' would make more sense here. 

  4. +++ b/src/Plugin/Tamper/Trim.php
@@ -0,0 +1,99 @@
+

    Nitpick - theres an extra line here to remove. 

  5. +++ b/src/Plugin/Tamper/Trim.php
@@ -0,0 +1,99 @@
+    } else {

    Nitpick - else statements should be on a new line.

    The whole expression could also be simplified using a shorthand if statement.

jamesdixon’s picture

jamesdixon CreditAttribution: jamesdixon commented
Status: Needs work » Needs review
FileSize
tamper-trim-plugin-2960790-6.patch4.6 KB
interdiff_2-6.txt865 bytes

Thanks @ericgsmith!

I made all your suggestions besides using a shorthand if. Wasn't sure how to fit that all on one line within 80 chars, or how to format that properly if using shorthand with multiple lines.

ericgsmith’s picture

ericgsmith CreditAttribution: ericgsmith as a volunteer commented
FileSize
tamper-trim-plugin-2960790-7.patch4.66 KB
interdiff-2960790-6-7.txt2.56 KB

Thanks again James - just updating the test against the new base class.

ericgsmith’s picture

ericgsmith CreditAttribution: ericgsmith as a volunteer commented
Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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