Hello,

Firstly, thanks for writing this module.

I've been using Feeds / Feeds X Path parser and Feeds tamper to import this feed
http://rss.oucs.ox.ac.uk/oxitems/generatersstwo2.php?channel_name=engfac...

The aim is to only import the items with a category of "#greatwriters"

When I run beta3 as is, it never enters the keywords_filter call back function, and so all nodes are imported

I added this code at the end of the "foreach ($instances as $instance) {" if else block

feeds_tamper.module (line 84 - approximately)
-----------------------------------------------------------------------------
elseif ($is_array) {

$plugin['callback']($result, $item_key, $element_key, $item[$element_key], $instance->settings);

}
-----------------------------------------------------------------------------
None of the other options ever seemed to work (multi wasn't set or skip was empty). I thought it might be as I was only searching for one word, but changing the filter to two words (newline separated) didn't change anything.

Adding in the above lines does lead to the callback being fired, but it does lead to a problem that everything fails the comparison and is deleted.

So I modified keyword_filter.inc

and it now is

----------------------------------------------------------------------------------------------------

if ($settings['func']($field, $word) !== FALSE) {
if (!$settings['invert']) {

return;
}

unset($result->items[$item_key]);
return;
}else{

if(is_array($field)){

foreach($field as $compare){

if ($settings['func']($compare, $word) !== FALSE) {

if (!$settings['invert']) {

return;
}

unset($result->items[$item_key]);
return;

}

}

}

}

------------------------------------------------------------------------------------------

This now works "for me", but I wonder if this is a problem with a loop somewhere?
I hope the above can demonstrate how to replicate the problem.

I've attached my changes in a zip file as well.

Hope this is useful. I wasn't sure if this was a bug, or an issue, and this is my first drupal.org comment, so apologies if problematic.

Pat (pgogy).

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

pgogy’s picture

Version: 7.x-1.0-beta3 » 7.x-1.x-dev
Status: Patch (to be ported) » Needs review
FileSize
1.25 KB

Hello,

We fixed the problem by creating a new plugin for this module (which I have attached).

The problem seems to be that if you have multiple results (I.e that child occurs more than once per xml item - say category) then the callback function is never called.

We made a new plugin to handle this (this was really simple - thanks for that), but had to alter the default plugin array for this plugin to make it work.

Not sure if this breaks the logic, but felt it handy to share the code.

Pat (pgogy)

twistor’s picture

Status: Needs review » Postponed (maintainer needs more info)

I looked at the file and I'm not quite sure what the goal is.

pgogy’s picture

Say you import an RSS Feed with multiple category nodes. The default keyword plugin won't filter them. This plugin will filter across all of the nodes with that name.

So it will check every node for that keyword. The default plugin didn't seem to do that.

jmrivero’s picture

Priority: Minor » Normal
Status: Postponed (maintainer needs more info) » Reviewed & tested by the community

The plugin in #1 does what I think the keyword filter should do, filter out any item that does not have the desired keyword, so it should be included as the default keyword filter once tested or at least as another plugin, because the keyword filter actually packed with the module does not work as expected.

pgogy’s picture

You can add it yourself if it helps - happy to help if you need it

jmrivero’s picture

Iv added the plugin to my installation and it work as a charm. The keyword filter included with the module wont work to filter a feed by a single keyword in a "categories" tag inside a RSS feed. Iv tried with different combinations in the options, even with the same word listed several times with different case combinations but nothing works, tamper wont filter the feed. Copied the multiple plugin in the folder, applied it as the filter and the items without the keyword got filtered out.
I dont know if the default keyword filter works in other cases, but to filter a feed by taxonomy terms from a drupal feed view, it doesnt.

pgogy’s picture

I think by the looks of it we had the same problem - glad it worked for you.

twistor’s picture

Title: Possible patch when using keyword filters and feeds xpath parser » Add multi support to keyword filter plugin.
Category: bug » feature
Status: Reviewed & tested by the community » Needs work

Ahh, I understand what you're getting at.

pgogy’s picture

Do I need to submit a patch? Not sure I know how to....

barraponto’s picture

@pgogy yes, you do. It's really hard to see the changes you've applied, particularly because things have changed since you've posted your changes. I'll see what I can get from it, I'm having the same issues. And I did bang my head against the table until I saw what was happening.

barraponto’s picture

Status: Needs work » Needs review
FileSize
2.57 KB

Gotta hate PHP, it can't consistently return FALSE or 0 for every needle/haystack function...

I don't think merging the lines is a good approach as it might misbehave the regular expressions. Instead I tried really hard to loop over the multiple values and multiple words in a performant way (as much as PHP would allow me). This code is PHP 5.3+, though, since I send an array of context to array_reduce in order to work around preg_match/strpos/mb_strpos differences :P

Really hate PHP. But this patch works on every condition (regex, invert) where previous patch didn't work.

Status: Needs review » Needs work

The last submitted patch, feeds_tamper-1358578-11-multi-keyword-filter.patch, failed testing.

barraponto’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, feeds_tamper-1358578-11-multi-keyword-filter.patch, failed testing.

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 11: feeds_tamper-1358578-11-multi-keyword-filter.patch, failed testing.

iLLin’s picture

I was in need of fetching a feed and then filtering on a specific category. I needed to make sure the article being pulled in had the necessary tags in the category RSS tag. There was multiple category tags per article. The current stable release does not work as it loops through the callback for each category tag on a per article basis. So essentially if the tag you were filtering on is not the VERY FIRST value, then it unsets the whole record. So it removed itself from the results before it had an opportunity to keep looping until it found it.

So I adjusted the function before the call, to pass the callback an array. This also will increase the speed of this callback as a bunch of loops are no longer needed. I also cleaned up some code in the .module file.

I am attaching a patch to handle this.

iLLin’s picture

Status: Needs work » Needs review
iLLin’s picture

And I just realized I most likely broke all the other "loop" plugins. I'll change the keyword to something else as a one-off to prevent breaking other things. Maybe array?

Status: Needs review » Needs work

The last submitted patch, 17: feeds_tamper_multi_keyword_filter-1358578-17.patch, failed testing.

iLLin’s picture

Status: Needs work » Needs review
FileSize
2.9 KB

New patch

twistor’s picture

Why can't you just use 'direct'?

iLLin’s picture

Great Point! I started digging into it based on this original patch and didn't even notice. Let me fix. Thanks twistor!

Status: Needs review » Needs work

The last submitted patch, 21: feeds_tamper_multi_keyword_filter-1358578-20.patch, failed testing.

iLLin’s picture

Status: Needs work » Needs review
FileSize
1.68 KB

I removed the code cleanup from the .module file and changed to direct, added a typecast to it to make sure it was an array... etc. Patch added.

That code in the .module isn't needed, but it can be another issue as it doesn't relate.

Status: Needs review » Needs work

The last submitted patch, 25: feeds_tamper_multi_keyword_filter-1358578-25.patch, failed testing.

iLLin’s picture

So I'm curious, why is the $field parameter passed by reference?

function feeds_tamper_keyword_filter_callback($result, $item_key, $element_key, &$field, array $settings) {

I changed the typecast up as maybe that is causing an issue with the tests?

Anyway new patch added.

iLLin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 27: feeds_tamper_multi_keyword_filter-1358578-27.patch, failed testing.

iLLin’s picture

Hmm, maybe my loops are off, but if they were, then they were off to begin with. If this doesn't work, I'll start running these tests locally to see if I can find out wtf is going on.

iLLin’s picture

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 30: feeds_tamper_multi_keyword_filter-1358578-30.patch, failed testing.

twistor’s picture

The keyword filter tests are breaking.

We can't change the type of the field from scalar to array, it will break too many other things in Feeds.

iLLin’s picture

I got it fixed, no worries :D

iLLin’s picture

Status: Needs work » Needs review

  • twistor committed b1c1051 on 7.x-1.x authored by iLLin
    Issue #1358578 by iLLin, barraponto: Add multi support to keyword filter...
twistor’s picture

Status: Needs review » Fixed

Status: Fixed » Closed (fixed)

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

nithinkolekar’s picture

Status: Closed (fixed) » Active

Still there is no information how we can use regex on $field values, but as per #11 regex can be used.