Foundation: Feeds migrate Processors/Tamper UI

Given the new UI direction we're headed in https://www.drupal.org/project/feeds_migrate/issues/3002361 I would propose the following process plugin configuration UX:

Summary on mapping overview:

Per mapping show/edit all process plugins (i.e. tampers):

https://www.drupal.org/project/entity_import has a working mapping interface which supports a number of process plugins. Use this for inspiration.

I've worked on connecting Tamper with Migrate process plugins: https://github.com/Etroid/feeds_migrate/pull/11

This is without the UI, currently.

I've updated the issue summary with instructions so an other developer who knows his/her way around in the form API is able to work on this issue.

I will take a stab at this at BADCamp.

I am updating the "getting started" steps:

• We have to create the migration group
• I think we want to enable the Feeds migrate UI module

From what I gather work is moving ahead on these features here:

https://github.com/Etroid/feeds_migrate/commits/process

Tests are not passing on this branch at the moment it seems because of differences between the d.org testbot and the github test bot and their composer syntax.

Martin made adding/removing multiple process plugins work. After his work, the Tamper integration needed to be updated and that's what I did today.

Details of the changes:

• The form logic for the derived Tamper process plugin is moved to a MigrateForm plugin called 'TamperForm'. This class designates all form related calls to the Tamper plugin.
• Implemented reading source definitions from the Migrate entity. This ensures that the Tamper Rewrite plugin can display replacement patterns.
• Added support for Tamper categories: the list of process plugins are now displayed in categories. For Tamper plugins, this makes the right plugin to apply easier to find.

Saving the configuration does not work yet. I get the following error when I try to save the process plugin configuration:

Call to undefined method Drupal\feeds_migrate\Plugin\feeds_migrate\mapping_field\DefaultFieldForm::loadMigratePlugin() in Drupal\feeds_migrate\MappingFieldFormBase->submitConfigurationForm()

I haven't looked at the details of this error because @benjifisher might already be working on it. He reported on Slack that he might work on this issue this weekend.

With the current code in the process branch, I believe adding a migration is broken. Uploading patch with all the changes in here to run tests.

This could fix some of the test failures and also fixes some of the coding standard issues. I will commit it later to the process branch.

Changes from #17 are added to the process branch.

Worked a little on fixing the tests that are failing. They are still failing, but that's because saving new mapping (even without configuring process plugins) doesn't work correctly anymore.

The methods that the test \Drupal\Tests\feeds_migrate\Kernel\MigrationMappingForm is testing no longer exist. So I think this kernel test should be completely revised.

A week ago I tried to investigate why tests are failing. I reported about it on Slack:

I’ve looked at fixing the ‘add/edit mapping form’ in the process branch, but that seemed to be quite complicated.

megachriz	I think I could use @Martin K ‘s help to fix that.
megachriz	I noticed the mapping edit form doesn’t correctly load in the currently configured mapping: source field remains empty. The 8.x-1.x branch doesn’t have this problem.
megachriz	And when saving new mapping (not configuring process plugins), the selected target field is saved as an empty string instead, causing the mapping overview page to crash with a fatal error.
megachriz	Some information seems to get lost when on the mapping add/edit form a subform for the selected target field gets loaded.
jamesdixon	This is for feeds migrate?
megachriz	@jamesdixon Yes.
jamesdixon	I've only tested adding while QAing the MVP list which explains why I have not run into this
megachriz	I think I took 1.5 hours to look at mapping add/edit form this week, because it is broken in the process branch. The tests also fail on this. I could only note the difference in outcome between the branches 8.x-1.x and process.
jamesdixon	Good to know. Thanks for your work on this.
megachriz	And I suspect that it has something to do with the mapping form not passing enough information to the subform. The subform in this context is the form where you type in a source (and possibly select process plugins), while the main form has a selector for the field you want to map to. (edited)
megachriz	The subform can be different per field type. For example, for the ‘text with summary’ field type (usually called ‘body’), you’ve multiple sources to type in: one for summary, one for value and one for format.
jamesdixon	Ah okay so sources are a little more complex than a static field you're grabbing from
megachriz	The sources are text fields here. But there can exist multiple if a field has multiple properties.
irinaz	Hi, sorry I could not make it at 10 am today :disappointed: I am reading through topics, it is great.
irinaz	@megachriz how this issue of different subforms is handled in feeds?
megachriz	@irinaz In Feeds, the mapping UI is a lot different from the mapping UI from Feeds Migrate. See screenshot for which part of the page is the main form and which is the subform:
megachriz	So the main form has the selector for destination field, while the subform has a text field for 'source' and the UI for process plugins.
megachriz	Issues in the process branch:Existing mapping configuration is not loaded correctly on the form. Source field remains empty.When saving the mapping form, the selected destination field is not saved. Instead an empty string gets saved for this setting.
megachriz	The reason the source field and process plugins are in a subform, is because the form could be different per field type. For example, the body field needs three source fields: One for 'summary', one for 'value' and one for 'format'. And for 'format' we later want to provide a selector instead of a text field.

