Comments

daniboy’s picture

Status: Active » Needs review
StatusFileSize
new1.1 KB

And the patch...

twistor’s picture

Issue summary: View changes
Status: Needs review » Needs work
Issue tags: +Needs tests
manasiv’s picture

The patch in #1 worked for me.
Use case:
I have a multi-valued field collection associated in a content type. A course has a lesson field collection.
Lesson consists of title, description, url fields. Feeds tamper was rewriting only the first lesson > url field. The other lesson urls were not being tampered/rewritten as required.

athanor’s picture

Thanks dude, you saved my day! Patch #1 helped me with rewrite of image uri-s mapped to multi value image field.

brtamas’s picture

I have adjusted that patch for 7.1.1

victoriachan’s picture

Hi,

Can you please explain how to use this? I have a multiple [first_name] field, and a multiple [last_name] field, and I want to import them to field_full_name.

So in my importer, I mapped:

  • first_name => Temporary target 1
  • last_name => field_full_name

And then in feeds_tamper settings, I added the 'Rewrite' plugin to the last_name field with the values: [first_name] [last_name]. But what I got was not right:

Eg. I had:

First names:

  1. Mickey
  2. Donald

Last names:

  1. Mouse
  2. Duck

Result:

  1. Mickey Mouse
  2. Mickey Mickey Mouse

It should be:

  1. Mickey Mouse
  2. Donald Duck

Am I using this wrongly?

Thanks,
Victoria

victoriachan’s picture

Status: Needs work » Needs review
StatusFileSize
new2.34 KB

Hi,

I've recreated this patch to make it work with multiple multiple value fields like described in my comment #6 above. So, for example you can now use the rewrite tamper for multiple [first_name] field with multiple [last_name] field, and the delta will be matched.

The rewrite plugin should still work with fields that are not multi value.

Example use case:

In importer, map:

- first_name => Temporary target 1
- last_name => field_full_name

In tamper settings:

Add a 'Rewrite' Feeds Tamper plugin to 'last_name' with the value of [first_name] [last_name]

Please review. Thanks!

Victoria

Status: Needs review » Needs work

The last submitted patch, 7: feed_tamper-multivalue_for_rewrite-1461308-7.patch, failed testing.

victoriachan’s picture

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

Updated patch for testing.

Status: Needs review » Needs work

The last submitted patch, 9: feed_tamper-multivalue_for_rewrite-1461308-8.patch, failed testing.

victoriachan’s picture

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

Trying again to pass test. Sorry for spamming.

Status: Needs review » Needs work

The last submitted patch, 11: feed_tamper-multivalue_for_rewrite-1461308-11.patch, failed testing.

victoriachan’s picture

Status: Needs work » Needs review
StatusFileSize
new15.77 KB

Updated again to pass test. Also added $delta to all 'multi => loop' plugin's callback functions though it won't be used for most of them (default to 0) so it is all standardised, and hopefully pass the tests.

Status: Needs review » Needs work

The last submitted patch, 13: feed_tamper-multivalue_for_rewrite-1461308-13.patch, failed testing.

victoriachan’s picture

Status: Needs work » Needs review
StatusFileSize
new351 bytes

I think my patch is failing the tests because the tests are now running with Feeds 7.x-2.0-beta3 whereas the previously passed test was running Feeds 7.x-2.0-beta2. This has nothing to do with my changes.

I'm attaching a patch with just a minimal change just to run it through the automated testing.

Not sure what to do to fix it as it is very difficult to understand the tests! It is now failing on the permissions setup in FeedsTamperEfqFinderTestCase (maybe 'administer fields' needs removing on line 22?) and also failing on the expected hashes in feeds_tamper_ui.test (defined in assertHashedDevseedFeedContent()), which I suspect just needs updating the hard coded hash, though I have no idea what the hashes are of, and it's probably better not to hard code the hash.

Possibly this is a separate issue: to get current dev to pass tests with latest Feeds version.

Help!

Status: Needs review » Needs work

The last submitted patch, 15: nochange.patch, failed testing.

Madis’s picture

No extensive testing, but patch in #13 solved my issue which was pretty much the same as described in #6.

