devel_generate functionality

+  $users = devel_get_users();  
+  $author = user_load(array('uid' => $users[array_rand($users)]));
+  $recipient = user_load(array('uid' => $users[array_rand($users)]));
+
+  $form_state = array(
+    'values' => array(
+      'recipient' => $recipient->name,
+    ),
+    'validate_built_message' => array(
+      'author' => $author,
+      'recipients' => array($recipient),
+      'subject' => devel_create_greeking(rand(1, $values['subject_length']), TRUE),
+      'body' => devel_create_content(),
+      'timestamp' => time() - rand(0, $values['time_range']),
+      'op' => t('Send message'),
+    ),
+  );
+  pm_send($form, $form_state);

This code should use privatemsg_new_thread(), as relying on the form functions is imho a bad idea.

http://blog.worldempire.ch/api/function/privatemsg_new_thread/1

While i wholeheartedly welcome devel generate for pmsg, i know some issues will arise (or rather some important issues will be made more hidden) if we provide and rely only on 1 generic generate method.

To explain what i mean i have to remind of those tricky bugs we've encountered when amount of content was distributed between authors and recipients in a particular way.

Consider this table of number of messages between author and recipients( i only write one matrix as an example, but consider that sometimes Recipient is also an Author of the message he receives):
A | R
--+--
1 | 1
1 | n << recent bug we fixed in #490650: Message sent to oneself breaks new/#replies data loading
n | 1
n | n

We recently fixed a bug that appeared when author was sending message to self as well as some other people. This bug would be impossible to catch if devel generate not allow to generate content differently.

We need to proceed bearing the above in mind. I would also welcome a dedicated privatemsg_debug.module that encompases all of the possible message usage scenarios to be thorough. i realize that 'all of the possible message usage scenarios' sound loaded and like it is *alot* of scenarios, but really this number is rather finite and if we take care to resolving this first then we will be able to write more encompasing tests moving forward.

Subscribing

Here's a reroll of the patch, a few changes to add replies to the threads, plus comments by Berdir.

Urgh, this is my first patch. Guess I didn't get it right.

From 8699e2815b34931df8a4fe55e30f295cfebf1487 Mon Sep 17 00:00:00 2001
From: Keigan Ashall <keigan.ashall@gmail.com>
Date: Thu, 17 Nov 2011 16:24:37 -0500
Subject: [PATCH] Issue #502666: Add devel_generate
---
 .DS_Store                     |  Bin 0 -> 6148 bytes
 privatemsg.devel_generate.inc |  244 +++++++++++++++++++++++++++++++++++++++++
 privatemsg.module             |   10 ++

Some don't like git format-patch style patches, but it's fine with me, *if*:

- You make sure that you only work on a single commit so that follow-up changes aren't split up into multiple "mails". You can do this using git commit --append.
- The commit message follows the guidines and loos something like this: "Issue #502666 by nagiek, other_persons, ...: Added devel_generate integration."

 create mode 100644 .DS_Store
 create mode 100644 privatemsg.devel_generate.inc
