This code in flag.inc (around line 1460 at the time of writing):

function get_views_info() {
  $entity_info = entity_get_info($this->content_type);
  return array(
    'views table' => $entity_info['base table'],

assumes that all entities has a base table. However, entities stored remotely may lack a base table.
A trivial work around for the notice would be an isset, but perhaps that's simply pushes the problem further down the stack. Should get_views_info() simply ignore entities that lack a base table?

I don't know what happens with the returned information later, so I can't tell what's the correct approach.

CommentFileSizeAuthor
#6 flag-fix-entities-no-base-table-notice-1862710-6.patch1.04 KBalexweber
#4 flag-fix-entities-no-base-table-notice-1862710-4.patch1.52 KBalexweber

Comments

joachim’s picture

joachim commented

Interesting problem! (And timely, as I've been working on a remote entity module myself: http://drupal.org/project/remote_entity !)

The right thing here would be:

- in get_views_info(), check for a base table as you suggest, and return NULL if there is no table
- in flag_views_data_alter(), check what's returned is non-NULL before adding the table data.

Another interesting problem is what to do when your remote entities use non-numeric IDs, such as GUIDs. Everything in Drupal assumes IDs are numeric. Unlike FieldAPI where you can take care of saying your entity type is not fieldable, there's no control like that in Flag. So you'd get a flag type for that entity type, which won't work, and which you should know not to use!

joachim’s picture

joachim commented
Version: 7.x-2.x-dev » 7.x-3.x-dev
joachim’s picture

joachim commented
Issue tags: +Novice

Tagging.

alexweber’s picture

alexweber commented
Status: Active » Needs review
StatusFileSize
new flag-fix-entities-no-base-table-notice-1862710-4.patch1.52 KB

Implemented functionality outlined in #1.

Note: the get_views_info() implementations in views handlers were ignored because if there isn't a base table the table data won't be added in hook_views_data_alter() in the first place.

joachim’s picture

joachim commented
Status: Needs review » Needs work 
+++ b/includes/flag/flag_user.inc
@@ -95,8 +95,9 @@ class flag_user extends flag_entity {
-    $views_info = parent::get_views_info();
-    $views_info['title field'] = 'name';
+    if ($views_info = parent::get_views_info()) {
+      $views_info['title field'] = 'name';
+    }

I don't think we need this check in flag_user... we know users have a base table!

alexweber’s picture

alexweber commented
Status: Needs work » Needs review
StatusFileSize
new flag-fix-entities-no-base-table-notice-1862710-6.patch1.04 KB

hah, good call! :)

joachim’s picture

joachim commented
Status: Needs review » Fixed

Fixed!

git commit -m "Issue #1862710 by alexweber: Fixed PHP notice for entities without a base table." --author="alexweber "

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