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

Cannot use exists validation without manually specifying connection name & fix multiple model support #96

Merged
merged 6 commits into from
Aug 18, 2021
Merged

Cannot use exists validation without manually specifying connection name & fix multiple model support #96

merged 6 commits into from
Aug 18, 2021

Conversation

danharrin
Copy link
Contributor

@danharrin danharrin commented Aug 17, 2021

At the moment, the connection name needs manually specifying when using the exists rule.

Rule::exists('sushi.Model');

This kinda defeats the object of passing the class as the table name, as it should resolve the connection automatically 😆

Thankfully, you just need to override the getConnectionName() method on the model.

Now, you can use:

Rule::exists('Model');

// or

Rule::exists(Model::class);

Copy link
Contributor

@mallardduck mallardduck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch, looks good.

@danharrin danharrin changed the title Cannot use exists validation without manually specifying connection name Cannot use exists validation without manually specifying connection name & fix multiple model support Aug 18, 2021
@danharrin
Copy link
Contributor Author

As per #88 - I've found a major issue with the approach made in that PR - only one Sushi model is supported at a time.

I've updated this PR to include the fix for this feature to work with multiple models.

@calebporzio
Copy link
Owner

Thanks @danharrin

@danharrin danharrin deleted the fix/exists-validation-without-connection-name branch August 18, 2021 16:54
@huyby
Copy link

huyby commented Sep 20, 2022

Edit: there's already a PR for this #98

This PR breaks Eloquent relationships defined in a Sushi model.

When Eloquent resolves a relationship, the related model will use the connection from the parent model, unless a connection name is defined explicitly in the related model itself. See Illuminate\Database\Eloquent\Concerns::newRelatedInstance()

As the parent model now returns the parent model class name as a connection name, the related model will also use this value and will not use the application's default connection.

namespace App\Models;

use Illuminate\Database\Eloquent\Model;


class Role extends Model
{
    use Sushi\Sushi;

   protected $rows = [
        [
            'id'          => 1,
            'name'    => 'admin',
        ],
   ];

    public function users()
    {
        return $this->hasMany(User::class);
    }
}

class User extends Model
{
}

Role::find(1)->users will fail as it will use Sushi's sqlite connection to execute the relationship query, instead of the applications default database connection.

I'll try to make a PR to resolve the issue, a bit short on time like everyone else 🙈

If anyone struggles with this, a quick fix is to define a connection name on the related model; or return the default application database connection name.

class User extends Model
{
    public function getConnectionName()
    {
        return config('database.default');
    }
}

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

Successfully merging this pull request may close these issues.

4 participants