Problem/Motivation

This is coming from D7 feeds, but you may already have a plan for this. Currently it doesn't seem like targets can be created that are not going to actually map to a real field on the end.

The reason this is useful is because for example feeds_ex has a JSON fetcher that only maps to real fields and if you want to "tamper" or modify by merging to you end up having to create a new field and dealing with the tamper in a formatter or something on the entity after the processing. If you had a temporary target you could set the mapping to this and capture the source data for use when doing a ParseEvent listener for example.

Steps to reproduce

Proposed resolution

This may go against the pure nature (or I missed something and I'll gladly document if you can point me in the right direction) but D7 seemed to have this ability to have fake sources and fake targets, in this case only fake targets, for the use of manipulating would be needed and would be the scope of this proposal.

Remaining tasks

  1. Discuss
  2. Write Fake Field Target
  3. Add test

User interface changes

New generic temporary target option

API changes

Hopefully not.

Data model changes

Also hopefully not.

CommentFileSizeAuthor
#18 3206138-rewrite.png136.93 KBmegachriz
#18 3206138-tamper-list.png182.12 KBmegachriz
#18 3206138-mapping-temp-target.png64.57 KBmegachriz
#13 Added_test_for_temporary_target-3206138-13.patch3.57 KBdiaodiallo
#12 Added_test_for_temporary_target-3206138-12.patch3.8 KBdiaodiallo
#9 Added_test_for_temporary_target-3206138-9.patch2.51 KBdiaodiallo
#7 Added_test_for_temporary_target-3206138-7.patch3.53 KBdiaodiallo
#7 Added_test_for_temporary_target-3206138-7.patch3.89 KBdiaodiallo

Issue fork feeds-3206138

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

joelpittet created an issue. See original summary.

megachriz’s picture

megachriz
Primary language Dutch
commented
Issue tags: +beta target

@joelpittet
Good point. This idea has been discussed on the Feeds meetings a while ago - and it was mentioned on #2938505-10: Add UI for editing and removing custom (CSV) sources, but I think we didn't create an issue for it yet.

Thanks for creating the issue. I make it a beta target.

MegaChriz opened merge request !27

megachriz’s picture

megachriz
Primary language Dutch
commented
Status: Active » Needs review

I've made a draft, but perhaps it is already good enough.

I did not write any tests for it yet. I think there should only be a Kernel test that checks that an import happens without errors when mapping to this target. I think that the Kernel test should cover the following:

  1. Mapping a single source to a temporary target.
  2. Mapping multiple sources to a temporary target.
andileco’s picture

andileco commented

This worked for me with no issues. Thanks for putting this together!

irinaz’s picture

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

Thank you!! Tested on DevPanel CloudIDE - works like a charm. I feel comfortable moving it to "Reviewed and tested by community".
I hope this can be pushed to release soon - this is much needed feature!!!

diaodiallo’s picture

diaodiallo commented
StatusFileSize
new Added_test_for_temporary_target-3206138-7.patch3.89 KB
new Added_test_for_temporary_target-3206138-7.patch3.53 KB

Added a Kernel test for temporary field import.
- Test if nodes are imported after targeting the temporary_target.

megachriz’s picture

megachriz
Primary language Dutch
commented

@diaodiallo
Thanks for creating a test! Good idea to map value 'alpha' to the temporary target.

Here are my remarks:

  • I think that creating fields (the createFieldWithStorage() calls + setUpBodyField() call) can be left out, including mappings to these fields.
  • The setUpFieldDisplay() method can be removed.
  • There could be a second test that maps two times to "temporary_target", one mapper with value "alpha", one mapper with value "beta". That feed type would then have at least three mappers: "title" mapping to "title", "alpha" mapping to "temporary_target" and "beta" mapping to "temporary_target".
  • testIfFieldEmpty() could be left out I think, because there is nothing mapped to field_alpha.
diaodiallo’s picture

diaodiallo commented
StatusFileSize
new Added_test_for_temporary_target-3206138-9.patch2.51 KB

Thanks @MegaChriz

I removed setUpBodyField() call and setUpFieldDisplay() method, however I had to create fields with createFieldWithStorage() to have the assertion works.

Have now:
- Test for map to one temporary_target
- Test for map to two temporary_target

Let me know if these tests are what you except but also if there is still a need for performance specially for the repetitive mappings I did.

megachriz’s picture

megachriz
Primary language Dutch
commented

@diaodiallo
I think the field creations can be left out, because we don't expect Feeds to write to these fields.

One thing I noticed that is missing in the patch is the temporary target itself. It is on the issue fork, but not in the patch. I think the easiest way to get that in the patch is by applying the plain diff that is provided next to the issue fork: https://git.drupalcode.org/project/feeds/-/merge_requests/27.diff

diaodiallo’s picture

diaodiallo commented

Thank you @megachriz, I added the temporary target in the patch. However I need to find again how to do this test without these two calls of createFieldWithStorage(). The test fell when I comment it.

diaodiallo’s picture

diaodiallo commented
StatusFileSize
new Added_test_for_temporary_target-3206138-12.patch3.8 KB

Thanks for checking this @megachriz, I removed the field creation.

diaodiallo’s picture

diaodiallo commented
StatusFileSize
new Added_test_for_temporary_target-3206138-13.patch3.57 KB

Removed unnecessary codes.

megachriz’s picture

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

Thanks @diaodiallo
I've committed #13 with a few small changes in the code comments and fixing the coding standard issues that are mentioned by the bot on https://www.drupal.org/pift-ci-job/2154380.

  • MegaChriz committed 41b17ff on 8.x-3.x authored by diaodiallo 
    Issue #3206138 by diaodiallo, MegaChriz, andileco, irinaz: Added a...

Status: Fixed » Closed (fixed)

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

kopeboy’s picture

kopeboy
Primary language Italian
Location Milan
commented
Category: Feature request » Support request

So how can we use Temporary targets with other Tamper plugins? I don't see a [temporary_target] replacement that lets me map that value to another field 🤔

megachriz’s picture

megachriz
Primary language Dutch
commented
StatusFileSize
new 3206138-mapping-temp-target.png64.57 KB
new 3206138-tamper-list.png182.12 KB
new 3206138-rewrite.png136.93 KB

@kopeboy
You can map a source to the target 'Temporary target (temporary_target)', and then that source becomes available in the Tamper interface:

Then for example using the Rewrite plugin you can use that source: