Requesting and Supporting in porting module to D8.

Comments

heykarthikwithu created an issue. See original summary.

platinum1’s picture

Are there any plans to port this module?

jay.dansand’s picture

Status: Active » Postponed

At some point this module will be ported to D8, but not for a while (I don't have time to work on a D8 port until my employer starts migrating to D8).

If someone wants to volunteer supporting a port, I'd be happy to make them a co-maintainer!

platinum1’s picture

Thank you for the reply!

lahode’s picture

For those how need a quick solution in D8, here you are:

function MYTHEME_preprocess_views_view_unformatted(&$variables) {
  $view = $variables['view'];
  $rows = $variables['rows'];

  if ($view->id() == 'my_view_name' && $view->getDisplay()->display['id'] == 'my_view_display') {
    $nids = [];
    foreach ($rows as $id => $row) {
      $node = $rows[$id]['content']['#node'];
      if (in_array($node->id(), $nids)) {
        unset($variables['rows'][$id]);
      }
      else {
        $nids[] = $node->id();
      }
    }
  }
}

Cheers

metakel’s picture

I studied #5's code but somehow it does not work. So I wrote the following code:

<?php

function MY_THEME_preprocess_views_view_table(&$variables) {
  $view = $variables['view'];
  $rows = $variables['rows'];

  if ($view->id() == 'MY_VIEW_NAME' && $view->getDisplay()->display['id'] == 'MY_DISPLAY_MACHINE_NAME') {

    $result_rows = []; // initial the result array for unique nid's row
    $result_nid = []; // initial the nid pool; if a row's nid is found here, it will not be added to the $result_row array and its nid will not be added here. Otherwise the nid is added to this pool
  
    foreach ($rows as $id => $row) {
        $row_nid = intval(strip_tags(trim($row['columns']['nid']['content'][0]['field_output']['#markup']->__toString())));
  
        // if not duplicate
        if (!in_array($row_nid, $result_nid)) {
              array_push($result_nid, $row_nid);
              array_push($result_rows, $row);
        } 
    }
    $variables['rows'] = $result_rows; // Change the views' result rows
  }
}

and I added it to the theme's template.functions.php file.

I am new to Drupal 8's coding and I am not sure whether the above code meets the standards. And I would like to know the followings too:

1. I want to use the "Views Bootstrap" method. This module provides a few new view display methods. I would like to use its "Bootstrap Table" display. However if I change the above PHP function name to "MYTHEME_preprocess_views_view_views_bootstrap_table" or "MYTHEME_preprocess_views_view_bootstrap_table", the function was not called. Do you have any idea how can I use the "Views Bootstrap Table" display instead of the built-in "Table" display?

2. I do not know how to get the NID from the $variable['rows'], so I used a very clumsy way: add a NID field to the view and hidden it with Bootstrap's hidden class (or CSS "display: none"). And you can see I used a very lengthy way to get the NID from the result rows. Is there a better way to get the result row's NID?

byrond’s picture

I found that "#node" was not available in the row, but you can get the node ID with $rows[$id]['content']['#row']->nid;

#5 worked for me after making that change. My site is currently using 8.3.x.

@metakel, you will need to change 'MY_THEME', 'MY_VIEW_NAME', and 'MY_DISPLAY_MACHINE_NAME' to values applicable to your site. I haven't used the Bootstrap preprocess hooks, so I can't give any advice there.

kaido.toomingas’s picture

Its mostly working.. mine was

use Drupal\views\ViewExecutable
function rk_special_views_pre_render(ViewExecutable $view) {
  if ($view->id() == 'someview') {
    $profiles = [];
    $new_result = [];
    foreach($view->result as $k => $row) {
      $id = $row->_entity->id();
      if (!in_array($id, $profiles)) {
        $profiles[] = $id;   
        $new_result[] = $row;
      }
    }
   $view->result = $new_result;
  }

}

but it will mess up the page using pagers since its done after query has run.. Will try to find better solution.. Maybe something related to hook_views_query_alter()

Jorge Navarro’s picture

Views distinct doesn't seem to be needed with D8. You can go to Advanced -> Query configuration options -> Check "Unique"

brandonratz’s picture

@Jorge Navarro, I see no such an option (Drupal 8.4.5). Query "Settings" is an option with Distinct (indicated by others to not be helpful)

ollaankoodeis’s picture

Any updates to this? Or better approach than messing with pre_render which breaks result count when using pagered result.

jay.dansand’s picture

We're just beginning to start our D8 migration, so conceivably I'll be porting Views Distinct in the medium term (say, 6 months).

dakku’s picture

@jaydansand I should be able to help with the D8 port, maybe we can discuss how we can collaborate!

tunaman’s picture

Wondering if there are any updates on the D8? I'm happy to pay for the development.

homersheineken2’s picture

Also looking to see if there is an update on porting to D8

jay.dansand’s picture

Hello all, thanks for the interest! Other projects were prioritized at work, so I have not gotten to Views Distinct yet. As always, if anyone wants to make a D8 port I will happily accept it, otherwise I promise I will get to it eventually: this sounds like what I said a year ago, but officially our D8 move is underway now, so it really should be within a year that we'll get to this module.

I'm also hoping that the D8 version will be a good time to adopt a better/different aggregation (not just filtering) strategy, since we won't have to worry about breaking existing installs, see #2158653: Improve Aggregation Behavior.

colan’s picture

Title: Porting module to Drupal 8 » Port Views Distinct module to Drupal 8
Assigned: Unassigned » colan
Status: Postponed » Active

I'm currently running into the issue that this module solves, and would be happy to work on the D8 branch as I need to get something sustainable in place. For now, I'll get the D7 filtering in, and would like to stick to alphas for now (just in case we want to re-jig things later).

I don't believe this will be a huge amount of work as there's not a lot of code in the D7 branch. It will be mostly replacing the procedural code with OO (and some refactoring to shrink large methods as per Clean Code).

@jay.dansand: Care to make me a maintainer so I can create the branch and start pushing to it? Until I get access, I'll simply work in my own unpushed branch. (If, for some reason, you don't want me to become a maintainer, I could work with patches here, but that would be less than optimal.)

