Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show all individuals gives zero results #4700

Closed
JustCarmen opened this issue Jan 1, 2023 · 10 comments
Closed

Show all individuals gives zero results #4700

JustCarmen opened this issue Jan 1, 2023 · 10 comments

Comments

@JustCarmen
Copy link
Contributor

See: https://www.webtrees.net/index.php/en/forum/help-for-release-2-1-x/37602-list-of-individuals#96825

When I try to load all individuals, I get zero results. Tested this in webtrees 2.1.14 and 2.1.15. In webtrees 2.1.13 it works as expected.

Multiple users confirm this error.

@Fa10175
Copy link

Fa10175 commented Jan 8, 2023

same problem

@jon48
Copy link
Contributor

jon48 commented Jan 9, 2023

I think commits 52288ec & 843973c are introducing the issue (the first one already introduces some conditions when that can happen, and the second one makes it more general).

The problem is that it creates a condition tree where it will always add a WHERE condition to the query.

if ($surnames === []) {
// SURN, with no surname
$query->where('n_surn', '=', $surname);
} else {
$query->whereIn($this->binaryColumn('n_surname'), $surnames);
}

In the case when we want to select all individuals, we have the inputs:

  • $surname = ''
  • $surnames can be a but more tricky, as it can be [] or [""] depending on the data in the tree.

In all cases, that will add either WHERE n_surn = "" or WHERE n_surname IN ("").
That can return results (for instance in the Royal Family demo tree), but not what is expected, as there should not be condition at all when show_all=yes and show_all_firstnames=yes.

I will not dare going further in terms of recommendation to fix, as this is quite a sensitive logic, and I am not sure that what I have in mind will not introduce another issue. Basically, is it as simple as not adding any query->where when $surname === '' (I don't know if '' is considered a valid surname to query) ?

@jon48
Copy link
Contributor

jon48 commented Jan 9, 2023

Having looked a bit further at the logic, this is actually a bit more subtle, and my first analysis is incomplete, and had not taken into account the new philosophy of the refactoring. I thought that $surname was the primary argument (and $surnames only a secondary parameter), but if my new understanding is correct, the refactored IndividualListModule::individuals is supposed to work as long as surnames are provided in $surnames, independently of the value of $surname, and in that case, the if/else condition makes sense.

This is actually earlier in the code that the logic is causing an issue.
Lines 362-383 are actually where the future $surnames is populated (with all surnames when $surname = '', for instance), and I believe that the intention is to pass those surnames to IndividualListModule::individuals.

However, and this is what lead me to believe that $surname was the important argument, the logic passing the argument is flawed :

'individuals' => $this->individuals($tree, $surname, array_keys($all_surnames[$surname] ?? []), $falpha, $show_marnm === 'yes', false),

If it OK when a surname has been defined, but when there are none, then $all_surnames[$surname] will return either an empty array or a very limited number of entries, not taking into account the surname list computed in l. 371:

$surns = array_filter($all_surnames, static fn (string $x): bool => $x !== '' && $x !== Individual::NOMEN_NESCIO, ARRAY_FILTER_USE_KEY);

I think that the fix is to flatten the $surns array when $surname === '', with something similar to the following instead of L458-471 :

if ($show === 'indi') {
    // Build the surname list
    // If no reference surname, then flatten the list of surnames calculated earlier
    if($surname === '') {
        $surnames = [];
        array_walk_recursive($surns, function($c, $s) use (&$surnames) { $surnames[] = $s; });   // Is it the best way to flatten ?
    }
   // Otherwise, use the surnames linked to the reference surname. That is what was done before.
   else {
        $surnames = array_keys($all_surnames[$surname] ?? []);
    }

    if ($families) {
        echo view('lists/families-table', [
            'families' => $this->families($tree, $surname, $surnames, $falpha, $show_marnm === 'yes'),
            'tree'     => $tree,
        ]);
    } else {
        echo view('lists/individuals-table', [
            'individuals' => $this->individuals($tree, $surname, $surnames, $falpha, $show_marnm === 'yes', false),
            'sosa'        => false,
            'tree'        => $tree,
        ]);
    }
}

@Fa10175
Copy link

Fa10175 commented Jan 10, 2023

i just try your tip for line 458-471
for me it's ok
thanks Jon48

@jon48
Copy link
Contributor

jon48 commented Jan 10, 2023

I think that the flattening step still needs some tweaking or an alternative: for a start, I find it ugly 😅, the recursive mode looks wrong to me, and it still seems to add individuals with no surname to the list of individuals whose surname starts by a letter (and I don't think that should happen).

I cannot look at it right now, but array_walk_recursive($surns, function($c, $s) use (&$surnames): void { if($s !== '') $surnames[] = $s; }); looks slightly better for the last concern, or, using Collections, $surnames = collect($surns)->map(static fn ($item, $key) => array_keys($item))->flatten()->filter(static fn($s): bool => $s !== '')->toArray();

@Fa10175
Copy link

Fa10175 commented Jan 17, 2023

Because i have same problem with webtrees 2.1.16
I try our 3 tips and i don't found any problem
I can't tell you which one is better but they work for me
now I wait for the next version with this correction
Thanks

jon48 added a commit to jon48/webtrees that referenced this issue Jan 29, 2023
jon48 added a commit to jon48/webtrees that referenced this issue Jan 29, 2023
@Fa10175
Copy link

Fa10175 commented Aug 19, 2023

I update to 2.1.17
and i have again the problem
Sans titre

despite the code modification suggested by jon48 which worked in 2.1.16 the result gives zero records for the "All" selection.
Please let me know how to correct this problem.

@reteP-riS
Copy link

It seems that the query always returns only those individuals which have one name record (regular, _AKA, _MARNM) without a surname surrounded by slashes!

@JustCarmen
Copy link
Contributor Author

I've found a workaround to list all individuals in 2.1.17. There is something wrong with the where clause of the database query.

In the file app/Module/IndividualListModule.php in the function Individual disable line 709:

if ($surnames === []) {
    // SURN, with no surname
    //$query->where('n_surn', '=', $surname);
} else {
    $query->whereIn($this->binaryColumn('n_surname'), $surnames);
}

If show_all = true, $surnames is empty. So in that case, the where clause narrows the selection while it shouldn't. But what I don't understand is why it does work on the demo site. So this is not a permanent solution, but it works for me and this might work in your case too.

@fisharebest
Copy link
Owner

I think also there is an issue where $surnames is [''] instead of [].

But thanks for your comments and investigation.

As soon as I have a fix for this bug, I'll create a new release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants