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

Created on 9 February 2022, over 2 years ago
Updated 29 May 2024, 18 days 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 work

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

Comments & Activities

Not all content is available!

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

  • πŸ‡ΊπŸ‡Έ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.

  • πŸ‡§πŸ‡ͺ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 about 1 month ago
  • πŸ‡§πŸ‡ͺBelgium tim-diels Belgium πŸ‡§πŸ‡ͺ
  • πŸ‡·πŸ‡Ί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.

  • πŸ‡§πŸ‡ͺ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.

  • πŸ‡·πŸ‡Ί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.

  • πŸ‡¨πŸ‡¦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?

Production build 0.69.0 2024