lighthouse
lighthouse copied to clipboard
Add testConditionOnQueriedField for WhereHasConditionsDirective
- [x] Added or updated tests
- [ ] Documented user facing changes
- [ ] Updated CHANGELOG.md
See #1470
Changes
Added a test for this issue
Breaking changes None
Details I cannot seem to run any tests at all, I just keep getting a bunch of errors. Care to tell me how the tests can be setup locally?
I cannot seem to run any tests at all, I just keep getting a bunch of errors. Care to tell me how the tests can be setup locally?
https://github.com/nuwave/lighthouse/blob/master/CONTRIBUTING.md#how-to-contribute-to-lighthouse
Ok well I seem to have identified where the issue comes from, also explaining why there's no issue right now in Lighthouse after adding the test.
We are coupling players to teams with an linking table called player_team, the resulting query when using the whereCondition looks like:
select count(*) as aggregate
from `teams`
where EXISTS (
select * from `players`
inner join `player_team`
on `player_team`.`player_id` = `players`.`id`
where `teams`.`id` = `player_team`.`team_id`
AND (`ID` = 120)
)
Since it's literally just taking the enum value as the column for the condition we get an ambiguous error, because what it should be checking is AND (players.id = 120). I tried prefixing the column but the engine doesn't allow that, it does seem that this should be supported by default though. If I prefix the final AND clause myself and run it in my SQL editor it works fine.
@Jofairden so you are not using joins instead of plain Eloquent relations? Automatically prefixing column names when joins are used is a long standing issue within Laravel.
I got my hands dirty and tried to fix this before, but found that the rabbit hole goes quite deep: https://github.com/spawnia/laravel-framework/tree/prefer-original-table-columns-on-join
@Jofairden so you are not using joins instead of plain Eloquent relations? Automatically prefixing column names when joins are used is a long standing issue within Laravel.
I got my hands dirty and tried to fix this before, but found that the rabbit hole goes quite deep: https://github.com/spawnia/laravel-framework/tree/prefer-original-table-columns-on-join
The relationship that's being queried here is a hasManyThrough relationship, specifically if we look at players on team: (PlayerTeam = pivot)
public function players()
{
return $this->hasManyThrough(
Player::class,
PlayerTeam::class,
"team_id",
"id",
"id",
"player_id");
}
Anyways, so are you saying this is more or less an issue in Laravel and less so in Lighthouse / our app ? That'd be quite an issue for us
@Jofairden you can try and replicate this issue without Lighthouse using the query builder yourself. I believe that this would produce a similar error:
$team->players()->where('id', 123)
$team->players()->where('id', 123)
Yup I can confirm this now:
=> Illuminate\Database\Eloquent\Relations\HasManyThrough {#4223}
>>> Team::query()->find(1)->players()->where('id', 1)->first()
Illuminate/Database/QueryException with message 'SQLSTATE[23000]: Integrity constraint violation: 1052 Column 'id' in where clause is ambiguous (SQL: select `players`.*, `player_team`.`team_id` as `laravel_through_key` from `players` inner join `player_team` on `player_team`.`player_id` = `players`.`id` where `player_team`.`team_id` = 1 and `id` = 1 limit 1)'
Having the same for a polymorphic relation:
public function players()
{
return $this->morphedByMany(Player::class,
'participant',
'participants',
'fixture_id')
->withPivot('location');
}
When I prefix the first one it works, but the filters aren't doing this:
Team::query()->find(1)->players()->where(players.id', 1)->first()
Any ideas on that?