In order to properly create user accounts during checkout, we need the following condition and action for Rules:

Condition: User exists - Checks if a user exists already for an e-mail address.

Action: Send an account e-mail - Lets you send one of the configured account e-mails to a given user.

I suppose we can track these in an issue here (so it's in the dcsprint5 list) and test them with checkout but write it as a patch against Rules itself. When it's ready to go we can kick it out to the Rules issue queue.

Files: 
CommentFileSizeAuthor
#22 rules-action-user-mail-1031530-22.patch3.19 KBNebel54
PASSED: [[SimpleTest]]: [MySQL] 351 pass(es).
[ View ]
#22 interdiff-1031530-13-22.txt452 bytesNebel54
#20 rules-action-user-mail-1031530-20.patch3.12 KBNebel54
PASSED: [[SimpleTest]]: [MySQL] 351 pass(es).
[ View ]
#20 interdiff-1031530-13-20.txt1.17 KBNebel54
#13 rules-action-user-mail-1031530-13.patch3.26 KBnedjo
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rules-action-user-mail-1031530-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]
#8 1031530.account_email_entity_exists.patch7.13 KBrszrama
PASSED: [[SimpleTest]]: [MySQL] 174 pass(es).
[ View ]
#5 1031530-2.patch4.53 KBrszrama
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1031530-2.patch.
[ View ]
#2 1031530.patch4.28 KBpcambra
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1031530.patch.
[ View ]

Comments

fago’s picture

>Action: Send an account e-mail
Yep, that would be a very very nice action!

>Condition: User exists
Hm, yes - however we might want to make this generically work. There is already the action "Fetch entity by property", with which you should be able to load users by mail address. Then we'd need #1019744: Condition: 'Data value is empty' to check for it being not existing.

To easing using that maybe it would be a good idea though to build this into a separate condition ala "Entity exists" which just works like the "Fetch entity by property" action?

pcambra’s picture

StatusFileSize
new4.28 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1031530.patch.
[ View ]

I'm attaching a patch for this condition & action.
I've done all the work in the checkout_commerce module, but at some point we should merge this into rules itself.
Related: #1041240: Replace entityCondition by propertyCondition in entity_metadata_table_query of callbacks.inc

pcambra’s picture

Status:Active» Needs review
pcambra’s picture

Assigned:Unassigned» pcambra
rszrama’s picture

StatusFileSize
new4.53 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch 1031530-2.patch.
[ View ]

I took what you had and ran with it a little bit. I had to rework pieces of the code to take advantage of an API function _user_mail_notify(). This lets us use the same function that the User module uses to send e-mails... I understand it's "namespaced" with the underscore, but it appears safe (the function definition hasn't changed in D6 at least).

If you wanna give this a look over, I'll get on building some default Rules to take care of anonymous checkout. What do you think of it, fago?

rszrama’s picture

I updated the action for sending an account e-mail to be a part of the User group instead of System.

fago’s picture

Patch basically looks good. Imho, both the action and condition are a very useful addition, so I'd be happy to add it directly to Rules too.

You got a tab in there:

+    'group' => t('System'),
+ 'base' => 'commerce_checkout_action_send_account_email',
+    'access callback' => 'rules_system_integration_access',

@condition:
Yep, we should make it use the entity API stuff + add the necessary property info for that.

rszrama’s picture

Project:Drupal Commerce» Rules
Version:7.x-1.x-dev» 7.x-2.x-dev
Component:Rules integration» Rules Engine
Assigned:pcambra» Unassigned
StatusFileSize
new7.13 KB
PASSED: [[SimpleTest]]: [MySQL] 174 pass(es).
[ View ]

Alrighty, here's a first pass at putting the condition and action into the Rules module in appropriate files. I tried to use existing callbacks as well. To test this, I used the following rule... it just checks to see if a user exists when they login (obviously TRUE) and then sends a welcome e-mail as a proof-of-concept.

{ "rules_test" : {
    "LABEL" : "Test",
    "PLUGIN" : "reaction rule",
    "REQUIRES" : [ "rules" ],
    "ON" : [ "user_login" ],
    "IF" : [
      { "entity_exists" : { "type" : "user", "property" : "mail", "value" : "[account:mail]" } }
    ],
    "DO" : [
      { "user_send_account_email" : {
          "account" : [ "account" ],
          "email_type" : "register_no_approval_required"
        }
      }
    ]
  }
}

When this lands, I'll update Commerce accordingly.

fago’s picture

Great! I've done another, more in-depth review.

+    // Attempt to send the account e-mail.
+    $result = _user_mail_notify($email_type, $account);

Looking at _user_mail_notify(), it only sends a mail depending on a configuration variable, but the action should work regardless of that setting. So let's just extract the parts of the function we need - that way we also don't have to call a private function ;)

+function rules_condition_entity_exists($type, $property, $value) {
+  $result = entity_metadata_table_query($type, $property, $value, 1);

We should use entity_query() here, such that it works with fields / any other given callback too.

+          'description' => t('The type of the entity that should be fetched.'),
+          'description' => t('The property value of the entity to be fetched.'),

I guess we need to update this descriptions :)

