Skip to content

Commit 96e1a06

Browse files
luisdalmolinopencode
luisdalmolin
and
opencode
authored
Fix latestOfMany relationships using wrong foreign key in subqueries (#213)
* Added failing test * Fix latestOfMany relationships using wrong foreign key in subqueries When using leftJoinRelationship with latestOfMany relationships that have custom foreign keys, the generated subquery was incorrectly using the parent model's primary key instead of the relationship's local key. This caused wrong join conditions and incorrect query results. 🤖 Generated with [opencode](https://opencode.ai) Co-Authored-By: opencode <[email protected]> * Fix variable scope issue in latestOfMany subquery closures The previous fix was accessing $this->localKey inside nested closures where $this context was not available, causing "Call to a member function make() on null" errors in PostgreSQL tests. Fixed by extracting $this->localKey to a variable before passing it to the closures. 🤖 Generated with [opencode](https://opencode.ai) Co-Authored-By: opencode <[email protected]> * wip * Changed min stability to stable * Fixed code style --------- Co-authored-by: opencode <[email protected]>
1 parent 63f0aeb commit 96e1a06

File tree

9 files changed

+189
-7
lines changed

9 files changed

+189
-7
lines changed

‎AGENTS.md‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
# Agent Guidelines for Eloquent Power Joins
2+
3+
## Commands
4+
- **Test**: `composer test` or `vendor/bin/phpunit`
5+
- **Single test**: `vendor/bin/phpunit tests/JoinRelationshipTest.php` or `vendor/bin/phpunit --filter test_method_name`
6+
- **Test with coverage**: `composer test-coverage`
7+
- **Lint**: `composer lint` or `vendor/bin/php-cs-fixer fix -vvv --show-progress=dots --config=.php-cs-fixer.php`
8+
9+
## Code Style
10+
- **PHP Version**: 8.2+
11+
- **Framework**: Laravel 11.42+/12.0+ package
12+
- **Formatting**: Uses PHP-CS-Fixer with @Symfony rules + custom overrides
13+
- **Imports**: Use global namespace imports for classes/constants/functions, ordered alphabetically
14+
- **Arrays**: Short syntax `[]`, trailing commas in multiline
15+
- **Quotes**: Single quotes preferred
16+
- **Test methods**: snake_case naming with `@test` annotation
17+
- **Namespaces**: `Kirschbaum\PowerJoins` for src, `Kirschbaum\PowerJoins\Tests` for tests
18+
- **Type hints**: Use strict typing, compact nullable syntax `?Type`
19+
- **PHPDoc**: Left-aligned, no empty returns, ordered tags
20+
- **Variables**: No yoda conditions (`$var === 'value'` not `'value' === $var`)

‎composer.json‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -53,5 +53,5 @@
5353
]
5454
}
5555
},
56-
"minimum-stability": "dev"
56+
"minimum-stability": "stable"
5757
}

‎src/JoinsHelper.php‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,8 @@ protected function __construct()
4141

4242
public static function make($model): static
4343
{
44+
static::$instances ??= new WeakMap();
45+
4446
return static::$instances[$model] ??= new self();
4547
}
4648

