pvhee on #721270-9: Assigning feed items to their own node:

I want to import a list of feeds (in OPML format e.g.) and each feed will be mapped on a different content type. For example, the first feed will be imported using RSS into a "story" content type, the second type using XML into a "shop item" content type.

...

I thought about modifying the Feed Node Processor to store feed items into different content types, based on e.g. a value of your feed list (e.g. your OPML specifies that feed 1 should be processed into content type x, or for that matter any other value parsed by your selected parser). Love to get feedback on this!

CommentFileSizeAuthor
#14 731338-14_content_type.patch1.61 KBpvhee
#9 731338-9_content_type.patch1.83 KBpvhee
#8 731338-8_content_type.patch1.66 KBalex_b
#7 731338-7_content_type_1.patch1.33 KBpvhee
#2 731338-2_content_type.patch1.29 KBalex_b
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alex_b’s picture

alex_b CreditAttribution: alex_b commented

I can easily imagine to add a node type as a mapping target, but what would be the rules by which we map to what content type?

Can you expand on "your OPML specifies that feed 1 should be processed into content type x" ? Would your OPML contain an explicit rule on which content type to create? Where would this information live in the OPML document and how should the OPML parser expose this information?

alex_b’s picture

alex_b CreditAttribution: alex_b commented
Status: Active » Needs review
FileSize
731338-2_content_type.patch1.29 KB

First shot at a patch for node type as a mapping target. Completely untested. Needs review.

pvhee’s picture

pvhee CreditAttribution: pvhee commented

Thanks Alex for breaking this out here!

but what would be the rules by which we map to what content type?

That is defined by the mapping source. If the mapping source has e.g. 'item_1' it should generate nodes of that type.

Would your OPML contain an explicit rule on which content type to create? Where would this information live in the OPML document and how should the OPML parser expose this information?

Yes. The OPML would have a 'type' attribute. One OPML feed line can then specify type='item_1', another line type='item_2'. This is not standard OPML, but it doesn't matter as you could use an XML parser for this as well (or anything else that specifies 'type').

alex_b’s picture

alex_b CreditAttribution: alex_b commented

The OPML would have a 'type' attribute. ... This is not standard OPML

Where exactly would this type attribute live? In an outline tag? Would you expect a patch to parse non standard elements to OPML parser go into Feeds?

pvhee’s picture

pvhee CreditAttribution: pvhee commented

Would you expect a patch to parse non standard elements to OPML parser go into Feeds?

I won't propose a patch for this. As you proposed in your patch (#2) we would only need a mapping target that sets the node type.

My implementation (in http://github.com/pvhee/feeds_multifeeds/) extended OPML to handle type attribute, but this can be defined by whatever mapping source.

alex_b’s picture

alex_b CreditAttribution: alex_b commented

#5: gotcha.

So all I need now is a review of #2 by you to get it committed. I've whipped up this patch and haven't even tried it : )

pvhee’s picture

pvhee CreditAttribution: pvhee commented
FileSize
731338-7_content_type_1.patch1.33 KB

Sorry for the late reply, in attachment is a reviewed patch of #2.

I've tested the patch and it works fine, except for one thing: when you change the node type I believe you need to rerun the node_object_prepare method so that the correct settings of that node type are loaded in the object (published, on front page, ..).

I've added that change to the patch and tested it.

alex_b’s picture

alex_b CreditAttribution: alex_b commented
Status: Needs review » Needs work
FileSize
731338-8_content_type.patch1.66 KB

I've tested the patch and it works fine, except for one thing: when you change the node type I believe you need to rerun the node_object_prepare method

I see that now we are running it twice if there is a mapping to a node type. node_prepare is an expensive operation, can we run it after map() and thus avoid it running twice?

I am a little queasy about the fact that a user can define a mapping target that may not exist on the node type that she is actually importing when using this functionality. Added a note.

pvhee’s picture

pvhee CreditAttribution: pvhee commented
Status: Needs work » Needs review
FileSize
731338-9_content_type.patch1.83 KB

I have moved node_prepare just before node_save, and removed it from the mapping itself, so it runs only once.

I am a little queasy about the fact that a user can define a mapping target that may not exist on the node type that she is actually importing when using this functionality. Added a note.

Hm yes, and this is difficult to solve if you want the user to give this power at import time. Any idea?

alex_b’s picture

alex_b CreditAttribution: alex_b commented
Status: Needs review » Needs work

No ideas on the content type target problem, but found another problem with the patch:

Running node_object_prepare() late will override mapped values with default values in some cases (node changed? node updated? ... not sure).

So in map() we want to make sure that the content type mapping is done first, then run prepare() then run the rest of the mapping stack. Does that sound reasonable?

pvhee’s picture

pvhee CreditAttribution: pvhee commented

It seems node_object_prepare changes the following values: status, promote, sticky, uid, created for new nodes, and date, log for existing nodes. Furthermore, it calls the nodeapi prepare hook so other values of the node could be changed too as well. See http://api.drupal.org/api/function/node_object_prepare.

As far as I know, those items aren't mapping targets for Feeds but internal properties of nodes. In that case, there is no need to split up the mapping. Or am I wrong here?

alex_b’s picture

alex_b CreditAttribution: alex_b commented

created is a mapping target, uid will be very soon...

B747’s picture

B747 CreditAttribution: B747 commented

Hi there, any progress on this?

pvhee’s picture

pvhee CreditAttribution: pvhee commented
FileSize
731338-14_content_type.patch1.61 KB

Issue is still open, in attachment the (still) incorrect patch from #9 rerolled against beta9.

The problem is that content type is not a normal mapping target, it also needs to call drupal_prepare_object such that other properties of the node can be changed with the new content type. However, this conflicts with other mapping targets as Alex outlined in #12. Probably the mapping needs to be split up, but this is not a straightforward change to make.

kenorb’s picture

kenorb CreditAttribution: kenorb commented
Issue summary: View changes
Status: Needs work » Closed (outdated)

Closed because Drupal 6 is no longer supported. If the issue verifiably applies to later versions, please reopen with details and update the version.