#8: Yes, hook_views_pre_render() is too late in the process. The solution is hook_views_post_execute(). This is what this module does in D7. Note the in-line documentation:

We use hook_views_post_execute instead of hook_views_pre_render in order to de-dupe before the pager/etc. is built. Post-execute is the first opportunity to check the results, so we do it then.

colan’s picture

Actually, I started working on the core issue for this instead: #2993688: Views relationships with multi-valued entity reference fields invalidate Distinct query option. Reviews, testers, and additional patches welcome!

Maybe we can all work on that one, and then we won't need this contrib module for D8?

solariel’s picture

StatusFileSize
new35.46 KB

Here's a first porting attempt. Quite crude but I hope salvageable.

jasonluttrell’s picture

@solariel, thanks for your work on this. I ran your script and enabled the module. Note that D9 requires "core_version_requirement: ^8.8 || ^9" in the views_distinct.info.yml file. But unfortunately, I did not see any difference in my view results. I still see multiple duplicative rows under the same group heading. Perhaps I'm missing something and not configuring it right; if so, please let me know. Thanks again.

jay.dansand’s picture

I've been looking over #2993688: Views relationships with multi-valued entity reference fields invalidate Distinct query option and there are a few potential patches that solve the problem differently. Given the (slow, but seemingly steady) progress over there, is there still interest in a D8/D9 version of this module?

If so, should the basis for that branch be #19, or does @colan have something going?

Sorry for the intermittent responses - sometimes I don't get notifications from d.o about issue activity; I don't know where the email goes, it doesn't seem to be in my spam folder. Anyway, I'm still potentially interested in getting an 8.9||9 branch going, though it seems I missed the important activity since I last made that statement.

colan’s picture

#21: The patch I wrote for the core issue works for me even though it won't work for everyone. I'd recommend concentrating efforts over there so that the process can be sped up. Otherwise, we're wasting time here, when this module won't actually be necessary in the long run.

Even if you can do just a little bit more work to produce a slightly better patch over there that works for you, it moves things along.

ericdsd’s picture

IMHO @colan is right, the root issue might be fixed in views itself as it not an edge case to me.

colan’s picture

Category: Feature request » Plan
Status: Active » Needs review

I'd like to see us deprecate this module for D8+ in favour of fixing the core issue (see above). Any objections? If not, let's go ahead with it.

liquidcms’s picture

@colan, i am little concerned with totally giving up on any work on this port for a couple reasons:

1. core issue isn't fixed and core fixes progress extremely slowly
2. the D7 version of this module did more than distinct based on id; i think it allowed you define any set of columns you wanted to define uniqueness.

nwom’s picture

StatusFileSize
new35.5 KB

