Contrib.social

Contextual Filters cause duplicate results when saving legacy 8.x-1.2 draggableviews_structure rows

Open on Drupal.org →
Created on 9 February 2022, almost 3 years ago
Updated 24 July 2024, 4 months ago

After updating from 8.x-1.2 to 2.0.1, legacy views using contextual filters will create duplicate rows in the draggableviews_structure table when saving.

I've tracked this down to the draggableviews_views_submit function in the draggableviews.module file.

  $view_args = !empty($view->args) ? json_encode($view->args) : '[]';

  $connection = Database::getConnection();
  $transaction = $connection->startTransaction();
  try {
    foreach ($input['draggableviews'] as $item) {
      // Remove old data.
      $connection->delete('draggableviews_structure')
        ->condition('view_name', $view_name)
        ->condition('view_display', $view_display)
        ->condition('args', $view_args)
        ->condition('entity_id', $item['id'])
        ->execute();

Since the previous structure inserted a value of '[]' into the 'args' column of the draggableviews_structure table, I've found that when using the "Save order" button to save the structure, new values are entered into the 'args' column containing the contextual filter values (e.g. '["3"]') but the rows with the old value of '[]' are not deleted. This causes the results in the view to be duplicated, since there is a row with '[]' displayed along with a row with the new value of $view_args.

In the code above since the condition('args', $view_args) will be for example condition('args', '["3"]') whereas the existing legacy value would need to be condition('args', '[]') in order to be properly deleted.

I propose changing it to something like:

  $view_args = !empty($view->args) ? json_encode($view->args) : '[]';

  $connection = Database::getConnection();
  $transaction = $connection->startTransaction();
  try {
    foreach ($input['draggableviews'] as $item) {
      // Remove potential legacy rows with args = '[]' to prevent duplicate results.
      if ($view_args !== '[]') {
        $connection->delete('draggableviews_structure')
        ->condition('view_name', $view_name)
        ->condition('view_display', $view_display)
        ->condition('args', '[]')
        ->condition('entity_id', $item['id'])
        ->execute();
      }

      // Remove old data.
      $connection->delete('draggableviews_structure')
        ->condition('view_name', $view_name)
        ->condition('view_display', $view_display)
        ->condition('args', $view_args)
        ->condition('entity_id', $item['id'])
        ->execute();

I'm not sure how this will affect other use cases, although my thinking is that it won't affect any, since if $view_args isn't empty it will delete any legacy rows with '[]', and replace them with the new value of $view_args, which should happen anyways...

...similarly, if $view_args is empty, things should proceed as usual and still not break anything...

🐛 Bug report
Status

Needs review

Version

2.0

Component

Code

Created by

🇺🇸United States Zooney

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment 11 months ago →
    🇺🇸United States joshuasosa

    I'm getting duplicate records displayed in my view on 2.1.3 due to draggableviews. Patch in #11 doesn't help.

  • Comment 7 months ago →
    🇧🇪Belgium tim-diels Belgium 🇧🇪

    This should actually be just an update hook to resave the items to the new structure instead of adding code that is not needed anymore afterwards?

  • Status changed to Needs work 6 months ago
  • Comment 6 months ago →
    🇧🇪Belgium tim-diels Belgium 🇧🇪
  • Comment 6 months ago →
    🇷🇺Russia marassa Moscow

    This should actually be just an update hook to resave the items to the new structure instead of adding code that is not needed anymore afterwards?

    @tim-diels: The problem here is that when looping through all records in an update hook out of view context, you have no way of knowing what the view args were when the record was saved.

  • Comment 6 months ago →
    🇧🇪Belgium tim-diels Belgium 🇧🇪

    I think the code goes over the necessary records and delete them. You could do the same in an update hook?
    Isn't the problem just that records with '[]' are faulty and should be deleted? Just wondering if I'm thinking correct.

  • Comment 6 months ago →
    🇷🇺Russia marassa Moscow

    Isn't the problem just that records with '[]' are faulty and should be deleted?

    Nope. The legacy records with '[]' work fine with new module version and views created earlier. If you simply delete them, you will lose the order set by draggable views. The problems only start if you open an old view for editing and save it. That's when a duplicate set of records is created with args set to actual args but the old records with '[]' are not removed creating the confusion.That's when it's a good time to remove legacy records for this specific view.

  • Comment 6 months ago →
    🇨🇦Canada mandclu

    I'm wondering if the change merged as part of 🐛 Call to a member function id() on null on $this->getEntity($row)->id() Fixed might prevent the duplicate records from being included in the form. Could someone test with the dev release?

  • Comment 5 months ago →
    🇦🇺Australia oceanic Melbourne, Victoria

    I've updated the patch to take into account the most recent 2.1.4 release → , which adds in some of the changes from previous patches.

  • Open in Jenkins → Open on Drupal.org →
    Core: 10.2.x + Environment: PHP 8.2 & MySQL 8
    last update 5 months ago
    1 pass
  • Merge request !42Issue #3263519: Update patch to take into account the most recent 2.1.4 release, which adds in... → (Open) created by oceanic
  • Status changed to Needs review 4 months ago
  • Comment 4 months ago →
    🇷🇺Russia marassa Moscow

    Patch #19 works fine for me, can't tell for others. Could this possibly create problems if the same view is used with or without arguments and the sorting order should be different?

contrib.social Blog FAQ Discussions
Production build 0.71.5 2024