Hi,
I added tamper plugin via web interface, but could not delete it - I click button delete, but plugin is not being removed. Let me know if I need to provide more details.
thanks, Irina

CommentFileSizeAuthor
#8 interdiff-2965816-58-8.txt647 bytesmegachriz
#8 feeds_tamper-delete-tamper-plugin-from-ui-2965816-8.patch1.33 KBmegachriz
#5 feeds-tamper-delete-tamper-plugin-from-ui-2965816-5.patch711 bytesericgsmith

Comments

irinaz created an issue. See original summary.

irinaz’s picture

irinaz commented

I run the following code but still cannot remove plugin

$feed_type = \Drupal\feeds\Entity\FeedType::load('article_import');

// Load a feed type.
/** @var \Drupal\feeds\FeedTypeInterface */
$feed_type = \Drupal\feeds\Entity\FeedType::load('article_import');

// Get Tamper functionality for this feed type.
/** @var \Drupal\feeds_tamper\FeedTypeTamperMetaInterface */
$tamper_meta = \Drupal::service('feeds_tamper.feed_type_tamper_manager')->getTamperMeta($feed_type);

// Example: get a single Tamper instance.
/** @var \Drupal\tamper\TamperInterface */
$uuid = "6c41bf13-a907-451b-87d8-fa4d465f628a";

$tamper = $tamper_meta->getTamper($uuid);

// Example: remove a tamper instance.
$tamper_meta->removeTamper($tamper);

megachriz’s picture

megachriz
Primary language Dutch
commented

I noticed that removing Tamper plugins is broken indeed while I was writing UI tests for Feeds Tamper in #2949203: Add UI tests.

I plan to look at this after #2949203: Add UI tests and #2975427: Add config schema for each Tamper plugin are done.

megachriz’s picture

megachriz
Primary language Dutch
commented
Priority: Normal » Major
ericgsmith’s picture

ericgsmith commented
Status: Active » Needs review
StatusFileSize
new feeds-tamper-delete-tamper-plugin-from-ui-2965816-5.patch711 bytes

Looks like the form was passing in the whole tamper plugin rather than the instance id.

I've corrected the calling code here however I wonder if it is also worth renaming removeTamper to be removeTamperInstanceId to provide more clarity? Alternatively we could rename and then also re implement removeTamper to accept a plugin instance rather than the uuid.

irinaz’s picture

irinaz commented

I tested patch, works like a charm, thank you!!

keshavv’s picture

keshavv commented
Status: Needs review » Reviewed & tested by the community

Working perfectly . Ready for RTBC

megachriz’s picture

megachriz
Primary language Dutch
commented
StatusFileSize
new feeds_tamper-delete-tamper-plugin-from-ui-2965816-8.patch1.33 KB
new interdiff-2965816-58-8.txt647 bytes

Thanks for the patch, Eric!

In #2949203: Add UI tests I added a functional test for removing a Tamper plugin. I marked the test as skipped there because during writing tests I also encountered this bug. Let's see if I wrote that test well by removing the "markTestIncomplete" mark.

  • MegaChriz committed e483175 on 8.x-2.x authored by ericgsmith 
    Issue #2965816 by MegaChriz, ericgsmith: Cannot delete tamper plugin via...
megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Reviewed & tested by the community » Fixed

Guess I wrote that test well. It passed! Committed #8.

Status: Fixed » Closed (fixed)

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