Here is an updated patch that adds the version requirements for D9. I have yet to test out the functionality, but will report back once I do. I can also see the benefit in keeping this a module in D8/D9 when you want to avoid having to enable aggregation on the entire view.

nwom’s picture

StatusFileSize
new35.5 KB

Oops, forgot I had to add a "#" before the core info as well. Here is an updated patch.

nwom’s picture

Thank you so much @solariel! It worked perfectly for my use-case. For some reason I was getting duplicate media rows for specific languages. By filtering the ID, this no longer occurs. When attempting to aggregate by ID via views, for some reason it would result in only one row showing.

colan’s picture

@liquidcms wrote in #25:

i am little concerned with totally giving up on any work on this port for a couple reasons:
1. core issue isn't fixed and core fixes progress extremely slowly

While it may take a while to get committed, it's easy for any of us to work on patches/MRs there and apply them to our projects with Composer Patches. This at least moves things along in core, even though it takes a while for things to finalize.

2. the D7 version of this module did more than distinct based on id; i think it allowed you define any set of columns you wanted to define uniqueness.

Fair enough. How about just doing that here? It would keep this module leaner. So folks would need to:

  1. Apply the core patch, and
  2. Use this module to add the functionality that's missing from the core issue.

We need to keep folks working on the core issue, or it won't happen. While it's easier to fix things in contrib, it's not good for maintainability/sustainability, when it can otherwise be done in core.

If that makes sense, we can shrink the port here, which would make it easier to maintain. Thoughts?

nwom’s picture

As a update to the patch. On every page where a view is shown, there are countless errors added to Watchdog:

Cannot use view object to get third party settings.

This is the only issue I've found so far.

liquidcms’s picture

Just curious, is this the standard way to port a module from 7 to 8? Make a patch against the D7 module?

liquidcms’s picture

I think i worded by last question wrong.. what i really mean is there a way to get a D8 branch created with this patched version of D7 as a starting point; otherwise, as far as i know, its not possible to install this with composer.

nwom’s picture

@liquidcms: There is a trick to installing modules made for D8 on D9, and can be used in this case as well. You basically just need to add the repository to fetch the drupal module directly, and it will ignore the version.

Add this under repositories:

"drupal.views_distinct": {
            "type": "package",
            "package": {
                "name": "drupal/views_distinct",
                "version": "1.x-dev",
                "type": "drupal-module",
                "source": {
                    "url": "https://git.drupalcode.org/project/views_distinct.git",
                    "type": "git",
                    "reference": "7.x-1.x"
                }
            }
        }

This under require:

"drupal/views_distinct": "1.x-dev",

And this under extras> patches (requires https://github.com/cweagans/composer-patches):

            "drupal/views_distinct": {
                "#2609938-27: Port Views Distinct module to Drupal 8": "https://www.drupal.org/files/issues/2021-02-08/2609938-27.patch"
            },
liquidcms’s picture

@NWOM.. awesome.. trying it now.

colan’s picture

Or you could just become a maintainer and create the branch, etc. :)

thirstysix’s picture

StatusFileSize
new6.42 KB

Thanks @NWOM.
As per your suggestion it's working fine with Drupal 8.9.x. But, still i am facing duplication issue on the pager with view ajax. https://www.drupal.org/files/issues/2021-02-23/views_distinct.zip

liquidcms’s picture

