node.save doesn't return the $node
eli - April 25, 2007 - 15:53
| Project: | Services |
| Version: | 6.x-1.x-dev |
| Component: | Code |
| Category: | bug report |
| Priority: | normal |
| Assigned: | brmassa |
| Status: | closed |
Description
node_service_save in node_service.module would be significantly more useful if it returned the newly created or updated node.
I'm pretty sure that was the author's intention since node.save specifies a "struct" return type in node_service_service(), though no struct is returned.
This is easily solved by adding:
return $node;
at line 88.
#1
this would be very useful, I've patched myself node_service.module since I need ID of inserted node
Voting for this
#2
Could you post the code you had?
#3
We could return the Node ID. If we return just the $node, I'm not entirely sure if it would include all "new" data associated with it.
#4
Returning the entire $node works fine. I've got code that relies on it.
That's almost certainly what the author of this module intended -- node.save specifies a "struct" return value.
#5
If any bugs popup, we can always do a node_load after the save. Either way, I agree that the node should be returned.
#6
eli: Could you post a patch to return the $node object?
#7
Here you are.
Attached patch simply adds a "return $node" to node_service_save().
#8
#9
The only issue with this is that as the node has not been saved way may not have a true node to return,. If it was a update this will be ok however for a new node we won't have an id etc.
#10
You sure about that? I'm using it to create new nodes and I'm getting a full node object...
#11
My comment yesterday was made after just eye balling the code.
I'll do a full test today and let you know how it goes.
#12
Patch commited needs to be ported to d6
#13
This patch?
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/services/se...
#14
Yes. Sorry Rob at the moment I'm sort of using this as a reminder and todo list!!!
#15
Yay!
#16
#17
This patch only works on saved nodes not newly created nodes.
It looks like drupal_execute() returns the new node path.
So I have hacked my node.save method like this.
<?phpfunction node_service_save($edit) {
if ($edit['nid']) {
$node = node_load($edit['nid']);
if ($node->nid) {
$ret = drupal_execute($node->type .'_node_form', $edit, $edit);
}
}
else {
$ret= drupal_execute($edit['type'] .'_node_form', $edit, $edit);
$path = explode('/', $ret);
if ($path[0] == 'node' && is_numeric($path[1])) {
$node = node_load($path[1]);
}
}
if ($errors = form_get_errors()) {
return services_error(implode("\n", $errors));
}
watchdog('services:node.save', t('@type: updated %title.', array('@type' => t($node->type), '%title' => $node->title)), WATCHDOG_NOTICE, l(t('view'), 'node/'. $node->nid));
return $node;
}
?>
Sorry I did not have time to create patch.
#18
Thanks for the 'patch'.
Why return the whole node? (I am bandwidth limited)
I have just created it, so I should already know all that is in it?
All that is needed is to return the node->nid
so I can remember it for future reference or ask for it using node.load.
#19
NaX: Good catch. Something changed with the way this whole module works starting in rev 1.5.2.3. It used to just call node_save($node). I don't quite understand the reason for the change.
DJL: There are other reasons why people would want the whole node. If you just want to return the nid, I suggest creating your own services module with a function that does just that (it's easy, I promise!)
#20
Ah, I see. It looks like it was to fix this security issue: http://groups.drupal.org/node/12573
There's gotta be a better way to do this (and check permissions) though.
#21
Surely it is easier to expect the user to issue a noad.load (nid) if they need the extra information - which does not require the creation of a new service. The simpler the better?
#22
I'm not sure either way is "easier."
But returning the whole node is the documented behavior, it's how things used to work, and people may have code that relies on it. I think it makes a lot more sense to just fix the bug and put things back the way they were rather than use this opportunity to change the function's signature.
#23
Where is this documentation? - I must admit I am operating somewhat in the dark here.
Have used the old xmlrpc to post pages of stuff so am basing what I do on my experience with that.
Reading the docs would help me progress and stop me making inappropriate suggestions!
Forgive me, I though services was a new module still under development.
#24
NaX's patch seems to do the trick. Any chance of this making it into the distribution? The node.save method is far more useful to remote services when they can get back data about the node they just created.
#25
Just ran into a case where NaX's code isn't sufficient. If the node being saved exists already and is being altered, the returned node is still the static cached node (i.e. the original node). This altered version of the method should fix that:
<?phpfunction node_service_save($edit) {
if ($edit['nid']) {
$node = node_load($edit['nid']);
if ($node->nid) {
$ret = drupal_execute($node->type .'_node_form', $edit, $edit);
}
}
else {
$ret = drupal_execute($edit['type'] .'_node_form', $edit, $edit);
}
if ($errors = form_get_errors()) {
return services_error(implode("\n", $errors));
}
$path = explode('/', $ret);
if (($path[0] == 'node' && is_numeric($path[1]))) {
$node = node_load($path[1], NULL, TRUE);
}
watchdog('content', t('@type: updated %title.', array('@type' => t($node->type), '%title' => $node->title)), WATCHDOG_NOTICE, l(t('view'), 'node/'. $node->nid));
return $node;
}
?>
#26
I was working through one of the great screen casts (http://files.thisbythem.com/screencasts/services-2.mov), but ran into several issues when I got to the pieces involving saving / editing nodes. Saw the comments posted on the group http://groups.drupal.org/node/13138, but didn't notice this issue until later. In order to get through the rest of the screen cast this evening I updated the drupal_execute code in node_services.module in the current dev release. I'm not sure if the above suggestions were already implemented in dev, or how much overlap my patch has with the above -- but I'll add my patch here if anyone wants to take a look at it. It's working for me, but I haven't tested it beyond the screen cast =)
--
Brian Hiatt
Civic Pixel
#27
@civicpixel: Your patch really did the trick, wothout it I was not able to save anything. Thanks ;-)
Anyway, I have updated your patch to return the node-nid of the created node...to fix the original issue here. Patch is appended.
#28
Guys,
i just commited the code David (with few changes). Thanks everyone! Its on CVS and soon on the next release.
Just remember that it will return the new node ID, not the entire node. Its consistent with several other web services.
regards,
massa
#29
Cool, thanks! Great work ;-)
#30