fisharebest / webtrees

Online genealogy
https://webtrees.net
GNU General Public License v3.0
418 stars 290 forks source link

Search via ‘Most common surnames’ incorrect #4989

Closed luenissla closed 3 weeks ago

luenissla commented 1 month ago

I have created a new small file with only eight people.

https://datenpool.bvff.de/tree/PD_Weltkriegsopfer_Wuppertal#

grafik

By clicking on a name under ‘Most common surnames’ https://datenpool.bvff.de/tree/PD_Weltkriegsopfer_Wuppertal/family-list?surname=TEPEL a person should actually be displayed. However, it is reported that there is no person with this name in the file.

grafik

A normal search for TEPEL finds the person.

grafik

grafik

I am using version 2.1.20 and had deactivated the Vesta modules to reproduce the error.

kiwi3685 commented 1 month ago

Good analysis. If you look at the url you show ((https://datenpool.bvff.de/tree/PD_Weltkriegsopfer_Wuppertal/family-list?surname=TEPEL) it includes the term "family-list", so that is what is happening. It only links to the Family list page, not Individuals.

That doesn't make sense to me either, but I don't know whether it is an error, or a deliberate choice.

I looked at the webtrees sites listed under the "Showcase" menu on webtrees.net. It shows an interesting set of changes.

In webtrees 1.7.x those links went always to the list of individuals. In webtrees 2.0.x that changed and both the Families list, and the Individuals list tables were displayed. In webtrees 2.1.x it changed again to just the Families list.

A strange progression of changes. I haven't yet found a discussion about the need for such changes (still searching). If there isn't any, then perhaps it is a bug. Hopefully not too hard to fix??

luenissla commented 1 month ago

If Webtrees only searches in the families, then it cannot find anything in this file. There are only eight individuals in this file, no families. But if Webtrees writes ‘Most common surnames’, then the names should also be searched for among the individuals, not just among the families. So it seems that a wrong function is being called ;-)

fisharebest commented 1 month ago

The code is designed so that it will use a custom "individual list" module if one is installed.

However, the family list is implemented as a "custom individual list", and this is being detected first.

kiwi3685 commented 1 month ago

Thanks Greg. I can see where this has been managed in other places, by changing :

->first(static fn (ModuleInterface $module): bool => $module instanceof IndividualListModule);

to:

->first(static function (ModuleInterface $module): bool {
// The family list extends the individual list
return
    $module instanceof IndividualListModule
        && !$module instanceof FamilyListModule
    ;
});

This change looks to be needed in: IndividualRepository.php (L:591) FamilyTreeStatisticsModule.php (L:154)

But it still seems to leave the HTML block template (AllTags) with various versions of #commonSurnames" etc still pointing to the family-list. I must be missing a file somewhere. Any suggestions?

fisharebest commented 4 weeks ago

This code is inconsistent in many places - which is why I wrote the comment instead of simply fixing it.

Since the core modules are loaded alphabetically, some code uses ->last() to get the individual list and ->first() to get the family list.

Other code fails to use the ModuleService::findByComponent() to filter by access controls.

As well as tidying these up, I think a good solution might be to change the family/individual lists to extend an abstract base class (or use a trait to share code) instead of one extending the other.

luenissla commented 3 weeks ago

Thanks Greg.