If a user is automatically logged in after they register ("Require e-mail verification when a visitor creates an account" is not enabled in user settings) the user.register action should return the user's session to save an extra login call.

CommentFileSizeAuthor
#10 user_register-1146896-10.patch2.35 KBdabblela
#7 user_register-1146896-7.patch2.31 KBdabblela
#1 user_register-1146896-1.patch1.04 KBdabblela

Comments

dabblela’s picture

dabblela commented
StatusFileSize
new user_register-1146896-1.patch1.04 KB

Patch attached

marcingy’s picture

marcingy commented

I disagree each resource should perform a specific function, so you do a registation followed by a login. This ensures that the client is robust to any changes on the server side.

dabblela’s picture

dabblela commented

I should add I'm working in the D6 branch for my project but I should be able to forward port this to D7 without much trouble if it looks OK.

marcingy’s picture

marcingy commented
Category: bug » feature

This is a feature request as well not a bug.

dabblela’s picture

dabblela commented

I see what you mean, but if user settings are set for immediate login, the resource is already performing the login function, just not telling the client about it. Forcing duplicate login calls seems like it could lead to unintended behaviors with implementations of hook_user that act on login.

marcingy’s picture

marcingy commented

I see where you are coming from as well. Let me have a think about this, might be worth speaking to Kyle too about this.

dabblela’s picture

dabblela commented
StatusFileSize
new user_register-1146896-7.patch2.31 KB

As per the discussion in IRC, please see attached.

marcingy’s picture

marcingy commented

From a quick scan on my part this looks good, I suppose it would be good if we had tests for this. I'll try the patch locally later against on d7.

voxpelli’s picture

voxpelli commented
Status: Needs review » Needs work

Some feedback on the patch - sorry for being nitpicky ;)

+++ b/resources/user_resource.inc
@@ -158,6 +158,14 @@ function _user_resource_definition() {
+              'name' => 'returnUser',

Rename "returnUser" to something more descriptive, like "returnSession" or just "session"?

+++ b/resources/user_resource.inc
@@ -158,6 +158,14 @@ function _user_resource_definition() {
+              'description' => 'Return session information if the user is logged in',

Modify to something like: "If true, a session for the user will be returned if possible."

+++ b/resources/user_resource.inc
@@ -230,13 +242,22 @@ function _user_resource_create($account) {
+    $new_user = array('uid' => $form_state['user']->uid);

Change the name of $new_user to something more descriptive - like $result.

+++ b/resources/user_resource.inc
@@ -230,13 +242,22 @@ function _user_resource_create($account) {
+        sess_regenerate();

Why do we need to regenerate the session here? If the user is logged in then the session regeneration will already have been dealt with elsewhere? 

+++ b/resources/user_resource.inc
@@ -230,13 +242,22 @@ function _user_resource_create($account) {
-  return $form_state['user'];

Why is this removed in this patch? Seems to be outside of any of the other code touched by this patch?

Powered by Dreditor.

dabblela’s picture

dabblela commented
Status: Needs review » Needs work
StatusFileSize
new user_register-1146896-10.patch2.35 KB

Patch attached. As for removing the return at the end, according to the if/else flow in the function (original and w/ this patch) there's no way the full user ever gets returned.

dabblela’s picture

dabblela commented
Status: Needs work » Needs review
marcingy’s picture

marcingy commented

Not sure if it is just me but it appears as nothing is being returned by the resource now?

kylebrowning’s picture

kylebrowning commented
Category: feature » task

This should actually be applied to the register method now as that is the form of registering a user, not the create method.

kylebrowning’s picture

kylebrowning commented
Status: Needs work » Closed (won't fix)

NO workin patch, re-open if you get one.