Errors creating and using newly created entities in rules

Oooh, regarding my last point about fetching, turns out I was forgetting to loop over the fetched entity. It would be nice if I could use the argument without a loop if the limit on the fetch is 1. Still doesn't solve the other problems, unfortunately, but may give me a workaround in the meantime.

Edit: Actually, the workaround won't work, because if you do an entity save on a newly created entity via Rules, the state I suppose is still trying to save the newly created entity as though it were new. Not really sure where to track this down, but it seems to me as soon as an entity is saved, it should satisfy any save requirements for that parameter throughout the Rule unless something else in it changes.

I may have tracked this down further with copious debug messages in the various RulesState::save*() methods. What appears to be happening is that my initial entity create action populates the state's $save array with a key using the underscore: entity_created. However, subsequent changes to the entity's properties and/or an entity save action add the wrapper again to the $save array using the hyphen: entity-created. My manual save action takes care of entity-created, since that's the actual selector, and I suppose when the rule is done evaluating, it tries again to save entity_created with the stale wrapper (i.e. having the duplicate data). What's interesting, though, is that altering the property does appear to alter it for both wrappers even though saving doesn't seem to populate the uid in both... will keep digging.

Aaaand, victory. Ok, as I suspected in my last post, the duplicate query was caused by a duplicate variable in the $state->save array. The reason the duplicate with the underscore gets there is because even though the RulesState::save*() methods accept a selector as the first argument, sometimes the variable name is passed. I see two ways forward... translate the $selector argument to turn _ to - or do this at the point of the calling. Since the argument is supposed to be a selector already, I think translating it at the point of calling the function is the way forward. The attached patch addresses this for RulesAction::executeCallback(), though there might be other instances. (Note that this patch may conflict with #1044338: Wrong variable name used for finding a state variable in RulesAction::executeCallback() since they both affect a same line.)

Additionally, the reason the saved entity isn't updated even if I force an immediate save through the "Save an entity" action is because RulesState::save() clones the wrapper when it adds it to $state->save (line 199). I'm not sure of the rationale here... why would we want to save a cloned wrapper instead of persisting the same one through the process? If I remove the clone, when I save, I can immediately use the uid in subsequent actions. Is there some drawback that led you to cloning in the first place?

Testbot fail. :-/ Perhaps because I put two patches in one issue?

Hmm, so, the original issue with not being able to actually save newly created entities has been resolved. That was an assured fix, and doing the strtr() in save() works just fine. However, for some reason, the save immediate accommodation isn't working. When I create and save a user account, its data is not populated. This is funny, because technically it should be working - so perhaps something else that's been committed is prevented it from working?

Basically, the user gets saved by my account_created wrapper's user variable isn't updated accordingly. It still just has the bare properties it received on creation with no uid. My workaround works fine, fetching the newly created account by property (e-mail address) and using the returned list item to populate the $order->uid. But I'm guessing this should work as the patch expected... will look at it a little more.

Investigated further to no avail, but I need to move on to other issues since I have a viable workaround. Perhaps the logic regarding when to clone should just be rolled back and addressed in another issue?

Basically, here's what doesn't work in my rule, even with the patch:

1. Create a user account.
2. Save the user account immediately.
3. Use the user account's uid anywhere (or any other property populated on save).

My debug messages showed that the wrapper in the saveNow() method is indeed updated. If you dump it before and after the save, it will show that the data in the wrapper is properly updated. However, back in the save() method, if I dump before and after the call to saveNow(), the data is the same. For some reason or another, even without the clone, it's not updated by reference.

I'm pretty certain my patch worked before, even though it would've broken other things... so I'm not sure why this patch doesn't work in my use case. Here's an export of the workaround in action:

{ "commerce_checkout_new_account" : {
    "LABEL" : "Create a new account for an anonymous order",
    "PLUGIN" : "reaction rule",
    "WEIGHT" : "2",
    "REQUIRES" : [ "commerce_checkout", "rules" ],
    "ON" : [ "commerce_checkout_complete" ],
    "IF" : [
      { "NOT entity_exists" : { "type" : "user", "property" : "mail", "value" : [ "order:mail" ] } },
      { "data_is" : { "data" : [ "order:type" ], "value" : "commerce_order" } }
    ],
    "DO" : [
      { "entity_create" : {
          "USING" : {
            "type" : "user",
            "param_name" : [ "order:mail" ],
            "param_mail" : [ "order:mail" ]
          },
          "PROVIDE" : { "entity_created" : { "account_created" : "Created account" } }
        }
      },
      { "data_set" : { "data" : [ "account-created:status" ], "value" : 1 } },
      { "entity_save" : { "data" : [ "account-created" ], "immediate" : 1 } },
      { "entity_query" : {
          "USING" : {
            "type" : "user",
            "property" : "mail",
            "value" : [ "account-created:mail" ],
            "limit" : "1"
          },
          "PROVIDE" : { "entity_fetched" : { "account_fetched" : "Fetched account" } }
        }
      },
      { "LOOP OVER" : [
          { "send_account_email" : { "account" : [ "list-item" ], "email_type" : "register_admin_created" } },
          { "data_set" : { "data" : [ "order:uid" ], "value" : [ "list-item:uid" ] } },
          { "data_set" : {
              "data" : [ "order:commerce-customer-billing:uid" ],
              "value" : [ "list-item:uid" ]
            }
          }
        ]
      }
    ]
  }
}

Just a reminder on this one... even though the clone issue wasn't fixed, the strtr() fix is still very important. Can we at least get that committed? I have a workaround for not being able to use a newly saved variable, but there's no working around the selector name issue.

Ok, cool. Indeed it does work as expected with that other patch applied. So, I'm wondering... can we get this patch applied today? If so, I can enable my default rule for creating user accounts in checkout as part of the beta. : )

(It'll still use the entity_fetch workaround until that user mess gets sorted out, though.)