Wasn't detecting when I was masquerading properly. Patch to follow.

CommentFileSizeAuthor
#2 1670978.2-masquerade_extras-masquerade_rules_is_masquerading.patch1.34 KBmrfelton
#1 1670978-masquerade_extras-masquerade_rules_is_masquerading.patch1.35 KBmrfelton

Comments

mrfelton’s picture

mrfelton commented
Status: Active » Needs review
StatusFileSize
new 1670978-masquerade_extras-masquerade_rules_is_masquerading.patch1.35 KB

Patch against latest code in the 7.x-1.x-rules branch, and using masquerade-7.x-1.x

mrfelton’s picture

mrfelton commented
StatusFileSize
new 1670978.2-masquerade_extras-masquerade_rules_is_masquerading.patch1.34 KB

Also, the function should not be expecting a value by reference, as that is not what Rules will pass to it. Updated patch resolves that.

wjaspers’s picture

wjaspers commented
Status: Needs review » Needs work 
+  if ($account->uid == $user->uid) {
+    return isset($_SESSION['masquerading']);
   }

I'm not sure that I trust this. Its possible if another module comes along and sets this value to null or the session hasn't been written yet (and the value 'cleared'), that we could get a false positive.

Another reason (I think) I originally went with the following code, is that I could detect if someone other than themself was masquerading.

if ($account == $user) {
    return !empty($user->masquerade_account);
  }

Otherwise, this looks good.
Thanks for all your feedback/patches, too!

mrfelton’s picture

mrfelton commented

But what is $user->masquerade_account ? I couldn't see this being set anywhere at all.

wjaspers’s picture

wjaspers commented

But what is $user->masquerade_account ? I couldn't see this being set anywhere at all.

Hmm. I went hunting for it, and can't find it now. Maybe the environment I had going was doing something that attached it to the user. (It was awhile ago).

Studying the code again, I think checking the $_SESSION will be ok. although an "empty()" check may be more appropriate than "isset()".. EDIT: I forgot about "anonymous", which empty() would match "0". So ... isset() is correct.

I also see another mistake I made, wherein we're checking the current user account against the database, instead of the account passed into the function.

wjaspers’s picture

wjaspers commented
Status: Needs work » Fixed

Per the prior patches you provided and the above comments:

http://drupalcode.org/project/masquerade_extras.git/commit/9fbe725131e05...

I tried to attribute the patch to both of using git's notes feature (see http://groups.drupal.org/node/161659), but I can't tell if it did anything.

Status: Fixed » Closed (fixed)

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