I feel I'm getting into circles trying to make progress on the issues/bugs introduced in the process branch on github. Because the amount of code changes that were made is getting huge it's hard to find where in the code a mistake was made and what is 'work in progress' code, meaning code that is part of redefining the structure.

I'll see if it helps if I'll try to break the changes into smaller chunks. Because if a thing/task gets big and beyond manageable, breaking it down could help.

Here is a patch that only fixes code style and comments. That's a start.

I'm only not sure about how we could create more clarity in the process branch on github if we would merge 8.x-1.x back into it after committing this. I often get confused on which code changes happened where when merging branches.

I found a few more changes that are only code/comment style fixes.

When glancing through the code I see that changes are made to the following things:

1. Migration add/edit form.
2. The factory for generating forms for migrate plugins.
3. Migration mapping form (the page that displays all mappings).
4. Migration mapping add/edit form.
5. Migration mapping delete form.
6. Tamper integration (that is what I added).
7. File upload form.
8. Migration helper service.

To me, it looks like not all changes are directly related to adding support for process plugins. I know that at least #4 and #6 are directly related, but not so sure about the rest.
So after committing the code/comment style fixes I found so far, I think it would be good to concentrate on changes made to the form factory (#2) because most of the remaining changes to the migration add/edit form (#1) seem to be directly related to #2. The only other change from #1 seems to be the "save and add mapping" feature, also already being worked on in #3063092: When migration is created change button to "save and add mapping".

And after #1/#2 are finalized, we can move on to other parts. With each part finalized, the amount of code changes coming from the process branch becomes smaller, making it easier for me to see where code might be wrong or incomplete.

Thanks for breaking it down @megachriz.

The work so far looks good. I like the direction you're taking with this.

@Megachriz mentioned https://www.drupal.org/project/feeds_migrate/issues/3063073#comment-1341... conflicts with this issue. I'm not sure if we should commit 3063073 or leave it until this has progressed further.

Committed #23, issue remains open. Since @Etroid made the majority of these changes, the commit is attributed to him.

@irinaz
What's currently in the dev release doesn't require any specific testing, because it only made changes to code style and comments. Next patches will likely make functional changes. For each of these patches that passes all tests, it would good to go through whole process of creating a migration and perform an import. Basically, take the steps taken on the demo I recorded. Because I'm not sure if the steps taken on the demo have full test coverage yet.

This patch ports the following commit from the process branch: https://github.com/Etroid/feeds_migrate/commit/35bc4ef001f80c90370f81785...
And adds changes from related commits.

Update form factory to reduce number of required arguments.
- We try and retrieve the form plugin type dynamically from the migrate plugin definition or class namespace.

Hopefully I found all the changes from related commits to keep tests passing.

It's a small change, but each small step makes the amount of code to review from the process branch smaller.

Okay, the changes to form factory also requires changes to MigrateFormPluginBase and MigrateFormPluginInterface. Let's add these changes to the patch as well, except for the method getSummary() which looks like that it won't be required for the form factory changes.

From @MegaChriz in slack feeds meeting:

It would be good to manual test this patch by going through the whole process of creating a migration and perform an import, because this process may not have full test coverage yet.

Discussion on Slack today for this issue:

Process plugin UI

Participants:

megachriz, jamesdixon

Tagging issue for Global Contrib Weekend Event at SFDUG. Thursday Jan 23.

Tagging issue for Global Contrib Weekend Event at SFDUG. Thursday Jan 23.

Thanks for reviewing, I committed #31.

Next step is to sort out one of the following:

1. Migration mapping form (the page that displays all mappings).
2. Migration mapping delete form.
3. Migration helper service.

I'll need to take a closer look at the code to see which of these make sense to handle next.

Here is also an update of the code that's left from the process branch, meant to be ported further in bits in this issue.

I see that in the process branch, the service "feeds_migrate.migration_entity_helper" (MigrationEntityHelperManager with MigrationEntityHelper) is replaced with "feeds_migrate.migration_helper" (MigrationHelper). Apparently this is done to simplify things.

I see that MigrationHelper::getMappings() doesn't return exactly the same as MigrationEntityHelper::getMappings(). Probably because of changes elsewhere. I also detected a serious issue (bug) in it: MigrationHelper::getMappings() initializes MigrationHelper::$mappings the first time when passing a migration. But the next time it returns MigrationHelper::$mappings, regardless of the migration that is passed to the method. So if you would want to get the mappings for two different migrations, you will only get the mappings from the first migration.

So that leaves me to think that the design of MigrationHelper isn't optimal. I wonder why MigrationHelper had to replace MigrationEntityHelper? Maybe because MigrationEntityHelperManager requires a migration ID so it doesn't work for unsaved migrations? I'm thinking: what if we override the Migration class from Migrate Plus with our own, and add methods from MigrationHelper on that? I'll open a separate issue to experiment with that.

Okay I opened #3108353: Override Migration class from Migrate Plus (and use that instead of MigrationEntityHelper) to experiment with the idea I talked about in #39.

It will make merging changes from the process branch harder, though. Code will be getting more diverge.

I rerolled the patch from #38 after committing #3108353: Override Migration class from Migrate Plus (and use that instead of MigrationEntityHelper). Oof!

Needs a lot of work because we will be moving away from MigrationHelper now and use Drupal\feeds_migrate\Entity\Migration instead. But I figured starting with a reroll of what's left from the process branch was the first thing to do.

I opened a new spin-off issue for this one: #3126936: Port mapping methods to \Drupal\feeds_migrate\Entity\Migration.

An interesting thing that I found is that MigrationHelper::saveMappings() is equal to MigrationHelper::deleteMappings(). So I suspect that causes at least part of the issues that the code in the process branch introduced.

Opened another spin-off issue: #3132505: Add unit tests for MappingFieldFormBase and MappingFieldFormManager.

Here is a patch where the process plugin UI is somewhat working!

The patch includes all the work done in #3132505: Add unit tests for MappingFieldFormBase and MappingFieldFormManager, so the patch is quite big!

There are a few issues left to tackle:

1. For Tamper plugins, the configurations are not saved.
2. Validating form input for process plugins is not yet implemented.
3. Deleting a mapper does not always work, specifically if the mapping is saved as "foo/bar" instead of "foo".
4. For the Body field funny things happen when trying to add process plugins for them: plugins added for "body/summary" are added to "body/value" instead. Though after saving, they do get listed under "body/summary".
5. When emptying mapping for a subfield - for example "body/format" - the mapping does not get removed on save.

Needs more (automated) tests to tackle these issues.

Changes:

• Fixed saving configurations for Tamper plugins.
• Removed Kernel test for MigrationMappingForm class since the mapping form changed a lot and I figure form tests could be better implemented as a functional test.
• Added $defaultTheme to the functional test classes.
• Hopefully fixed the test \Drupal\Tests\feeds_migrate\FunctionalJavascript\Form\MigrationMappingFormTest.
• Some coding standard fixes.

Not fixed yet:

1. Validating form input for process plugins is not yet implemented.
2. Deleting a mapper does not always work, specifically if the mapping is saved as "foo/bar" instead of "foo".
3. For the Body field funny things happen when trying to add process plugins for them: plugins added for "body/summary" are added to "body/value" instead. Though after saving, they do get listed under "body/summary".
4. When emptying mapping for a subfield - for example "body/format" - the mapping does not get removed on save.

In this patch I fixed the following things:

• Implemented form validation for process plugins.
• Fixed an issue where process plugin configuration could get duplicated on a second save.

I updated the issue summary to reflect the remaining tasks.

Richard will be testing this.

@Richard: Can you please test to see if you're able to submit the mapping form without filling in all required fields (ie: name or label)?

If there are any bugs please document them here so we know what steps to take to cause the bug.

@jamesdixon

Looks like you can indeed submit the mapping form without the required fields.

Thanks @barnes1999. Which fields are required that can be filled out? If you list them here we can work towards adding validation.

Hi @jamesdixon

Apologies for my vague update. It appears the only field that's required to complete the step of adding the mapping itself is its destination. When I try to submit it with the other fields totally empty, Drupal will give the message "Migration mapping for field (field) has been updated," but no mapping will actually be created.

Entering a value for at least one of the fields results in the mapping being created and it showing up in the UI, and the mapping will remain how it is when you first created it even if you try to empty each of its fields, even if Drupal is still saying "Migration mapping for field (field) has been updated." I imagine something that could be done would be some kind of error message that says "No values for mapping entered, mapping item was not created." Does that answer your question?

@barnes1999
Thanks for finding that bug! I added a test for it in the attached patch which is called MigrationMappingFormTest::testAddMappingWithoutSource().

I also added tests for the other identified bugs:

• Adding process plugins for multiple properties of a field (body field) doesn't work correctly (process plugins are not added or are added to the wrong property).
• Removing mapping for a single property for a multiproperty field does not remove the mapping for that property.
• Adding and removing mapping for custom destination field results into errors and mapping doesn't get removed.

There is only a single functional change and that is that it is now possible to add mapping without having javascript enabled. This isn't only a nice add-on, but it allows me to write functional tests that run faster (functional javascript tests take quite a lot more time to run).

I also occasionally got errors when reordering mappings, but I have yet to identify the steps to reproduce that.

@megachriz

Just ran the test you described after applying the patch. The test results in 3 errors:

PHPunit Test failed to complete; Error: PHPUnit 7.5.20 by Sebastian Bergmann and contributors.

Testing Drupal\Tests\feeds_migrate\FunctionalJavascript\Form\MigrationMappingFormTest EEE 3 / 3 (100%)

Time: 22.14 seconds, Memory: 8.00 MB

There were 3 errors:

1) Drupal\Tests\feeds_migrate\FunctionalJavascript\Form\MigrationMappingFormTest::testAddMigrationMapping
chmod(): Operation not permitted

