In some scenarios, the logout action triggers this warning:

Notice: Trying to get property of non-object in entitycache_user_logout() (line 408 of /www/pfw/releases/20140912130110/profiles/pfwprofile/modules/contrib/entitycache/entitycache.module)

This happens because hook_user_logout is trying to be cache_clear $account without checking previously that this object exists and has this field.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

alexmoreno’s picture

Assigned: Unassigned » alexmoreno
alexmoreno’s picture

Fix attached.

alexmoreno’s picture

recrit’s picture

Status: Active » Needs review
alansaviolobo’s picture

Status: Needs review » Reviewed & tested by the community

patch works well.

hussainweb’s picture

+++ b/entitycache.module
@@ -398,19 +398,25 @@ function entitycache_comment_delete($comment) {
+  if(isset($account) && ($account->uid)) {

Nitpick: Space after 'if' in this block and the next two.

Also, the condition could be simply collapsed to 'if (isset($account->uid))'

This can be fixed on commit.

greggles’s picture

Here's a reroll to add a space after the if and remove the parentheses which look unnecessary to me.

hussainweb’s picture

+++ b/entitycache.module
@@ -196,21 +196,27 @@ function entitycache_comment_delete($comment) {
+  if (isset($account) && $account->uid) {

Thanks for the fixes. The above could just be isset($account->uid), or if you care about 0 values, !empty($account->uid)

Chris Charlton’s picture

Bump.

parasolx’s picture

Agree with hussainweb. isset($account->uid) is enough to check either the uid is valid or not. So i just reattach the modified version with !empty(account->uid)

DamienMcKenna’s picture

Version: 7.x-1.2 » 7.x-1.x-dev
Assigned: alexmoreno » Unassigned
Status: Reviewed & tested by the community » Needs review

The latest patch needs to be reviewed.

mcdruid’s picture

I think @hussainweb was suggesting that the condition can be simply:

if (isset($account->uid)) {

...which I think makes sense.

  • mcdruid committed 3dd2318c on 7.x-1.x authored by alexmoreno
    Issue #2341355 by alexmoreno, parasolx, greggles, mcdruid, hussainweb:...
mcdruid’s picture

Status: Needs review » Fixed

Thanks everyone!

Status: Fixed » Closed (fixed)

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