HI,

We have tested the flag module on our drupal 8.2.1 .
When adding a flag relation in a view, the following error occurs:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'flagging_node_field_data.sid' in 'on clause': SELECT node_field_data.langcode AS node_field_data_langcode, node_field_data.created AS node_field_data_created, node_field_data.nid AS nid, flagging_node_field_data.id AS flagging_node_field_data_id FROM {node_field_data} node_field_data INNER JOIN {flagging} flagging_node_field_data ON node_field_data.nid = flagging_node_field_data.entity_id AND (flagging_node_field_data.flag_id = :views_join_condition_0 AND flagging_node_field_data.uid = :views_join_condition_1 AND flagging_node_field_data.sid = :views_join_condition_2) WHERE (( (node_field_data.status = :db_condition_placeholder_3) AND (node_field_data.type IN (:db_condition_placeholder_4)) )) ORDER BY node_field_data_created DESC; Array ( [:db_condition_placeholder_3] => 1 [:db_condition_placeholder_4] => recipe [:views_join_condition_0] => bookmark [:views_join_condition_1] => 1 [:views_join_condition_2] => ***FLAG_CURRENT_USER_SID*** )

Adding flags to a content type is not a problem only when trying to create a view, we receive this error. View is also not reachable after saving. This gives the same error.

Kind regards

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

germaike mv created an issue. See original summary.

joachim’s picture

Title: Unknown column 'flagging_node_field_data.sid' » Unknown column 'flagging_node_field_data.sid'
Issue tags: -views, -relationship
Spokje’s picture