/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:165
/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:115
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:572
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:406
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/FeedsMigrateJavascriptTestBase.php:48
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/Form/MigrationMappingFormTest.php:30

2) Drupal\Tests\feeds_migrate\FunctionalJavascript\Form\MigrationMappingFormTest::testEditMigrationMapping
chmod(): Operation not permitted

/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:165
/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:115
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:572
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:406
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/FeedsMigrateJavascriptTestBase.php:48
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/Form/MigrationMappingFormTest.php:30

3) Drupal\Tests\feeds_migrate\FunctionalJavascript\Form\MigrationMappingFormTest::testDeleteMigrationMapping
chmod(): Operation not permitted

/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:165
/mnt/c/Users/richa/sites/example.localhost/web/core/lib/Drupal/Core/Test/FunctionalTestSetupTrait.php:115
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:572
/mnt/c/Users/richa/sites/example.localhost/web/core/tests/Drupal/Tests/BrowserTestBase.php:406
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/FeedsMigrateJavascriptTestBase.php:48
/mnt/c/Users/richa/sites/example.localhost/web/modules/contrib/feeds_migrate/tests/src/FunctionalJavascript/Form/MigrationMappingFormTest.php:30

ERRORS!
Tests: 3, Assertions: 0, Errors: 3.

Is this the result we should expect or is something up with my instance?

