Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
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.
Comment | File | Size | Author |
---|---|---|---|
#27 | node_service_drupal_execute_fix_and_nid_return.patch | 2.72 KB | ddanier |
#26 | node_service_drupal_execute_fix.patch | 2.79 KB | civicpixel |
#7 | node_service_return_node.patch | 463 bytes | eli |
#3 | services_node_save.patch | 1.15 KB | RobLoach |
Comments
Comment #1
bas.hr CreditAttribution: bas.hr commentedthis would be very useful, I've patched myself node_service.module since I need ID of inserted node
Voting for this
Comment #2
RobLoachCould you post the code you had?
Comment #3
RobLoachWe 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.
Comment #4
eli CreditAttribution: eli commentedReturning 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.
Comment #5
snelson CreditAttribution: snelson commentedIf any bugs popup, we can always do a node_load after the save. Either way, I agree that the node should be returned.
Comment #6
RobLoacheli: Could you post a patch to return the $node object?
Comment #7
eli CreditAttribution: eli commentedHere you are.
Attached patch simply adds a "return $node" to node_service_save().
Comment #8
marcingy CreditAttribution: marcingy commentedComment #9
marcingy CreditAttribution: marcingy commentedThe 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.
Comment #10
eli CreditAttribution: eli commentedYou sure about that? I'm using it to create new nodes and I'm getting a full node object...
Comment #11
marcingy CreditAttribution: marcingy commentedMy comment yesterday was made after just eye balling the code.
I'll do a full test today and let you know how it goes.
Comment #12
marcingy CreditAttribution: marcingy commentedPatch commited needs to be ported to d6
Comment #13
RobLoachThis patch?
http://cvs.drupal.org/viewvc.py/drupal/contributions/modules/services/se...
Comment #14
marcingy CreditAttribution: marcingy commentedYes. Sorry Rob at the moment I'm sort of using this as a reminder and todo list!!!
Comment #15
RobLoachYay!
Comment #16
marcingy CreditAttribution: marcingy commentedComment #17
NaX CreditAttribution: NaX commentedThis 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.
Sorry I did not have time to create patch.
Comment #18
DJL CreditAttribution: DJL commentedThanks 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.
Comment #19
eli CreditAttribution: eli commentedNaX: 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!)
Comment #20
eli CreditAttribution: eli commentedAh, 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.
Comment #21
DJL CreditAttribution: DJL commentedSurely 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?
Comment #22
eli CreditAttribution: eli commentedI'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.
Comment #23
DJL CreditAttribution: DJL commentedWhere 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.
Comment #24
jrbeemanNaX'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.
Comment #25
jrbeemanJust 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:
Comment #26
civicpixel CreditAttribution: civicpixel commentedI 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
Comment #27
ddanier CreditAttribution: ddanier commented@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.
Comment #28
brmassa CreditAttribution: brmassa commentedGuys,
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
Comment #29
ddanier CreditAttribution: ddanier commentedCool, thanks! Great work ;-)
Comment #30
marcingy CreditAttribution: marcingy commented