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

createOrFirst is not triggering creating nor created model events #2935

Closed
prwnr opened this issue May 7, 2024 · 8 comments
Closed

createOrFirst is not triggering creating nor created model events #2935

prwnr opened this issue May 7, 2024 · 8 comments

Comments

@prwnr
Copy link

prwnr commented May 7, 2024

  • Laravel-mongodb Version: 4.3.0
  • Laravel version: 11.6.0
  • PHP Version: 8.2.2

Description:

There seem to be a problem, where using the createOrFirst builder method, that it does not trigger the creating or created model events when the model is being created (not retrieved, as there is no matching model found).

This seems to be caused by the fact that the createOrFirst method is overwritten in MongoDB package, but it is not calling the $model->save() method after an attempt to find the model, which would trigger those two events.

This is how it behaved in the past (I just upgraded from v3 to v4) and it is how it behaves in Laravel without MongoDB.

Steps to reproduce

  1. set up a simple Model class
  2. call Model::query()->createOrFirst() on it
  3. see that it won't trigger the creating and created event if it recently created the model (there was no matching model for the attributes)

This will however trigger the retrieving event instead - even if the model was recently created, but not actually "retrieved"

Expected behaviour

When a model is being created as a new one with use of createOrFirst method, it should trigger the creating and created model events.

Actual behaviour

When a model is being created as a new one with use of createOrFirst method, there is no creating nor created events dispatched.

Workaround

At the moment the only workaround I could think of is to manually try to fetch the model first with use of find/where query, and if its not found, create it with use of Model::query()->create() method, or $model->save() - which both will dispatch the creating and created events.

@GromNaN
Copy link
Member

GromNaN commented May 7, 2024

Hello, thanks for the feedback. I'm tracking it in Jira PHPORM-180

You are right that this events are not triggered because newModelInstance is used. It might be possible to explicitly trigger this events in MongoDB\Laravel\Eloquent::createOrFirst.

I'm curious, what do you do with these events?

@GromNaN GromNaN added the bug label May 7, 2024
@jeromegamez
Copy link
Contributor

jeromegamez commented May 8, 2024

Thank you @prwnr for putting this into better words than I was preparing until I saw your post 😅

I'm facing the same issue when using models that use another field than _id as primary key, like so:

<?php

declare(strict_types=1);

namespace App\Models;

use Illuminate\Database\Eloquent\Concerns\HasUuids;
use MongoDB\Laravel\Eloquent\Model as MongoDBModel;

abstract class UuidModel extends MongoDBModel
{
    use HasUuids;

    protected $primaryKey = 'id';

    public function uniqueIds()
    {
        return ['id'];
    }
}

or when using a trait

<?php

declare(strict_types=1);

namespace App\Models\Traits;

use Ramsey\Uuid\Uuid;

trait HasUuid
{
    public static function bootHasUuid(): void
    {
        static::creating(static function (self $model): void {
            // Automatically generate a UUID if using them, and not provided.
            if (empty($model->{$model->getKeyName()})) {
                $model->{$model->getKeyName()} = Uuid::uuid4()->toString();
            }
        });
    }
}

This stopped working with release 4.2, my current workaround is staying with release 4.1.3

@prwnr
Copy link
Author

prwnr commented May 8, 2024

@GromNaN We are using the creating event to set our tenancy fields, as we need them always set upon creation to prevent any model from being created without them. The creating event
appeared to be the best fit for this purpose in the lifecycle of a model when we wrote this code several years ago.

We also use it in a similar manner as @jeromegamez suggested, setting a UUID on the id field during creation.

I've also run some additional tests and I can see that saving and saved events are also not being triggered when using firstOrCreate method that was overwritten by MongoDB - however they are both being dispatched when using the regular create method.

This itself will also be causing problems with Sluggable package that we are using in our application, that relies on those two events.

@GromNaN
Copy link
Member

GromNaN commented May 28, 2024

Could you check this PR that should fix your issue: #2980

@prwnr
Copy link
Author

prwnr commented May 29, 2024

@GromNaN I'm sorry, but I'm failing to install your fork's branch, not sure why, but probably I am doing something wrong as Im installing fork for the first time.

Added your repo to repositories list as VCS and set package version to your branch "dev-PHPORM-180 as 4.3.0" and it can't find it Your requirements could not be resolved to an installable set of packages - tried some other variation of the branch name, or master branch, or changing repository type from VCS to git, but nothing works for me. Most likely some dumb thing I am missing.

@GromNaN
Copy link
Member

GromNaN commented May 29, 2024

I would have gone through the same process as you did. Alternatively, you can install from source

composer reinstall --prefer-source mongodb/laravel-mongodb

Add the remote:

cd vendor/mongodb/laravel-mongodb
git remote add gromnan https://github.com/GromNaN/laravel-mongodb-fork.git

And checkout the branch

git fetch gromnan
git checkout PHPORM-180

@GromNaN
Copy link
Member

GromNaN commented May 30, 2024

Fixed by #2984 in an other way.

@GromNaN GromNaN closed this as completed May 30, 2024
@jeromegamez
Copy link
Contributor

Thank you @GromNaN! 🙏🏻

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

Successfully merging a pull request may close this issue.

3 participants