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

Add support for Laravel-style relations #167

Open
wants to merge 58 commits into
base: develop
Choose a base branch
from

Conversation

bennothommo
Copy link
Member

@bennothommo bennothommo commented Feb 19, 2024

This PR allows people to define Laravel-style relations using methods that return Relation objects. So instead of:

public $belongsTo = [
     'user' => User::class,
];

You may use:

public function user(): BelongsTo
{
    return $this->belongsTo(User::class);
}

The main reason for this is to reduce friction for porting Laravel code over to Winter, and as a preference point for people who may wish to use the more declarative style of defining relations.

I've adjusted the unit tests to demonstrate this in action, and will be porting over the relation tests in the Winter system module into Storm, as they really should be in Storm anyway.

In order to maintain parity with the array-style configuration of relations in Winter, method-defined relations will need to either implement one of the following to indicate that they provide relations, and allow certain functionality to find them as required (for example, deleting related models when deleting a primary model):

  • The relation method must have a return type that correlates to a Relation class (like with the example above)
  • Alternatively, the method must use the Winter\Storm\Database\Attributes\Relation attribute, like so:
use Winter\Storm\Database\Attributes\Relation;

#[Relation]
public function user()
{
    return $this->belongsTo(User::class);
}

Todo

  • Fix attachment relations and tests
  • Add support for model traits with new format
  • Finish porting tests from Database Tester plugin in core
  • Full documentation of new syntax

We shouldn't test completely third-party code.
We no longer extend this class, and it doesn't seem to provide any functionality anyway.
This method is strictly a relationship method and should be grouped with the other relationship methods.
In order to maintain some semblance of parity with Laravel, we'll now use Laravel's relation constructors and simply overwrite the "new" relation methods to create Winter's relation instances.

Winter's relation name and defined constraints functionality will now be called in our own handler after the relation has been instantiated. This will allow us to review the relation directly before any Winter code runs.
This allows people using the relation method format to define the "delete" attribute available to relation definition arrays.
The "validateRelationArgs" method does a whole lot of nothing - the most it does is enforce required parameters. These can be picked up in Laravel anyway.
Added the ability to define the field name and use this as a constraint for attachment relations, separating it from the relation name.
The "Test" suffix might inadvertently be picked up by PHPUnit as a test case.
Copy link
Member

@LukeTowers LukeTowers left a comment

Choose a reason for hiding this comment

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

Still have performance issues with the softdelete trait

Instead of extending the Relation class, we will make the soft delete methods available immediately to the relations.

However, you can only *enable* soft delete if the related model uses the soft delete trait, thereby ensuring the correct soft delete functionality is available to the model when required.
@bennothommo
Copy link
Member Author

@LukeTowers I've changed the soft delete methods on the relation to be available at all times, but the actual soft delete flag will only be enabled when softDeletable() is called on the relation AND the related model has the SoftDelete trait.

src/Database/Dongle.php Outdated Show resolved Hide resolved
src/Html/Helper.php Outdated Show resolved Hide resolved
@LukeTowers
Copy link
Member

@bennothommo is there a related docs PR?

@bennothommo
Copy link
Member Author

@LukeTowers yep, here: wintercms/docs#176

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

Successfully merging this pull request may close these issues.

3 participants