I've tried fixing the issue of "emptying mapping for a subfield". For example when mapping to the body field, you may want to remove mapping for the property "format".

I ended up changing a lot of code:

• Mapping configuration for custom destinations is now mostly structured the same as field destinations. All custom destinations will have a property called "value". This could have some unwanted side effects: for example during testing custom destinations were sometimes saved as "foo/value" instead of "foo".
• The configuration structure for MappingFieldForm plugins changed. It was something like this:
  

  [
    'destination' => ['key' => 'body'],
    'value' => [...],
    'summary' => [...],
  ];

  It now is:

  [
    'destination_key' => 'body',
    'mapping' => [
      'value' => [...],
      'summary' => [...],
    ],
  ];

  I wanted to be able to loop through properties. And I noticed each time there was check if $property != 'destination'. So I moved all properties to an array called 'mapping'. I hope I adjusted the code at all places for this structure.

Since I changed a lot, I won't be surprised if I broke some other stuff. So it could be that the patch in #57 performs better and that this one introduces new errors. Let's see what all the tests have to say.

I focussed on fixing the test failures from Drupal\Tests\feeds_migrate\Functional\Form\MigrationMappingFormTest.

Changes:

• A warning is now displayed when adding or editing mapping and do not configure a source nor process plugins for any property. This issue was reported by @barnes1999 in #56.
• Fixed an issue with adding process plugins for multiple properties of a field with JS disabled. Previously, when you added a process plugin for one property and then added one for another property, the first added process plugin got removed. This one was challenging to fix. I added the methods getPropertyState() and setPropertyState() for this to temporary store on the form state which process plugins were added to which property. This made the test Drupal\Tests\feeds_migrate\Functional\Form\MigrationMappingFormTest::testAddMigrationMappingWithProcessPluginForBodyField() pass.
• Fixed an issue where emptying config for one property could result into two process lines getting saved for the same field. For example: process configuration for the body field got saved as both "body" and "body/value". Now it gets saved as "body".
• Fixed most coding standard issues (I hope).