Same issue here, latest Flag Module that worked was 8.x-4.x-dev with datestamp 1477080266 (# Information added by Drupal.org packaging script on 2016-10-21)

joachim’s picture

Category: Bug report » Support request
Priority: Major » Normal

There's no sid column, so I don't know where this is coming from in your views query.

Try reinstalling.

Spokje’s picture

Category: Support request » Bug report

The problem seems to be in giving Anonymous Users permission to flag/unflag Bookmark.

Reproducable by:

- Go to https://simplytest.me/
- Install: Drupal Core 8.2.2 (English, standard profile)
- Additional Project: Flag
- Enable modules Flag and Flag Bookmark
- Ensure that [site_root_url]/bookmarks works ("No bookmarks available.")
- Check Flag Bookmark and Unflag Bookmark Permissions for Anonymous User and Save permissions
- Yet again go to [site_root_url]/bookmarks.
- "The website encountered an unexpected error. Please try again later. "
- In Recent Log Messages:

Drupal\Core\Database\DatabaseExceptionWrapper: Exception in Flag Bookmark list[flag_bookmark]: SQLSTATE[42S22]: Column not found: 1054 Unknown column 'flagging_node_field_data.sid' in 'on clause': SELECT COUNT(*) AS expression FROM (SELECT 1 AS expression FROM {node_field_data} node_field_data INNER JOIN {flagging} flagging_node_field_data ON node_field_data.nid = flagging_node_field_data.entity_id AND (flagging_node_field_data.flag_id = :views_join_condition_0 AND flagging_node_field_data.uid = :views_join_condition_1 AND flagging_node_field_data.sid = :views_join_condition_2) INNER JOIN {users_field_data} users_field_data_node_field_data ON node_field_data.uid = users_field_data_node_field_data.uid WHERE (( (node_field_data.status = :db_condition_placeholder_0) ))) subquery; Array ( [:db_condition_placeholder_0] => 1 [:views_join_condition_0] => bookmark [:views_join_condition_1] => 1 [:views_join_condition_2] => ***FLAG_CURRENT_USER_SID*** ) in Drupal\views\Plugin\views\query\Sql->execute() (line 1488 of /home/d1648/www/core/modules/views/src/Plugin/views/query/Sql.php).

As stated before the Flag Bookmark Module/View does work Core 8.2.2 with the Flag 8.x-4.x-dev build from timestamp 1477080266 and Anonymous Flag/Unflag permissions.

joachim’s picture

Issue tags: +beta blocker

Looks like its this in the relationship handler:

        // Add in the SID from Session API for anonymous users.
        $this->definition['extra'][] = [
          'field' => 'sid',
          'value' => '***FLAG_CURRENT_USER_SID***',
          'numeric' => TRUE,
        ];
Spokje’s picture

It is.

As it turns out the surrounding if (isset($flag_roles[RoleInterface::ANONYMOUS_ID])) never returned True in the Flag 8.x-4.x-dev build from timestamp 1477080266, so that code never got fired.

In the current Flag 8.x-4.x-dev build however, it does return True, causing the Exception.

In my site I've commented out the whole block

        // Add in the SID from Session API for anonymous users.
        $this->definition['extra'][] = [
          'field' => 'sid',
          'value' => '***FLAG_CURRENT_USER_SID***',
          'numeric' => TRUE,
        ];

since it's clearing not working without the SID column in the table.

That however leaves us with nothing handling Anonymous Users Flagging/Unflagging.

In my case I solved this using hook_views_pre_render() to unset Flags that are not in \Drupal::request()->getSession()->get('flaggings', []) for Anonymous users.

I'm completely unsure if that approach is the right way to go, so I'm hesitant to put this up as a patch, but for what it's worth, it something along the lines of:

if ($view->getUser()->isAnonymous()) {
  if ($session_flaggings = \Drupal::request()->getSession()->get('flaggings', [])) {
    foreach ($view->result as $index => $result_row) {
      if (!in_array($result_row->flagging_node_field_data_id, $session_flaggings)) {
        unset($view->result[$index]);
      }
    }
  } else {
    $view->result = array();
  }
}

Clearly at some point in time the sid column was intended to do Anonymous User handling, but got outdated somehow. So something has to replace it.

joachim’s picture

> unset($view->result[$index]);

That will mess up your view's paging. Definitely not a permanent fix.

Spokje’s picture

Agreed, my Use Case had no paging to worry about.

I'm going to leave the fixing into more capable hands.

Spokje’s picture

Assigned: Unassigned » Spokje
Status: Active » Needs review
FileSize
1.71 KB

Ok, slept a night over this and came up with a patch.

This patch adds in the id's from the Session in the Views Query in the WHERE condition for Anonymous Users, instead of the non-existing ***FLAG_CURRENT_USER_SID***.

socketwench’s picture

Status: Needs review » Needs work
+++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
@@ -134,16 +134,23 @@ class FlagViewsRelationship extends RelationshipPluginBase {
+          // Since flagging_node_field_data id can't be NULL, adding this in ¶

Whitespace.

Also, when adding the flag relationship -- but before completing the Configure Relationship: Flags dialog -- I get the following error:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'id' in 'field list': SELECT node_field_data.created AS node_field_data_created, node_field_data.nid AS nid, id AS id FROM {node_field_data} node_field_data WHERE (( (node_field_data.status = :db_condition_placeholder_0) )) ORDER BY node_field_data_created DESC LIMIT 11 OFFSET 0; Array ( [:db_condition_placeholder_0] => 1 )

That may not be related to this issue. After completing the relationship configuration, the error does not appear and the view is accessible.

joachim’s picture

> This patch adds in the id's from the Session in the Views Query in the WHERE condition for Anonymous Users

I don't think this is going to scale.

There's going to be a limit to how many clauses you can add to an SQL query, isn't there?

I think #2475893: Support for Flagging by anonymous users needs a rethink.

Spokje’s picture

Also, when adding the flag relationship -- but before completing the Configure Relationship: Flags dialog -- I get the following error:

SQLSTATE[42S22]: Column not found: 1054 Unknown column 'id' in 'field list': SELECT node_field_data.created AS node_field_data_created, node_field_data.nid AS nid, id AS id FROM {node_field_data} node_field_data WHERE (( (node_field_data.status = :db_condition_placeholder_0) )) ORDER BY node_field_data_created DESC LIMIT 11 OFFSET 0; Array ( [:db_condition_placeholder_0] => 1 )

That may not be related to this issue. After completing the relationship configuration, the error does not appear and the view is accessible.

This error is unrelated to this issue.

Reproducible by:

- Go to https://simplytest.me/
- Install: Drupal Core 8.2.3 (English, standard profile)
- Additional Project: Flag (8.x-4.x-dev)
- Create View
- Add Relationship 'Content flag'
- Click button 'Add and configure relationship'
- Error appears above Preview area.

> This patch adds in the id's from the Session in the Views Query in the WHERE condition for Anonymous Users
I don't think this is going to scale.

There's going to be a limit to how many clauses you can add to an SQL query, isn't there?

Well, if there is, you need a pretty flag-happy user to reach it.

If you're worried about the amount of ORs in a WHERE clause: Perhaps a replace those with one 'IN'?

Anyway, because of:

I think #2475893: Support for Flagging by anonymous users needs a rethink.

I'm not putting any more time into this.

Spokje’s picture

Assigned: Spokje » Unassigned
joachim’s picture

> Well, if there is, you need a pretty flag-happy user to reach it.

I don't know what order of magnitude the limit would be.

On this site, I have 3000 flaggings (they're the issues I follow... I've commented elsewhere how Flag is a bad architectural choice for this on d.org, but that's what the current situation is).

I don't really know what the use cases are for anonymous flaggings, so I don't know how many flaggings an anon user might rack up.

Spokje’s picture

I don't really know what the use cases are for anonymous flaggings, so I don't know how many flaggings an anon user might rack up.

Well, in my case it's making flagging entities available for anon users in the same way it is for logged in users.
So it might end up with 3000+ flags as you have (in very rare cases), but usually it won't go any higher then 10-20 flags.

Anyway, I agree on the architectural "challenges", but for me the current patch works. I'll delete the whitespace and re-upload it, and see what comes out of the whole discussion. In the mean time people can use this patch, if needed/wanted.

Spokje’s picture

Status: Needs work » Needs review
FileSize
1.7 KB

Removed whitespace as noticed by socketwench in #11

joachim’s picture

Status: Needs review » Postponed
Issue tags: -beta blocker +Release blocker

This will be affected by changes at #2832106: identify different anonymous users by storing session id on Flaggings.

Downgrading blocking status, as that other issue is the key one.

(And actually, I'd be ok with this not being a blocker at all.)

Spokje’s picture

Fully agree with the postponing and this not being the key issue.

Not too sure if beta blocker => release blocker is a downgrade, but hey, that's only semantics...

andypost’s picture

Status: Postponed » Reviewed & tested by the community

This fixes nasty bug that makes flag unusable with views
So I suggest to fix it first to unblock usage of flag and then continue poking anonymous flagging

andypost’s picture

Status: Reviewed & tested by the community » Needs work

Fix is incomplete, sometimes view still fails

joachim’s picture

Title: Unknown column 'flagging_node_field_data.sid' » Fix Views support for anonymous flaggings (Unknown column 'flagging_node_field_data.sid')

Retitling.

Also the patch is going to need to be completely reworked once the other issue is in.

For one thing, hook_views_query_substitutions() needs to be restored.

joachim’s picture

Status: Needs work » Postponed

And postponed on the other issue, of course.

diegodalr3’s picture

FileSize
1.72 KB

I need it compatible with all entities, I think we can use $this->options['table'] in table name of conditions:
'flagging_' . $this->options['table'] . '.id'

joachim’s picture

> 'flagging_' . $this->options['table'] . '.id'

That doesn't look like a good idea to me -- table names should always be obtained from the handlers' methods, which take aliasing into consideration.

Also, note that the session stuff is getting ripped out in the other issue.

joachim’s picture

Status: Postponed » Active
metakel’s picture

I have the same error and it turned the user page (e.g. /user/1) to "the website encountered an unexpected error. Please try again later."

After I removed the permission for anonymous users to "flag" and "unflag", the above page was restored.

joachim’s picture

"The website encountered an unexpected error. Please try again later." is the error message that Drupal outputs when it has error output turned off. Please turn it on to get useful debugging output!

Rar9’s picture

This patch 24 still works with 8.x-4.0-alpha2

joachim’s picture

+++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
@@ -134,16 +134,23 @@ class FlagViewsRelationship extends RelationshipPluginBase {
+        if ($session_flaggings = \Drupal::request()->getSession()->get('flaggings', [])) {

It can't possibly be working, as we no longer store flaggings in the session.

BTW, please test against latest from git, not from releases.

aken.niels@gmail.com’s picture

FileSize
843 bytes

Isn't this as simple as following patch? Seems to work fine for me. Like joachim said, the patch in 24 will not work since flaggings are not stored in the session.

aken.niels@gmail.com’s picture

I'm sorry, did not test that thoroughly at all, previous patch will break for logged in users. Attached patch corrects that.

joachim’s picture

Status: Active » Needs work
+++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
@@ -134,15 +134,15 @@ class FlagViewsRelationship extends RelationshipPluginBase {
-          'value' => '***FLAG_CURRENT_USER_SID***',

We need to keep this placeholder, as it's what allows Views to cache the build query.

aken.niels@gmail.com’s picture

When using ***FLAG_CURRENT_USER_SID*** the view seems to fail to fetch results. When reverting that (by reinserting session_id()) it seems to work again?

joachim’s picture

IIRC the hook that handles replacing ***FLAG_CURRENT_USER_SID*** with a value got removed at some point. You can find it in the D7 code.

Abelluard’s picture

FileSize
631 bytes

Hello there,
as everyone here i'm encountering this issue on a view that have a flag's relationship. This flag can be used by anonymous user.

With the 4-0-0-alpha2 version, the drush command "entup" create the famous "session_id" column in the "flagging" table.
The strange thing is that in FlagViewsRelatioship class, the name of the field is still "sid", or i guess that it's "session_id".

Here a quick patch to fix this, but I don't know about regression... It just works on my dev env.

VISIOS’s picture

I used flag on a drupal 7 site for anonymous flagging . After getting the error message that started this issue and digging through the issues I understood, that views support is broken for anonymous flagging and older patches (#32) are not working.

So basically anon users can flag, but there is currently no way to display all the flagged entities because of the broken view support?

As my coding skills won't help here and I can't contribute in this way, I may ask if someone is eager to get this feature working because there are quite common use cases for anon flagging. I might chip in some money if this would be helpful as I'm having a client project that is very interested in this feature.

Thanks, Severin

socketwench’s picture

Looks like it's this hook we need to re-implement: https://api.drupal.org/api/drupal/core%21modules%21views%21views.api.php...

socketwench’s picture

Status: Needs review » Needs work

The last submitted patch, 39: 2825993.39.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

aken.niels@gmail.com’s picture

@socketwench, seeing that your patch uses the placeholder, does that seem to work alright for you? On my instance it did not work and only `session_id()` did the trick (although that would cause some caching problems as mentioned earlier by Joachim).

VISIOS’s picture

@ambidex @socketwench I applied the patch #38 and it made the view working without the error message of the missing column. I don't know how this is impacting caching. I haven't changed the placeholder as @ambidex id.

In the view I set the flags relationship to my flag and selected "by any user".

The view returns all the flagged content by anyone including anonymous users. Adding the field "(Flags) Flagging: Session ID" gives the session id associated with each flagged content.

How do I filter this to the session id of the current (anonymous) user?

Any help is very appreciated.

VISIOS’s picture

Any new insights regarding views support for anonymous flaggings? Is there anybody that could help me with this issue and probably is available for paid work to make this happen? Thanks!

s_leu’s picture

The last patch caused fatal errors on my environment due to missing use statement for the ViewExecutable type hint of the $view argument in flag_views_query_substitutions().

Adding a patch that fixes this.

VISIOS’s picture

Thanks for the new patch. It did apply cleanly and I did not encounter any problems.

Let me ask a question that is bugging me since month but unfortunately I didn't get answers from proficient people: How do I filter the view to the session id of the current (anonymous) user? The use case is that anonymous users can flag content (products) and see all flagged content on a special "wishlist" page.

I guess this is not possible at the moment, isn't it? Has this to be implemented in src/Plugin/views/sort/FlagViewsSortFlagged.php? Would this be another issue to open?

Who can help? Thanks!

Ddroid_za’s picture

Hi, I have the same requirements as Visios, any help, feedback would be appreciated. Thanks!

Ddroid_za’s picture

ok, so i installed the flag bookmark module. Clicking flags work fine as anonymous user and the values seem to be stored n the session. Going to the bookmark views page afterwards presents me with an empty view, until I clear the caches via drush. I have applied the patch above.

Ddroid_za’s picture

Ok, so disabling views data caching makes this behave as expected.

VISIOS’s picture

@Ddroid_za Good catch, I can confirm that setting the relationship to display flagged content "by current user" is showing also flagged items by anonymous users.

But this is only working one time when I clear the registry with drush cr. When I reload the page the view returns nothing for an anonymous
user. Drupal cache and and the cache setting in the view are disabled.

FlagViewsRelationship.php is disabling the cache for anonymous users (line 140 $this->pageCacheKillSwitch->trigger(); but I don't know why this odd behaviour with clearing the cache is working only on time.

It would be cool, if someone can shed some light on this.

Ddroid_za’s picture

@Visios, I found that disabling the view caches here, http:yoururl/admin/structure/views/settings/advanced makes it work without clearing the caches all the time. Obviously not ideal for a site using a lot of views and as stated it not recommended.

VISIOS’s picture

@Ddroid_za Thank you. This did the trick, of course with the downside of slowing down the website. Hopefully some clever minds will come up with a solution that involves views caching.

aken.niels@gmail.com’s picture

Because of this issue that you're experiencing, I'm still using the patch I created in #32. I'm pretty sure it wrecks the caching of the view (as stated in #33) and keeps it rebuilding, but this way we can enable the site caching globally for all other pages.

drclaw’s picture

Component: Flag Bookmark » Views integration
FileSize
1.7 KB

I believe the issue here is that the query substitutions hook is just in the wrong file. hook_views_query_substitutions() is in the views_execution group so it should be in a file named flag.views_execution.inc (or just in the flag.module file is fine too). The way it is now in flag.views.inc, the query substitution hook only runs when the file is included as part of invoking one of the other hooks in that file (i.e. when the cache is cleared).

Updated patch moves the hook and it should run on every page load after that.

Also I've updated the hook to use the session manager service to fetch the session id instead of calling session_id() directly.

For more info take a look at:

andypost’s picture

Good catch, no idea hot to add test for that

s_leu’s picture

Confirming that the latest patch fixes the issue, please commit.

VISIOS’s picture

Tested patch #53, confirming that anonymous flagging works with enabled views cache. Very cool, thanks @drclaw.

joachim’s picture

Status: Reviewed & tested by the community » Needs work

> I believe the issue here is that the query substitutions hook is just in the wrong file. hook_views_query_substitutions() is in the views_execution group so it should be in a file named flag.views_execution.inc

Good catch!!

A few nitpicks though:

  1. +++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
    @@ -134,15 +134,15 @@ class FlagViewsRelationship extends RelationshipPluginBase {
    +      if (isset($flag_roles[RoleInterface::ANONYMOUS_ID]) && \Drupal::currentUser()->isAnonymous()) {
    

    Any chance we could inject this service?

  2. +++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
    @@ -134,15 +134,15 @@ class FlagViewsRelationship extends RelationshipPluginBase {
    +
    

    Stray whitespace.

  3. +++ b/src/Plugin/views/relationship/FlagViewsRelationship.php
    @@ -134,15 +134,15 @@ class FlagViewsRelationship extends RelationshipPluginBase {
    +        // Check on PHP session id for anonymous users
    

    Missing full stop.

drclaw’s picture

Status: Needs work » Needs review
FileSize
3.73 KB

No problem on all accounts! Updated patch attached!

VISIOS’s picture

Status: Needs review » Reviewed & tested by the community

All green here. Patch is working. Thanks & please commit.

  • joachim committed 62d2687 on 8.x-4.x authored by socketwench
    Issue #2825993 by Spokje, Ambidex, drclaw, socketwench, s_leu,...
joachim’s picture

Status: Reviewed & tested by the community » Fixed

Committed.

Thanks everyone.

(Though if we hadn't ripped out all the D7 code in porting this module, we wouldn't have had to waste all this time on this issue...)

Status: Fixed » Closed (fixed)

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