diff --git a/.DS_Store b/.DS_Store
new file mode 100644
index 0000000000000000000000000000000000000000..5008ddfcf53c02e82d7eee2e57c38e5672ef89f6
GIT binary patch
literal 6148
zcmeH~Jr2S!425mzP>H1@V-^m;4Wg<&0T*E43hX&L&p$$qDprKhvt+--jT7}7np#A3
zem<@ulZcFPQ@L2!n>{z**<q8>++&mCkOWA81W14cNZ<zv;LbK1Poaz?KmsK2CSc!(
z0ynLxE!0092;Krf2c+FF_Fe*7ECH>lEfg7;MkzE(HCqgga^y>{tEnwC%0;vJ&^%eQ
zLs35+`xjp>T0<F0fCPF1$Cyrb|F7^5{eNG?83~ZUUlGt@xh*qZDeu<Z%US-OSsOPv
j)R!Z4KLME7ReXlK;d!wEw5GODWMKRea10D2@KpjYNUI8I
literal 0
HcmV?d00001

No idea what .DS_Store is, but I'm pretty sure it's not needed here :)

Googling it, looks like it's some Mac OSX Stuff: http://en.wikipedia.org/wiki/.DS_Store

You probably need to remove that from your commit and best add it to a .gitignore file.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ * Form; Integrates with Devel Generate

Should end with a ., also think the default is "Form builder; ..", see http://api.drupal.org/api/drupal/modules--user--user.module/function/use... and the corresponding validate/submit docblocks.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+    $options[$i] = $i;
+  }
+  ¶
+  $form['kill_content'] = array(
+    '#type' => 'checkbox',
+    '#title' => t('<strong>Delete all messages</strong> before generating new messages.'),

Trailing space. Below are more, look at the output of git diff (directly, not at a generated patch file) or use dreditor to see them easily.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  $form['num_messages'] = array(
+    '#type' => 'textfield',
+    '#title' => t('How many threads would you like to generate?'),
+    '#default_value' => 10,
+    '#size' => 10,
+  );
+  $form['max_thread'] = array(
+    '#type' => 'textfield',
+    '#title' => t('Max thread length?'),
+    '#default_value' => 5,
+    '#size' => 10,
+  );

The keys num_messages and max_thread don't really make sense. Maybe num_threads and max_thread_length?

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  $form['submit'] = array(
+    '#type' => 'submit',
+    '#value' => t('Do it!'),
+  );

Generate instead of Do it! ? :)

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  // minus anonymous and admin
+  if (count($users) - 2 < $max_recipients) {

Comments should be a real sentence and end with a ., something like "Ignore anonymous and admin for user count check."

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+    form_set_error($form_state['values']['max_recipients'], t('You only have @count_users users to handle @count_recipients recipients. You must increase the number of users.', array('@count_users' => count($users), '@count_recipients' => $max_recipients)));
+  }

The first argument to form_set_error() is the name of the form field for which this error is. In your case, 'recipients][max_recipients' (missing brackets at end/start are not a type, that's how it must be for nested elements).

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  drupal_set_message(format_plural($form_state['values']['num_messages'], '1 message created.', '@count messages created'));
+}

thread instead of messages?

Thinking about it, some time ago, we were talking about using "conversion" instead of "thread" in user facing strings, but this is not really user facing, so thread is fine.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  $author = user_load(array('uid' => $users[array_rand($users)]));

using array('uid' => ) is deprecated in D6 and doesn't work anymore in D7, so just user_load($user...

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  $recipients = array();
+  foreach ($recipient_uids as $recipient_uid) {
+    $recipients[] = user_load($recipient_uid);
+  }

user_load() isn't statically cached in D6. But you can use privatemsg_user_load(), which does. See http://api.worldempire.ch/api/privatemsg/privatemsg.module/function/priv...

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+  privatemsg_devel_generate_new_thread($values);
+  $context['results']['num_mids'] ++;
+}
+
+function privatemsg_devel_generate_batch_finished($success, $results, $operations) {
+  if ($success) {
+    $message = t('Created @num_mids messages successfully.', array('@num_mids' => $results['num_mids']));
+  }

Same here, you count the number of new threads, not messages.

Thanks! That's a great review. Will come back.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $form['submit'] = array(
+ '#type' => 'submit',
+ '#value' => t('Do it!'),
+ );

I know it's weird language, but that's in the other devel_generate forms.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $author = user_load(array('uid' => $users[array_rand($users)]));

Was in the original patch, had just left it. Changed.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $recipients = array();
+ foreach ($recipient_uids as $recipient_uid) {
+ $recipients[] = user_load($recipient_uid);
+ }

Sure.

Patch is as a single commit.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $form['submit'] = array(
+ '#type' => 'submit',
+ '#value' => t('Do it!'),
+ );

I know it's weird language, but that's in the other devel_generate forms.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $author = user_load(array('uid' => $users[array_rand($users)]));

Was in the original patch, had just left it. Changed.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,244 @@
+ $recipients = array();
+ foreach ($recipient_uids as $recipient_uid) {
+ $recipients[] = user_load($recipient_uid);
+ }

Sure.

Patch is as a single commit.

Re "Do it", I originally checked 7.x, which uses "Generate", while 6.x uses "Do it", as you said. I'd say we should do it the 7.x way, or it is another thing that would need to be updated when porting the patch to 7.x. But I don't really care...

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,240 @@
+  $users = devel_get_users();
+  $max_recipients = $form_state['values']['recipients']['max_recipients'];

The recipients fieldset doesn't have #tree set to TRUE, so it's just $form_state['values']['max_recipients']

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,240 @@
+  $author = user_load($users[array_rand($users)]->uid);

$users is just an array with the user id's, the ->uid part is unecessary.

Because the same mistake is also made a bit below with the recipients, it currently loops forever when trying to generate messages.

+++ b/privatemsg.devel_generate.inc
@@ -0,0 +1,240 @@
+  $recipients = array();
+  foreach ($recipient_uids as $recipient_uid) {
+    $recipients[] = privatemsg_user_load($recipient_uid);
+  }

privatemsg_user_load() also supports an array of user id's, so you can just write $recipients = privatemgs_user_load($recipient_uids);

I tried out the patch and it didn't really work yet, the function privatemsg_devel_generate_new_thread() was overcomplicated quite a bit and also had multiple errors (it e.g. the mentioned uid thing above), so I fixed the function, this one actually works:

function privatemsg_devel_generate_new_thread($values) {
  module_load_include('inc', 'devel_generate');

  $users = devel_get_users();
  // Do not allow anonymous (key) to send/receive private messages
  unset($users[key($users)]);

  $author = privatemsg_user_load($users[array_rand($users)]);
  $subject = devel_create_greeking(rand(1, $values['subject_length']), TRUE);
  $body = devel_create_content();

  $timestamp = rand(0, $values['time_range']);
  $options = array(
    'author' => $author,
    'timestamp' => time() - $timestamp,
  );

  // Remove author.
  unset($users[array_search($author->uid, $users)]);
  // Get a random amount of user ids.
  $recipient_uids = array_rand($users, rand($values['min_recipients'], $values['max_recipients']));
  $recipients = privatemsg_user_load($recipient_uids);
  // Convert to array if just a single user has been loaded.
  if (!is_array($recipients)) {
    $recipients = array($recipients);
  }
  // Remove recipients that failed to load.
  // @todo: privatemsg_user_load() should only return successfully loaded user
  // objects.
  $recipients = array_filter($recipients);

  $validated = privatemsg_new_thread($recipients, $subject, $body, $options);
  $thread_id = $validated['message']['thread_id'];
  $num_replies = rand(0, $values['max_thread_length']);
  $reply_timestamp = $options['timestamp'];

  // Generate threads.
  for ($j = 0; $j <= $num_replies; $j++) {
    $reply_body = devel_create_content();
    $reply_author = $recipients[array_rand($recipients)];
    $reply_timestamp = rand(0, $reply_timestamp);

    $reply_options = array(
      'author' => $reply_author,
      'timestamp' => time() - $reply_timestamp,
    );
    privatemsg_reply($thread_id, $reply_body, $reply_options);
  }
}

I've created ~5500 threads without any errors with this.

I also noticed on a clean d6 site, that it only works if users have the write privatemsg permission because the API verifies the permissions. Maybe add a validation check to see if authenticated users have that permission?

OK, using your function. Added a check for 'write privatemsg' permission. Drupal 6 doesn't have a user_role_permissions function, so created one. Seems to work fine.

Sorry, whitespace errors. Use this.

One more time!

Commited to 6.x-2.x, thanks for working on this!

Are you up for a 7.x-2.x port? :)

7.x patch. I'm not sure how to see whitespace before actually doing an upload...

Also, sometimes, I don't know why, it spits an error that recipients column is NULL. I can't figure out why.

Thanks!

Re whitespaces, doing git diff returns by default a colored diff which also highlights trailing whitespaces (At least it does so under linux). Not sure about Windows.

Many editors/IDE's can be configured to remove them automatically or e.g. only from changed lines which is very helpful. Netbeans for example can be configured to format code automatically almost perfectly according to the coding standards.

Patch looks good, will try it out and commit when I find the time.

bump

Sorry for letting you wait for so long!

Our usage (or maybe my simplifcation above) is the problem of the NULL problem.

We're currently using the key of the $uids array but it's a numerical array. So we always used an integer between 0 and 49 for the users and you might or might not have users with such id's :)

I fixed that and commited the patch. We probably need the same fix for 6.x-2.x as well, can you check that?

No worries!

Oh yeah, user check. Do you have the patch you committed? Would be easier to see.

I simply changed a single line of code, using drupal_map_assoc() to make sure that the keys are uid's as well., see http://drupalcode.org/project/privatemsg.git/blob/refs/heads/7.x-2.x:/pr....

This issue is being closed because it is against a branch for a version of Drupal that is no longer supported.
If you feel that this issue is still valid, feel free to re-open and update it (and any possible patch) to work with the 7.x-1.x branch (bug fixes only) or the 7.x-2.x branch.
Thank you.