Comments

cwgordon7’s picture

StatusFileSize
new3.07 KB

Thanks dmitrig01, patch rerolled.

cwgordon7’s picture

StatusFileSize
new2.57 KB

The best patches are always shorter (thanks dmitrig01!)

cwgordon7’s picture

StatusFileSize
new2.67 KB

Oops, rerolled.

cwgordon7’s picture

StatusFileSize
new2.58 KB

Patch rerolled after discussion with chx.

cwgordon7’s picture

StatusFileSize
new13.36 KB

Here is a rerolled patch that includes revised .test files to use the new function.

webchick’s picture

Oh, yes PLEASE! I'll try and test this a bit later.

webchick’s picture

Ok, suggestions upon reading the patch. Haven't tried it yet.

+  function drupalCreateNode($settings = array(), $account = NULL, $mod_node = NULL) {

1. It seems to me that $account is a redundant argument. You could simply do $settings['uid'] = 4, no?
2. I don't think mod_node is appropriate here. If we want the ability to clone nodes, let's add a drupalCloneNode function.

I would do something like this instead:

+  function drupalCreateNode($properties = array()) {

Then I could call it like this:

$properties = array(
  'uid' => 4,
  'type' => 'event',
  'field_start_date' => '2007-12-20',
);
$node = $this->drupalCreateNode($properties);

Otherwise, great addition, and it seems like this will remove TONS of code.

cwgordon7’s picture

In response to webchick:

It seems to me that $account is a redundant argument.

No, it's not. It's there for convenience; you can, of course, do 'uid' => 4, but if you pass in a user, the user's id as well as the user's name will be used. This is very convenient if you, say, have a user you want to have creating the node.

I don't think mod_node is appropriate here. If we want the ability to clone nodes, let's add a drupalCloneNode function.

You completely misunderstood the purpose of $mod_node; it's not a node to clone, but rather a node to resubmit, as you would do upon creating a revision.

The "TONS of code" you mentioned is there for convenience to the programmer, and I think it is acceptable.

-cwgordon7

Rok Žlender’s picture

Status: Needs review » Needs work

No, it's not. It's there for convenience; you can, of course, do 'uid' => 4, but if you pass in a user, the user's id as well as the user's name will be used. This is very convenient if you, say, have a user you want to have creating the node.

I don't understand why you would need users name. Only thing saved in db is uid.

You completely misunderstood the purpose of $mod_node; it's not a node to clone, but rather a node to resubmit, as you would do upon creating a revision.

For creating new revision you could simply do

$properties = array(
  'uid' => 4,
  'revision' => '1',
);
$node = $this->drupalCreateNode($properties);

I think webchick didn't have your tons of code in mind but rather tons of code in tests that is replaced by simple call to drupalCreateNode.

webchick’s picture

I think webchick didn't have your tons of code in mind but rather tons of code in tests that is replaced by simple call to drupalCreateNode.

Yep, that's right. Sorry about the miscommunication. And I still think simple is better on this function, although after looking more closely at the code, I'd change it to require specifying a node type as well; not all test environments can be guaranteed to have a 'page' type, and we ought to use the node type's defaults rather than some arbitrary defaults we define.

Working on an alternate patch..

Rok Žlender’s picture

StatusFileSize
new8.58 KB

I had something like attached patch in mind. There is one problem with this. Node revisions uid is taken directly from global $user and not from $node->uid so only way to force it to use another user would be to change global $user temporarily which seems a bit ugly to me. If anyone has another idea...

webchick’s picture

Awesome. That is exactly what I had in mind too. :)

I don't have any thoughts atm about that revisions problem. Hrm.

webchick’s picture

Well.

What about something like this (completely untested):

...
+    $node = ($settings + $defaults);
+    $node = (object)$node;
+ 
+    node_save($node);
+
+    // Compensate for node_revision's hard-coded $user->uid.
+    db_query('UPDATE {node_revision} SET uid = %d WHERE vid = %d', $node->uid, $node->vid);
...

Also, I think that 'type' should be a (required?) parameter into the function, rather than assuming that all nodes we want to create are pages. I can list off a dozen use cases for this off the top of my head, for example the OG's SimpleTest addOg function and Revision Moderation SimpleTest Tutorial (which it looks like I'll have to rewrite next week based on all the activity SimpleTest module's seen lately. That's a good thing. :D).

Additionally, although I haven't had a chance to test this patch yet, when I tried to do this myself I was running into issues with node type defaults (comments disabled, etc.) not being set properly. I got around this by doing a drupalPostRequest() or whatever that function is, to 'node/add/$type', which has the added bonus of testing your entire node save process. :) Not sure if this is the desired way to fix this, but just something to double-check as you're going through.

Thanks for picking this up again; sorry I dropped the ball.

Rok Žlender’s picture

Committed to HEAD thanks cwgordon07 for your patch and webchick for reviews and ideas. New nodes content type can also be set in $properties sent to drupalCreateNode I tried with story and it worked.

Rok Žlender’s picture

Status: Needs work » Fixed
webchick’s picture

Status: Fixed » Patch (to be ported)
boombatower’s picture

Status: Patch (to be ported) » Fixed

Doesn't seem anyone is porting and I'm not sure what it is to be ported to.

Anonymous’s picture

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.