Trying to get property of non-object in flag_entity->applies_to_entity() [#2839047]

After deleting a flagged entity from my site using the Drupal API, I am getting the php error below when an outdated / cached search engine link is used to access the entity:

Request URI: https://my-site.com/ca/flag/flag/wishlist/35598

Notice: Trying to get property of non-object in flag_entity->applies_to_entity() (line 131 of /var/www/company_xxx/html/sites/all/modules/contrib/flag/includes/flag/flag_entity.inc).

I understand that the entity no longer exists, and that the url being used is invalid, but I wish the Flag module could better handle these situations without having to throw a php error.

See patch below which I created to fix this bug. If you are getting the same error message, I hope this helps.

Comment	File	Size	Author
#14	2839047-property-non-object-flag-entity-14-D7.patch	1.44 KB	somersoft
#14	7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass
#11	2839047-property-non-object-flag-entity-3.patch	1.91 KB	gturnbull
#11	7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass, 1 fail
#7	2839047-property-non-object-flag-entity-2.patch	1.1 KB	gturnbull
#7	7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass
#2	2839047-property-non-object-flag-entity.patch	1.24 KB	gturnbull
#2	7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass, 1 fail

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

23 December 2016 at 17:19

gturnbull created an issue. See original summary.

Comment #2

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 17:28

File	Size
2839047-property-non-object-flag-entity.patch	1.24 KB
7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass, 1 fail

Comment #3

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 17:50

Issue summary:

View changes

Comment #4

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 17:50

Issue summary:

View changes

Comment #5

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 17:55

Status:

Active

» Needs review

Comment #6

23 December 2016 at 18:50

Status:

Needs review

» Needs work

The last submitted patch, 2: 2839047-property-non-object-flag-entity.patch, failed testing.

Comment #7

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 20:08

File	Size
2839047-property-non-object-flag-entity-2.patch	1.1 KB
7.x-3.x: PHP 5.6 & MySQL 5.5, D7 30 pass

Comment #8

gturnbull CreditAttribution: gturnbull commented 23 December 2016 at 20:08

Status:

Needs work

» Needs review

Comment #9

joachim CreditAttribution: joachim as a volunteer commented 24 December 2016 at 22:16

Version:	7.x-3.9	» 7.x-3.x-dev
Status:	Needs review	» Needs work
Issue tags:	-Notice: Trying to get property of non-object

Good catch, and thanks for working on a patch for this.

+++ b/includes/flag/flag_entity.inc
@@ -128,8 +128,13 @@ class flag_entity extends flag_flag {
+    // Ensure the entity exists
+    // The entity can be empty if an outdated search engine link is used to access a flagged entity that has been deleted using the Drupal API
+    // For example, if the url http://my-site.com/ca/flag/flag/wishlist/35598 is loaded, after entity 35598 has been deleted

We don't need to be as wordy here. There's no need to say 'deleted by the Drupal API'; just 'deleted' will do!

And a simpler example than yours is a flag link that's out of date -- user A can load a page with a link to flag a node, and user B can delete that node between that and user A clicking the flag link.

Also, please remember to wrap comments to 80 chars.

```
+++ b/includes/flag/flag_entity.inc
@@ -128,8 +128,13 @@ class flag_entity extends flag_flag {
+    if (is_object($entity)) {
```
I'd rather we check is_null(), as that's what fetch_entity() returns when no entity can be loaded, and also if we made this check separately rather than nesting if()s.

Lastly, please please please don't tag issues before reading about how to use the tags on issues! They are not for repeating the title, or for random keywords!