I had to do a small change though since NULL values messed things up otherwise. Replaced
isset($value[$delta]) with array_key_exists($delta, $value) in feeds_tamper_rewrite_callback() .

elpino’s picture

I manually applied the patch and it worked great.

I had a piped list of filenames in source 'secondary images' in CSV
ie. 527014-2.jpg|527014-3.jpg

Applied the Explode plugin then applied the Rewrite plugin (patched) with replacement pattern "public://import_photos/[brand]/[secondary images]" and all image paths were replaced succesfully in multivalue field, even rows that only had one value before the explode plugin.

Because I made the changes manually, I'm not sure the patch applies cleanly though. I would change the status to 'Needs Review' at least, but I believe my test proves that it works.

elpino’s picture

Spoke to soon,
caught the following warning in watchdog log

Warning: Missing argument 7 for feeds_tamper_rewrite_callback(), called in /Users/adrian/Documents/sites/altas.viastara/sites/all/modules/feeds_tamper/feeds_tamper.module on line 77 and defined in feeds_tamper_rewrite_callback() (line 41 of /Users/adrian/Documents/sites/altas.viastara/sites/all/modules/feeds_tamper/plugins/rewrite.inc).

Don't know if that is related to #17 #1461308-17: Enabled "multi: loop" for the rewrite plugin, but not likely because my test didn't have NULL values...

4kant’s picture

I had a task very similar to #18.
For rewriting the path to the image I used the relative path "sites/files/mydirectory/[value]".

With patch #1 everything worked fine. No errors or warnings in watchdog.

nwom’s picture

Here is a re-roll of #13 with the change mentioned in #17. Leaving at needs work, since tests are needed.

nwom’s picture

nwom’s picture

After applying #22, I Immediately get a WSOD on the Feeds tamper screen of an existing import. This happens with #13 as well.

firfin’s picture

Status: Needs work » Needs review
StatusFileSize
new15.75 KB

The WSOD was caused by some overzealous copy pasting I think. The variable $source was defined twice in the callbacks of the urlencode and urldecode tampers.
Fixed this and made a patch. It is working flawlessly for me now. Used this patch to get multi-valued XML source into a multivalued entityreference using a rewrite. Let's see what the testbot says....

zkrebs’s picture

I used #24 successfully yesterday to import multiple taxonomy terms needing hierarchy so I could rewrite parent>> to each term during import.

It applied OK to 7.x-1.2

zkrebs’s picture

Status: Needs review » Reviewed & tested by the community
reszli’s picture

I can also confirm this works

fonant’s picture

Just a quick note that the patch #24 doesn't apply to Feeds any more.

Can't find a way to rewrite individually "exploded" values from a CSV field, so will have to workaround by rewriting the data in the CSV first.

megachriz’s picture

@fonant
Do not that this is a patch for the D7 version of Feeds Tamper. Did you try to apply it on the D7 version of Feeds Tamper?

I'm still willing to commit code to the D7 version of this project, but I won't put much time in testing it myself.

fonant’s picture

@megachriz Doh! Silly me. Yeah, I'm working with Tamper in Drupal 10.

Ideally wanting to explode a CSV field, and then rewrite each value individually (to prepend HTTP path to files).

I've avoided the problem by editing the CSV file to prepend the HTTP paths in advance, so I only need to explode the field to get the multiple values. That works fine :)

jeebsuk’s picture

@megachriz @firfin - We ran into this issue today on a D7 site we are still having to maintain for the next few months.

The patch was a lifesaver, thanks very much. +1 to RTBC.

firfin’s picture

Still working and tested :-)

Don't know if it's necessary, but I don't have tiem to put into writing tests for this. A commit credit would be nice though :-)

  • megachriz committed 432a0809 on 7.x-1.x
    Issue #1461308 by daniboy, manasiv, brtamas, victoriachan, Madis, elpino...
megachriz’s picture

Status: Reviewed & tested by the community » Fixed

Thanks for testing. I applied the patch locally and run all existing tests to see if it didn't break any tests - and it didn't. The patch has been committed to the dev version.

Now that this issue is closed, review the contribution record.

As a contributor, attribute any organization that helped you, or if you volunteered your own time.

Maintainers, credit people who helped resolve this issue.

Status: Fixed » Closed (fixed)

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