‎src/Mixins/RelationshipsExtraMethods.php‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -298,21 +298,22 @@ protected function performJoinForEloquentPowerJoinsForHasMany()
298298
if ($isOneOfMany && !$hasCheck) {
299299
$column = $this->getOneOfManySubQuery()->getQuery()->columns[0];
300300
$fkColumn = $this->getOneOfManySubQuery()->getQuery()->columns[1];
301+
$localKey = $this->localKey;
301302

302-
$builder->where(function ($query) use ($column, $joinType, $joinedModel, $builder, $fkColumn) {
303-
$query->whereIn($joinedModel->getQualifiedKeyName(), function ($query) use ($column, $joinedModel, $builder, $fkColumn) {
303+
$builder->where(function ($query) use ($column, $joinType, $joinedModel, $builder, $fkColumn, $parentTable, $localKey) {
304+
$query->whereIn($joinedModel->getQualifiedKeyName(), function ($query) use ($column, $joinedModel, $builder, $fkColumn, $parentTable, $localKey) {
304305
$columnValue = $column->getValue($builder->getGrammar());
305306
$direction = Str::contains($columnValue, 'min(') ? 'asc' : 'desc';
306307

307308
$columnName = Str::of($columnValue)->after('(')->before(')')->__toString();
308309
$columnName = Str::replace(['"', "'", '`'], '', $columnName);
309310

310311
if ($builder->getConnection() instanceof MySqlConnection) {
311-
$query->select('*')->from(function ($query) use ($joinedModel, $columnName, $fkColumn, $direction, $builder) {
312+
$query->select('*')->from(function ($query) use ($joinedModel, $columnName, $fkColumn, $direction, $parentTable, $localKey) {
312313
$query
313314
->select($joinedModel->getQualifiedKeyName())
314315
->from($joinedModel->getTable())
315-
->whereColumn($fkColumn, $builder->getModel()->getQualifiedKeyName())
316+
->whereColumn($fkColumn, "{$parentTable}.{$localKey}")
316317
->orderBy($columnName, $direction)
317318
->take(1);
318319
});
@@ -321,7 +322,7 @@ protected function performJoinForEloquentPowerJoinsForHasMany()
321322
->select($joinedModel->getQualifiedKeyName())
322323
->distinct($columnName)
323324
->from($joinedModel->getTable())
324-
->whereColumn($fkColumn, $builder->getModel()->getQualifiedKeyName())
325+
->whereColumn($fkColumn, "{$parentTable}.{$localKey}")
325326
->orderBy($columnName, $direction)
326327
->take(1);
327328
}

‎tests/LatestOfManyJoinTest.php‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
<?php
2+
3+
namespace Kirschbaum\PowerJoins\Tests;
4+
5+
use Kirschbaum\PowerJoins\Tests\Models\Address;
6+
use Kirschbaum\PowerJoins\Tests\Models\RequestedAddress;
7+
8+
class LatestOfManyJoinTest extends TestCase
9+
{
10+
/** @test */
11+
public function test_left_join_relationship_with_latest_of_many_uses_correct_foreign_key()
12+
{
13+
// Create test data
14+
$address = Address::create([
15+
'kvh_code' => 'KVH123',
16+
'name' => 'Test Address',
17+
]);
18+
19+
RequestedAddress::create([
20+
'kvh_code' => 'KVH123',
21+
'requested_at' => now()->subDays(2),
22+
'status' => 'pending',
23+
]);
24+
25+
RequestedAddress::create([
26+
'kvh_code' => 'KVH123',
27+
'requested_at' => now()->subDay(),
28+
'status' => 'approved',
29+
]);
30+
31+
// This should generate a query that uses the correct foreign key relationship
32+
$query = Address::query()
33+
->leftJoinRelationship('latest_requested_address')
34+
->toSql();
35+
36+
// The issue: the generated query incorrectly uses addresses.id instead of addresses.kvh_code
37+
// in the subquery for latestOfMany
38+
// Expected: all joins should use kvh_code
39+
// Actual: subquery uses addresses.id instead of addresses.kvh_code
40+
41+
// This assertion will fail because the subquery incorrectly uses addresses.id
42+
$this->assertStringNotContainsString('"requested_addresses"."kvh_code" = "addresses"."id"', $query);
43+
}
44+
45+
/** @test */
46+
public function test_left_join_relationship_with_latest_of_many_returns_correct_data()
47+
{
48+
// Create test data
49+
$address = Address::create([
50+
'kvh_code' => 'KVH456',
51+
'name' => 'Test Address 2',
52+
]);
53+
54+
$oldRequest = RequestedAddress::create([
55+
'kvh_code' => 'KVH456',
56+
'requested_at' => now()->subDays(2),
57+
'status' => 'pending',
58+
]);
59+
60+
$latestRequest = RequestedAddress::create([
61+
'kvh_code' => 'KVH456',
62+
'requested_at' => now()->subDay(),
63+
'status' => 'approved',
64+
]);
65+
66+
// Execute the query
67+
$result = Address::query()
68+
->leftJoinRelationship('latest_requested_address')
69+
->where('addresses.kvh_code', 'KVH456')
70+
->first();
71+
72+
// This should work correctly if the join uses the right foreign key
73+
$this->assertNotNull($result);
74+
$this->assertEquals('KVH456', $result->kvh_code);
75+
}
76+
}

‎tests/Models/Address.php‎

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
<?php
2+
3+
namespace Kirschbaum\PowerJoins\Tests\Models;
4+
5+
use Illuminate\Database\Eloquent\Model;
6+
use Illuminate\Database\Eloquent\Relations\HasMany;
7+
use Illuminate\Database\Eloquent\Relations\HasOne;
8+
use Illuminate\Database\Eloquent\SoftDeletes;
9+
10+
class Address extends Model
11+
{
12+
use SoftDeletes;
13+
14+
protected $table = 'addresses';
15+
16+
protected $fillable = [
17+
'kvh_code',
18+
'name',
19+
];
20+
21+
public function requested_addresses(): HasMany
22+
{
23+
return $this->hasMany(RequestedAddress::class, 'kvh_code', 'kvh_code');
24+
}
25+
26+
/**
27+
* Get the latest requested address for this access address.
28+
*
29+
* @return HasOne<RequestedAddress, $this>
30+
*/
31+
public function latest_requested_address(): HasOne
32+
{
33+
return $this->requested_addresses()
34+
->one()
35+
->latestOfMany('requested_at');
36+
}
37+
}

‎tests/Models/RequestedAddress.php‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
<?php
2+
3+
namespace Kirschbaum\PowerJoins\Tests\Models;
4+
5+
use Illuminate\Database\Eloquent\Model;
6+
use Illuminate\Database\Eloquent\Relations\BelongsTo;
7+
8+
class RequestedAddress extends Model
9+
{
10+
protected $table = 'requested_addresses';
11+
12+
protected $fillable = [
13+
'kvh_code',
14+
'requested_at',
15+
'status',
16+
];
17+
18+
protected $casts = [
19+
'requested_at' => 'datetime',
20+
];
21+
22+
public function address(): BelongsTo
23+
{
24+
return $this->belongsTo(Address::class, 'kvh_code', 'kvh_code');
25+
}
26+
}

‎tests/TestCase.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ public function setUp(): void
1919

2020
protected function getPackageProviders($app)
2121
{
22-
return [PowerJoinsServiceProvider::class];
22+
return [
23+
PowerJoinsServiceProvider::class,
24+
];
2325
}
2426

2527
public function assertQueryContains(string $expected, string $actual, string $message = '', ?int $times = null): void

‎tests/database/migrations/2020_03_16_000000_create_tables.php‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,24 @@ public function up()
111111
$table->foreignId('tag_id');
112112
$table->morphs('taggable');
113113
});
114+
115+
Schema::create('addresses', function (Blueprint $table) {
116+
$table->increments('id');
117+
$table->string('kvh_code')->unique();
118+
$table->string('name');
119+
$table->timestamps();
120+
$table->softDeletes();
121+
});
122+
123+
Schema::create('requested_addresses', function (Blueprint $table) {
124+
$table->increments('id');
125+
$table->string('kvh_code');
126+
$table->timestamp('requested_at');
127+
$table->string('status')->default('pending');
128+
$table->timestamps();
129+
130+
$table->index('kvh_code');
131+
});
114132
}
115133

116134
/**

0 commit comments

Comments
 (0)