[DQT] Fix visit list filtering in candidate matching

[HenriRabalais]

Description

Fixes a bug where DQT filters did not apply visit list criteria when matching candidates. The getCandidateMatches() method was not passing the $visitlist parameter to buildQueryFromCriteria(), and the visit filtering logic in buildQueryFromCriteria() was incorrectly adding a duplicate session table join.

Changes

• src/Data/Query/SQLQueryEngine.php:
  • Pass $visitlist parameter to buildQueryFromCriteria() in getCandidateMatches() method
  • Remove duplicate session table join from buildQueryFromCriteria() - the session table is already joined by getFieldNameFromDict() when querying session-scoped fields
• modules/dataquery/php/query.class.inc: Remove FIXME comment and simplify visit list assignment since functionality is now verified

Testing Instructions

1. Navigate to the Data Query Tool
2. Select fields with session scope
3. Add a filter on a parameter and select only a subset of visits (e.g., only "V1" and "V2")
4. Run the query
5. Verify that results only include data from the specified visits
6. Compare with results when all visits are selected - should see different candidate counts

Related Issues

Resolves #10435

[driusan]

I don't understand this change.

My understanding from the PR description is that it was causing some kind of problem, but calling addTable with the same string multiple times should only add it once, so calling it a second time shouldn't change the results at all. That's the point of addTable.

If this is unnecessary because buildQueryFromCriteria is doing it, why is it only the addTable and not the rest of the duplicated logic that needs to be removed?

Since the addWhereClause on line 593 remains this code path should be making sure the table is added to the query before referencing it. Unless I'm missing something I think either the whole thing should be removed or none of it should be.

[HenriRabalais]

So the error I was getting was this:

[ERROR] SQLSTATE[42000]: Syntax error or access violation: 1066 Not unique table/alias: 's'

because addTable is being called in with slightly different ON clauses and treats them as separate table additions:

$this->addTable("LEFT JOIN session s ON (s.CandidateID=c.ID)"); from getFieldNameFromDict() in the imaging browser and biboank query engines.

$this->addTable("LEFT JOIN session s ON (s.CandidateID=c.ID AND s.Active='Y')"); from buildQueryFromCriteria()

So I figured I could just remove the second addTable because when visitlist is provided, we're filtering session-scoped fields, which means the session table is already joined by getFieldNameFromDict(). And that way the individual engines determine how the session should be joined. But the where clause below for visit filtering is still needed to actually filter based on the visit list.

If you prefer I can just update the imaging query engine and biobank query engine to include the s.Active='Y' part of the ON clause instead of removing this line.

[driusan]

I think them missing AND s.Active='Y' is a bug regardless of any SQL errors (we don't want cancelled/deleted sessions included in results), so yes, please update those query engines.