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.

Sign up for GitHub

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

newPivot method is the same as Laravel #185

Closed
wants to merge 8 commits into from
Closed

newPivot method is the same as Laravel #185

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
71ae70b
newPivot method is the same as Laravel
mjauvin Jul 30, 2024
eeda95b
remove overriden Pivot class as well
mjauvin Jul 30, 2024
f7891e4
forgot ::class suffix
mjauvin Jul 30, 2024
059e20e
remove phpstan ignore rule
mjauvin Jul 30, 2024
f2dd375
remove unused rule
mjauvin Jul 30, 2024
e9df5f7
parent method accepts array of ids
mjauvin Jul 30, 2024
879811f
Revert "remove overriden Pivot class as well"
mjauvin Jul 30, 2024
1f9ecaf
use Storm's pivot model
mjauvin Jul 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 0 additions & 9 deletions phpstan-baseline.neon
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,10 +65,6 @@ parameters:
count: 1
path: src/Database/Model.php

-
message: "#^Return type \\(Winter\\\\Storm\\\\Database\\\\Pivot\\) of method Winter\\\\Storm\\\\Database\\\\Model\\:\\:newPivot\\(\\) should be compatible with return type \\(Illuminate\\\\Database\\\\Eloquent\\\\Relations\\\\Pivot\\) of method Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:newPivot\\(\\)$#"
count: 1
path: src/Database/Model.php

-
message: "#^Static property Illuminate\\\\Database\\\\Eloquent\\\\Model\\:\\:\\$dispatcher \\(Illuminate\\\\Contracts\\\\Events\\\\Dispatcher\\) in isset\\(\\) is not nullable\\.$#"
Expand All @@ -90,11 +86,6 @@ parameters:
count: 1
path: src/Database/MorphPivot.php

-
message: "#^Parameter \\#1 \\$ids of method Winter\\\\Storm\\\\Database\\\\Pivot\\:\\:newQueryForRestoration\\(\\) expects array\\<int\\|string\\>\\|string, int given\\.$#"
count: 1
path: src/Database/MorphPivot.php

-
message: "#^Parameter \\#2 \\$string of function explode expects string, int given\\.$#"
count: 1
Expand Down
16 changes: 0 additions & 16 deletions src/Database/Model.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -822,22 +822,6 @@ public function offsetExists($offset): bool
// Pivot
//

/**
* Create a generic pivot model instance.
* @param \Winter\Storm\Database\Model $parent
* @param array $attributes
* @param string $table
* @param bool $exists
* @param string|null $using
* @return \Winter\Storm\Database\Pivot
*/
public function newPivot(EloquentModel $parent, array $attributes, $table, $exists, $using = null)
{
return $using
? $using::fromRawAttributes($parent, $attributes, $table, $exists)
: Pivot::fromAttributes($parent, $attributes, $table, $exists);
}

/**
* Create a pivot model instance specific to a relation.
* @param \Winter\Storm\Database\Model $parent
Expand Down
2 changes: 1 addition & 1 deletion src/Database/MorphPivot.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,7 @@ public function newQueryForRestoration($ids)
}

if (!str_contains($ids, ':')) {
return parent::newQueryForRestoration($ids);
return parent::newQueryForRestoration([$ids]);
Copy link
Member

Choose a reason for hiding this comment

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

This matches Laravel's code exactly, so I'm fine not making this change. Really what should be changed is the docblock in Laravel's code.

}

$segments = explode(':', $ids);
Expand Down