Let's see if there are still failing tests now.

Note: I sometimes still notice an issue when reordering mappings. Sometimes I get directed to a non-existing page afterwards. The path then equals one of the field names that I mapped to.

Well, I quickly found out now what caused that mapping reordering issue. After saving mapping, you got redirected to the mapping overview form, but for some reason with a destination query parameter in the url. For example:
/admin/structure/migrate/sources/xml_migration_new/mapping?destination=body
Which resulted into getting redirected to /body after reordering mappings.

I removed the destination query parameter in MigrationMappingFormBase::save().

Also fixed a few more coding standard issues.

I think this patch is ready for review and testing.

Reroll, needed after committing #3147461-14: Automated Drupal 9 compatibility fixes.

I've applied patch #62 and tried to access the mapping tab in a Drupal 8 installation I was testing successfully in before and got the white screen in return with the following log entry:

location: http://d8.ddev.site/admin/structure/migrate/sources/xml_migration/mapping
referer: http://d8.ddev.site/admin/structure/migrate/manage/a/migrations/xml_migration

TypeError: Argument 6 passed to Drupal\feeds_migrate\MappingFieldFormBase::__construct() must be an instance of Drupal\feeds_migrate\Plugin\MigrateFormProcessPluginManagerInterface, instance of Drupal\feeds_migrate\Plugin\MigrateFormPluginManager given, called in /var/www/html/web/modules/contrib/feeds_migrate/src/MappingFieldFormBase.php on line 99 in Drupal\feeds_migrate\MappingFieldFormBase->__construct() (line 78 of /var/www/html/web/modules/contrib/feeds_migrate/src/MappingFieldFormBase.php)
#0 /var/www/html/web/modules/contrib/feeds_migrate/src/MappingFieldFormBase.php(99): Drupal\feeds_migrate\MappingFieldFormBase->__construct(Array, 'default', Array, Object(Drupal\feeds_migrate\Entity\Migration), Object(Drupal\Core\Field\FieldTypePluginManager), Object(Drupal\feeds_migrate\Plugin\MigrateFormPluginManager))
#1 /var/www/html/web/modules/contrib/feeds_migrate/src/MappingFieldFormManager.php(77): Drupal\feeds_migrate\MappingFieldFormBase::create(Object(Drupal\Core\DependencyInjection\Container), Array, 'default', Array, Object(Drupal\feeds_migrate\Entity\Migration))
#2 /var/www/html/web/modules/contrib/feeds_migrate/modules/feeds_migrate_ui/src/Form/MigrationMappingForm.php(371): Drupal\feeds_migrate\MappingFieldFormManager->createInstance('default', Array, Object(Drupal\feeds_migrate\Entity\Migration))
#3 /var/www/html/web/modules/contrib/feeds_migrate/modules/feeds_migrate_ui/src/Form/MigrationMappingForm.php(151): Drupal\feeds_migrate_ui\Form\MigrationMappingForm->getMappingFieldFormPlugin('title', Array)
#4 /var/www/html/web/modules/contrib/feeds_migrate/modules/feeds_migrate_ui/src/Form/MigrationMappingForm.php(82): Drupal\feeds_migrate_ui\Form\MigrationMappingForm->buildTableRow(Array, Object(Drupal\Core\Form\FormState), Array, 0)
#5 [internal function]: Drupal\feeds_migrate_ui\Form\MigrationMappingForm->buildForm(Array, Object(Drupal\Core\Form\FormState))
#6 /var/www/html/web/core/lib/Drupal/Core/Form/FormBuilder.php(532): call_user_func_array(Array, Array)
#7 /var/www/html/web/core/lib/Drupal/Core/Form/FormBuilder.php(278): Drupal\Core\Form\FormBuilder->retrieveForm('migration_mappi...', Object(Drupal\Core\Form\FormState))
#8 /var/www/html/web/core/lib/Drupal/Core/Controller/FormController.php(91): Drupal\Core\Form\FormBuilder->buildForm(Object(Drupal\feeds_migrate_ui\Form\MigrationMappingForm), Object(Drupal\Core\Form\FormState))
#9 [internal function]: Drupal\Core\Controller\FormController->getContentResult(Object(Symfony\Component\HttpFoundation\Request), Object(Drupal\Core\Routing\RouteMatch))
#10 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(123): call_user_func_array(Array, Array)
#11 /var/www/html/web/core/lib/Drupal/Core/Render/Renderer.php(573): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#12 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(124): Drupal\Core\Render\Renderer->executeInRenderContext(Object(Drupal\Core\Render\RenderContext), Object(Closure))
#13 /var/www/html/web/core/lib/Drupal/Core/EventSubscriber/EarlyRenderingControllerWrapperSubscriber.php(97): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->wrapControllerExecutionInRenderContext(Array, Array)
#14 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(151): Drupal\Core\EventSubscriber\EarlyRenderingControllerWrapperSubscriber->Drupal\Core\EventSubscriber\{closure}()
#15 /var/www/html/vendor/symfony/http-kernel/HttpKernel.php(68): Symfony\Component\HttpKernel\HttpKernel->handleRaw(Object(Symfony\Component\HttpFoundation\Request), 1)
#16 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/Session.php(57): Symfony\Component\HttpKernel\HttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#17 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\Core\StackMiddleware\Session->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#18 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\Core\StackMiddleware\KernelPreHandle->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#19 /var/www/html/web/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\page_cache\StackMiddleware\PageCache->pass(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#20 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\page_cache\StackMiddleware\PageCache->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#21 /var/www/html/web/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#22 /var/www/html/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\Core\StackMiddleware\NegotiationMiddleware->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#23 /var/www/html/web/core/lib/Drupal/Core/DrupalKernel.php(708): Stack\StackedHttpKernel->handle(Object(Symfony\Component\HttpFoundation\Request), 1, true)
#24 /var/www/html/web/index.php(19): Drupal\Core\DrupalKernel->handle(Object(Symfony\Component\HttpFoundation\Request))
#25 {main}