>+ $this->save[$selector] = array($immediate ? $wrapper : clone $wrapper, self::$blocked);
You got a hunk from #1044342: Errors creating and using newly created entities in rules in your patch.

fago’s picture

Status:Needs review» Needs work
mitchell’s picture

Any update on this?

jpstrikesback’s picture

Would be cool to pull this:

http://drupal.org/node/1349924

Back into rules

nedjo’s picture

Status:Needs work» Needs review
StatusFileSize
new3.26 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch rules-action-user-mail-1031530-13.patch. Unable to apply patch. See the log in the details link for more information.
[ View ]

This issue ended up with two pieces that are pretty much unrelated, so I pulled one into a separate issue, #1777204: Add condition "Entity exists by property".

Here's an untested patch for the remaining piece, a "Send account e-mail" action. I've followed fago's comment in #9 and pulled relevant code from _user_mail_notify().

nedjo’s picture

Title:Add a User condition and action to support anonymous checkout Rules» Add a User action "Send account e-mail"
jlporter’s picture

Component:Rules Engine» Rules Core

#13 - patch applied cleanly and option was available in user section of add action

however I got a blank screen in the overlay with just an error "Unknown action user_send_account_email." but the save button is there?!

jlporter’s picture

Status:Needs review» Needs work

doh! Clearing cache worked!

The only issue now is that it's not getting all the data it needs such as uid from the created entity.

Here are the warnings that showed when the email was triggered by the rule:

Notice: Undefined property: stdClass::$uid in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$pass in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$login in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$uid in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$pass in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$login in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$uid in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$pass in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$login in user_pass_reset_url() (line 2295 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$uid in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$pass in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).
Notice: Undefined property: stdClass::$login in user_cancel_url() (line 2317 of /mnt/var/contracts.blr.com/modules/user/user.module).

I'm going to try a different email type, we aren't setting their password. Not sure which one to use but this was from the "Welcome (new user created by administrator)" email type.

Just tried Account Activation type and got similar errors. I wonder if I should fetch the newly created user entity over again? I will test that in the morning and use the fetched entity to see if it gets uid/login/etc.

Good work nedjo! Bumping this back down to needs work. I might be able to finish this up if I learn rules internals a little better :)

jlporter’s picture

Status:Needs work» Reviewed & tested by the community

I figured out that the entity needs to be forced to save and it populates the uid and other fields properly! Works great! Please merge!

fago’s picture

Status:Reviewed & tested by the community» Needs work
+++ b/modules/user.eval.inc
@@ -98,5 +98,38 @@ function rules_action_user_unblock($account) {
+    module_load_include('inc', 'rules', 'modules/user.rules');
+
+    $types = rules_user_account_email_options_list();

I don't think this is necessary.

+++ b/modules/user.eval.inc
@@ -98,5 +98,38 @@ function rules_action_user_unblock($account) {
+      // If a user registered requiring admin approval, notify the admin, too.
+      // We use the site default language for this.
+      drupal_mail('user', 'register_pending_approval_admin', variable_get('site_mail', ini_get('sendmail_from')), language_default(), $params);

This should pass the right language also.

jlporter’s picture

I think the issues brought up by fago should be moved to another issue as no work has been done on this in 3 months.

Having this functionality working in the next release outweighs the recommendations/observations made.

Nebel54’s picture

Status:Needs work» Needs review
StatusFileSize
new1.17 KB
new3.12 KB
PASSED: [[SimpleTest]]: [MySQL] 351 pass(es).
[ View ]

I have updated nedjo's patch to address fago's suggestions, but i cannot figure out which language other than language_default() should be used for the admin notification mail. This part is directly copied from the user-module's _user_mail_notify() function - so it looks pretty ok for me.

fago’s picture

Status:Needs review» Needs work

I'm sry, your interdiff shows that I was wrong with
$types = rules_user_account_email_options_list();
- we'll need that to keep proper replacements. It's just the include loading that should go.

This part is directly copied from the user-module's _user_mail_notify() function - so it looks pretty ok for me.

True - I've missed that this is just the admin notification. So I agree that's fine.

So let's re-add that part and fix the issue.

Nebel54’s picture

Status:Needs work» Needs review
StatusFileSize
new452 bytes
new3.19 KB
PASSED: [[SimpleTest]]: [MySQL] 351 pass(es).
[ View ]

I reverted to patch #13 and just deleted the redundant include:
module_load_include('inc', 'rules', 'modules/user.rules');

So the patch should be fine now.

Nebel54’s picture

Status:Needs review» Reviewed & tested by the community

I just tested the patch and sorted out that the module_load_include was necessary.

So in short: Patch #13 does work and does what it is supposed to do.

Patches 20+22 are not necessary anymore. As @jlporter already tested #13. And the code works for me too I reset the status to RTBC.

fago’s picture

Status:Reviewed & tested by the community» Fixed

Thanks, yes this looks good now. I improved the check for authenticated users to check $account->uid and committed it

Btw usually, you are not supposed to RTBC your own patches/changes. ;-)

Status:Fixed» Closed (fixed)

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

artresys’s picture

Sorry. It was a mistake from my side.

The last submitted patch, 13: rules-action-user-mail-1031530-13.patch, failed testing.

The last submitted patch, 13: rules-action-user-mail-1031530-13.patch, failed testing.