@colan, that might be necessary.. :(

i thought this was working; but i think it breaks VBO. As soon as i enable any field to filter; the vbo checkboxes go missing. i think in view preview they are missing regardless - need to check if that is caused by this module being enabled. but when saved, the real view they are missing only if a filter is enabled.

colan’s picture

Assigned: colan » Unassigned

Sorry, I just realized this was still assigned to me. I forgot to unassign myself after finding the solution I went with in #18.

For those interested in taking this over, I'd contact Jay directly and fallback to Taking over unsupported (abandoned) projects if no response.

glynster’s picture

After trying all sorts of methods (including https://www.drupal.org/project/drupal/issues/2993688), we gave @ThirstySix #36 a go and it worked right away. We used this module a lot on Drupal 7 sites. Thanks ThirstySix!

liquidcms’s picture

@ThirtsySix's work is greatly appreciated; but not sure passing around .zip files is very manageable. But i think the issue is the project has possibly been abandoned. Anyone able to take over this project or at least request to become co-maintainer?

The guide on how to do this is here: https://www.drupal.org/docs/develop/managing-a-drupalorg-theme-module-or...

Would be great to get a proper D8 branch added here and get TS's zip committed and tagged.

SuperEngineer64’s picture

I installed it, but there is no aggregation option?

glynster’s picture

@SuperEngineer64 you do not have to use aggregation. Just edit one of the field you want to use as unique and select the Views Distinct filter.

@liquidcms agree about getting this pushed.

glynster’s picture

@NWOM I did not read your comment. I have installed and patched as you suggested. Much better and easier to maintain as you say. That said the pager is not filtering. Do you have any suggestions on how to fix?

AndyLicht’s picture

I´ve only installed it via filesystem. I get not effect by filtering/distinct "media: names". Is it necceassary to install it by composer?
I´m using D 8.9.17

snaphuman’s picture

#33 worked well as it filter results by configuring the selected field with the views_distinct module, however there still is an issue with the pagination. I'm currently working on Drupal 9.2.4

bluehead’s picture

#8 is a life saver and maintains pagers in order. In my case I had to modify the line where $id is defined to point at Taxonomy term.

kazah’s picture

Drupal 9.2.8
Doesn't work ... no aggregation option, only filtering duplicates.
I have node with a paragraph that has multiple fields.

Solved the problem by installing the module Views view field

natefollmer’s picture

@kazah, this module doesn't add any options to aggregation. When you open your field setting there will be a set of 'Views Distinct' options.

@NWOM - Have you found a solution to the Cannot use view object to get third party settings warnings? It makes it impossible to debug anything else because I'm getting so many of these warnings.

drupgirl’s picture

+1 for @NWOM - Have you found a solution to the Cannot use view object to get third party settings warnings?

Thanks for working on this!

hockey2112’s picture

#36 worked for me. I had to clear cache after installing and editing my View field. Thank you!

nwom’s picture

@natfollmer @drupgirl: Nope sadly didn't find a solution to the issue.

xjkwak’s picture

#9 worked for me. Thanks https://www.drupal.org/u/jorge-navarro!

grabby’s picture

Just a note of caution that I installed #36 and it worked, but the “Cannot use view object to get third party settings” warnings were relentless, so I decided to uninstall it. Unfortunately, to do so requires deleting the configurations of all views you used it on or created after enabling the module, whether or not you enabled its filtering!

dan_metille’s picture

@Grabby thanks for the note of caution, unfortunately too late for me as I already installed #36 on few websites.

Before exporting the configs at risk and uninstalling the module, I would like to ask if there is a possibility to fix that code?

mxwright’s picture

@sahaj @Grabby, since it's a custom module, it's easy to remove that warning from the code, but removing the config from all the views may be more difficult!

hockey2112’s picture

Any fix for the "Cannot use view object to get third party settings" warnings?

drupgirl’s picture

Yes, the fix is to manually comment out that section from the .module file.

function _views_distinct_field_settings_get($view, string $field_id, string $display_id = 'default') {
  /**
  if (!($view instanceof View) || !($view instanceof ViewUI)) {
    \Drupal::logger('views_distinct')->notice('Cannot use view object to get third party settings.');
  }
   */

  $third_party_settings = $view->getThirdPartySetting('views_distinct', 'settings');
thulenb’s picture

I am currently upgrading my D7 site to D9. My website uses quite a few complicated views with relationships, contextual filters and exposed filters and sorting. The core aggregation and distinct query features only work for less complex views. When developing my D7 site six years ago I remember that nothing was able to remove the duplicates besides Views Distinct Module. Now, several year later I am facing the exact same problem. Looking at the project page I recognized that there is no D8/9 version of this great module. I tried for days to find a solution, without success. Luckily I saw this issue and the ZIP file. This saved my D9 website. Of course, it feel not good - especially on a fresh site - to use a non-offical module version that might lead to downstream issues. However, for me this was the only solution. Hopefully, we get a D8/9/10 version in the future.

liquidcms’s picture

Yes, duplicates is a nightmare in D8+. I use a slew of different query_alters in code to solve most of my issues, sometimes aggregation works (but rarely). The port of this module i think breaks a lot of things like paging; so maybe only useful on certain views.

klemendev’s picture

aharown07’s picture

hockey2112’s picture

I just now used the version from #36 on a D10 website. Worked fine. Here's the contents of the .info file:

name: Views Distinct
type: module
description: Allows filtering "distinct" Views result rows based on arbitrary fields.
package: Views
core_version_requirement: ^8.8 || ^9 || ^10
dependencies:
  - drupal:views