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

loadCount Incorrectly Triggers retrieved Event for Parent Model #51276

Closed
AmRo045 opened this issue May 3, 2024 · 3 comments
Closed

loadCount Incorrectly Triggers retrieved Event for Parent Model #51276

AmRo045 opened this issue May 3, 2024 · 3 comments

Comments

@AmRo045
Copy link

AmRo045 commented May 3, 2024

Laravel Version

11.6.0

PHP Version

8.2.4

Database Driver & Version

mysql

Description

I have observed an unexpected behavior with the loadCount method. The loadCount method is intended to load the count of related models without firing the retrieved event on the parent model, similar to how withCount operates. However, it appears to be triggering the retrieved event for the parent model with invalid data. The following is the log for the retrieved event for Post and Comment model:

[2024-05-03 07:50:29] local.INFO: Retrieved model:  {"id":1,"title":"Commodi voluptatem hic rem iure consectetur temporibus aut.","body":"Itaque magnam ea quo nulla et. Minima vel quo cupiditate culpa rerum. Et iusto enim dolorum. Explicabo dolor mollitia officia in ea.","created_at":"2024-05-03T07:33:54.000000Z","updated_at":"2024-05-03T07:33:54.000000Z"} 
[2024-05-03 07:50:29] local.INFO: Retrieved model:  {"id":1,"comments_count":3} 

which triggers from this route:

Route::get('/', function () {
    $post = \App\Models\Post::first();
    $post->loadCount('comments');

    dd('DONE! Checkout the storage/logs/laravel.log file');
});

the Post model retrieved event handler:

protected static function booted(): void
{
    static::retrieved(function($model) {
        info('Retrieved model: ', $model->toArray());
    });
}

Steps To Reproduce

  1. Clone this repository: https://github.com/AmRo045/laravel-bug-report-load-count and set up the basics.
  2. Run the migrations.
  3. Create a fake post with some comments using tinker and the following snippet:
\App\Models\Post::factory()->hasComments(3)->create();
  1. Start the server and navigate to the home page.
  2. Check the storage/logs/laravel.log file.
Copy link

github-actions bot commented May 6, 2024

Thank you for reporting this issue!

As Laravel is an open source project, we rely on the community to help us diagnose and fix issues as it is not possible to research and fix every issue reported to us via GitHub.

If possible, please make a pull request fixing the issue you have described, along with corresponding tests. All pull requests are promptly reviewed by the Laravel team.

Thank you!

@AmRo045
Copy link
Author

AmRo045 commented May 10, 2024

There is a workaround for this issue that involves disabling events for the model, but I'm not sure if it's the right approach:

public function loadAggregate($relations, $column, $function = null)

/**
 * Load a set of aggregations over relationship's column onto the collection.
 *
 * @param  array<array-key, (callable(\Illuminate\Database\Eloquent\Builder): mixed)|string>|string  $relations
 * @param  string  $column
 * @param  string|null  $function
 * @return $this
 */
public function loadAggregate($relations, $column, $function = null)
{
    if ($this->isEmpty()) {
        return $this;
    }

    $firstItem = $this->first();

    return $firstItem::withoutEvents(function() use ($relations, $column, $function, $firstItem) {
        $models = $firstItem->newModelQuery()
            ->whereKey($this->modelKeys())
            ->select($this->first()->getKeyName())
            ->withAggregate($relations, $column, $function)
            ->get()
            ->keyBy($this->first()->getKeyName());

        $attributes = Arr::except(
            array_keys($models->first()->getAttributes()),
            $models->first()->getKeyName()
        );

        $this->each(function ($model) use ($models, $attributes) {
            $extraAttributes = Arr::only($models->get($model->getKey())->getAttributes(), $attributes);

            $model->forceFill($extraAttributes)
                ->syncOriginalAttributes($attributes)
                ->mergeCasts($models->get($model->getKey())->getCasts());
        });

        return $this;
    });
}

@driesvints
Copy link
Member

As Taylor indicated on the related PR, this is expected as an actual query is executed.

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

No branches or pull requests

3 participants