p.s. should the whole message output be pasted or is it the preferred way to go with gist links or attached text files instead?

@rkoller
Thanks for testing! The error you are getting seemed to be caused by a cache issue. In the patch, changes were made to services and routes. Can you do a cache rebuild and try again?

Should the whole message output be pasted or is it the preferred way to go with gist links or attached text files instead?

If the error message is very long, I’d prefer it being attached in a text file. Then I have to scroll less through the issue. And well, it may draw a bit attention away from other things on this issue.
But I think there are no rules on how to share the information, just personal preferences.

@MegaChriz ahhhh good to know! i try to remember for the future to do a cache rebuild. haven't thought or even knew that changes to services and routes in patches might be the cause for errors as well.
and i can confirm that the mapping tab is now accessible without an error! i will play around with it. on a side note. one issue from the list i am writing up after the bof got solved with that patch already (see the screenshot).

Ran into a recoverable fatal error. Was struggling to get the import working with my XML example and switched to JSON. With that setup I managed to run an initial import creating three nodes. Mapping a unique_id to ID and name to title. For title I've applied a single process plugin to convert case (first character to lowercase). After the import worked I've added characters to trim removing the spaces in the string and added a second convert case (convert to uppercase - just for test purposes). Then I went to the importer settings and changed the settings to update existing content. when clicking save the fatal error happened (see the attached txt file)

@irinaz
Can you provide the exact steps that lead to the issue of not being able to remove a Tamper plugin? If I add the “Rewrite” tamper to the “title” field, and click “Remove”, the “Rewrite” tamper does get removed. Did you add other Tamper plugins too? To which field/property did you add the Tamper plugin?

Committed #62 with small changes.

@ipumpkin
Can you post your changes in a new issue? And document what you changed/fixed?

Hi @MegaChriz, thanks for your module, i has some more updates in other issue, kindly need your review.