Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
This is a follow up from #350409: Checking for uniqueness via a CCK field which stores an id .
Some parsers like CSV parser don't detect URLs and they create GUIDs from random patterns (Parser CSV md5's a cvs file's row).
If there was a mapper for GUID and URL this problem would go away. Users could define which element of the feed item contains GUID and/or URL and thus define how to check for the uniqueness of an item.
Comment | File | Size | Author |
---|---|---|---|
#19 | 368561-19_mapper_feedapi_url_guid.patch | 2.06 KB | Aron Novak |
#18 | 368561-18_mapper_feedapi_url_guid.patch | 2.06 KB | Aron Novak |
#15 | 368561-15_mapper_feedapi_url_guid.patch | 2.04 KB | Aron Novak |
#13 | 368561_mapper_feedapi_url_guid_1.patch | 1.57 KB | Aron Novak |
#10 | 368561_mapper_feedapi_url_guid.patch | 1.58 KB | Aron Novak |
Comments
Comment #1
burgs CreditAttribution: burgs commentedThis would be easy if guid came through as $field_name into the hook_feedapi_mapper function.
Anyone got any ideas of how to fix this?
Comment #2
tauno CreditAttribution: tauno commentedYou can update the guid in $node->feedapi->feed_item->options->guid, but it looks like the unique check has already been made by the time the feed element mappers do their work. See Step 4 of _feedapi_invoke_refresh() in feedapi/feedapi.module.
Comment #3
alex_b CreditAttribution: alex_b commentedThat's true. This feature request goes very deep into FeedAPI mapper's architecture... My initial post wasn't very clear on that. Properly flagging this as a feature request.
Comment #4
alex_b CreditAttribution: alex_b commentedComment #5
alex_b CreditAttribution: alex_b commentedThis feature will go hand in hand with #539752: Support 'unique' conditions: for full integration with feedapi_node we will need this mapper, and for implementing unique checking right, we will need #539752: Support 'unique' conditions.
Bumping this to 2.x.
Comment #6
Aron NovakI see two possible ways here to implement this feature:
1) The mapper remains node-dependent as the feedapi_mapper_node.inc is node-dependent. Then manipulating the node object is fine. As tauno wrote, $node->feedapi->feed_item->options->guid is the place where the guid can be put. However if i consider other processors (like FeedAPI Data), it's not such a good idea.
2) Manipulating the $feed_element variable. This is a totally new approach, until now, FeedAPI Mapper only did feed_element -> node mapping. What i propose here is a feed_element -> feed_element mapping, for example foo1 field of the feed element will be copied to the guid of the feed element. In this case this mapper can be node-independent. This would mean big changes in the architecture.
At first attempt, i'd say 1) is fine, because with the unique conditions feature, guid is not really a critical value anymore, because the mapper can override it.
Comment #7
Aron NovakPatch for approach 1), see #6. Basically this works when mapping to a node object.
Comment #8
Aron NovakThe patch needs review, the ideas in #6 needs to be discussed.
Comment #9
alex_b CreditAttribution: alex_b commented#6: I am not sure whether I understand the practical advantages of 2).
#7: url is options->original_url, not url. I'm getting bitten by this one all the time, too.
Comment #10
Aron Novak#9: yeah, original_url. It's really sick that the db schema and the form name convention is different in FeedAPI :(
I corrected that one and also i updated the mapper according to #539752: Support 'unique' conditions
Comment #11
alex_b CreditAttribution: alex_b commentedO, on a second read, this should be actually part of feedapi_node module - sorry, should have seen this when reviewing #7.
Once this is refactored as a patch to feedapi_node, we should be good to go.
We should be able to commit this without #536666: FeedAPI Mapper 2 compatibility.
Comment #12
Aron NovakAhha, this needs a little tweak in feedapi_mapper.module, as i see. Take a look on #541526: Ability to use mappers from other modules
The new patch will land soon.
Comment #13
Aron NovakComment #14
alex_b CreditAttribution: alex_b commentedWill need work once #539752-8: Support 'unique' conditions is fixed - can we move this into the .module file so that we don't need custom loaders like #541526: Ability to use mappers from other modules. ?
Comment #15
Aron NovakThis one will be fine very likely.
Depends on #541814: Breaks when enabling multiple processors and #539752: Support 'unique' conditions
Comment #16
alex_b CreditAttribution: alex_b commentedThese lines should go away after [##541814] is committed.
Comment #17
alex_b CreditAttribution: alex_b commentedPlease adjust these comments:
// Currently only does site-wide deduping
// Check for any duplicates, decide in _feedapi_node_save() whether to look for in-feed duplicates or any duplicates.
// Both fields support unique-ness
// Both fields support testing for unique items.
Comment #18
Aron Novak#541814: Breaks when enabling multiple processors is committed, so i updated the patch as well.
Comment #19
Aron NovakFixed a bug in #18, the array is keyed by the item ids, not the feed ids.
Comment #20
alex_b CreditAttribution: alex_b commentedOuch - if we commit this we lock feedapi_node into feedapi_mapper 2.x.
We have 3 options: either we repatch this to add it to feedapi_mapper 2.x or we branch for feedapi 2.x or we build a check for whether $active_processor is a string and thus determine the API version - I lean towards the latter.
Comment #21
Aron NovakI'd not like to branch. It would be nice to support both mapper branches.
"$active_processor is a string and thus determine the API version " This seems to be problematic.
Let's examine the function signature:
feedapi_node_feedapi_mapper($op, $feed_node, $active_processor, $node = NULL, $feed_element = array()
function node_feedapi_mapper($op, $node, $feed_element = array(), $field_name = '', $sub_field = '') {
Mapper 1.x third argument: $feed_element = array(), but can be a string also:
if (is_string($feed_element) || is_numeric($feed_element)) {
- from node mapper.Mapper 2.x third argument: $active_processor, always a string
So just is_string($active_processor) is not helpful.
I move this under feedapi_mapper/mappers/.
Comment #22
Aron NovakI also fixed a tiny bug in the returning stucture.
I replaced
to