Needs review
Project:
Exportables
Version:
6.x-1.x-dev
Component:
Code
Priority:
Normal
Category:
Feature request
Assigned:
Unassigned
Reporter:
Created:
28 Jan 2010 at 23:42 UTC
Updated:
9 May 2011 at 00:40 UTC
Jump to comment: Most recent file
Comments
Comment #1
randallknutson commentedHere is an initial patch. I haven't fully tested it yet but I thought I'd get it out there.
Comment #2
jazzslider commentedI haven't had a chance to test it either yet, but I wanted to make a couple of notes after looking at the patch:
1. I like how you've got this functionality separated out as its own, separately-enableable module. That's not how we did it with taxonomy terms, but maybe we should. However, if we were to go that route, I'd prefer seeing all the sub-modules namespaced inside exportables (or something shorter, like how media_mover uses the mm_* prefix for all its contrib modules).
2. Your comments in node_content_node_load_all() seem inconsistent with the code; you said you were only pulling in nid and title, but then you selected * anyway. I was wondering (a) why, and (b) if there would be any appreciable performance benefits to only selecting nid and title, as your comments suggest.
Anyway, I hope to be able to test this out soon …just thought I'd make some comments on the go so you knew I was looking at this.
Thanks!
Adam
Comment #3
randallknutson commentedThanks for looking at this! I see this as a big thing for our company for static nodes (essentially anything not user generated).
1. I've renamed the module to exportables_node and think that is a pretty good convention to use. I'm open to another one if you have an idea.
2. Yes, my comments were from an earlier version and I didn't change them. I'll update them to more accurately reflect what is happening. I'm not sure if nid, node or * is more performant. I was mostly trying to avoid a node_load on every node in the system. We're going to have a lot of nodes and that could get ridiculous.
Hopefully sometime today I'll get a chance to work on this a little more. I tested it a bit but ran into a bug where it imports into the new system but doesn't set the machine name so it constantly fills exportable_machine_names with (0, node_content, '') and markes the feature as overridden.
I'm also hoping to add an admin settings page where you can select which node types are exportable. For us it will primarily be pages and webforms so we don't need to see all the other user generated content types in the list.
Finally, there are some fields in the node that probably shouldn't be pushed, like uid and created. You have a setting called "unique properties" that doesn't appear to do anything. Is that designed to filter out bits of the exportable?
Randall
Comment #4
randallknutson commentedOk, I've got some more work done on this. I had to work on the main module a little bit. There were a couple bugs and I added a prerender callback function that I'd like to use. I also renamed the node module to exportables_node.
I've tested it a bit and it pretty much works except it always shows as overridden. I'm going to add a little code to allow for the removal of pieces of the node per node type that should fix this.
Bugs fixed in exportables.module:
line 193: changed the default properties so unique properties didn't get overridden.
line 217: added a prerender callback to alter the node before final render. Unset all the unique properties.
line 780: add a check so exportables_machine_names doesn't get a bunch of blank values when enabling feature.
line 887: fix the property call to use the title_property instead of hard coded to name.
Comment #5
joshk commentedHas anyone looked at collaboration with node_export?
http://drupal.org/node/753544
Comment #6
doublejosh commentedsubscribe.