Skip to content

[10.x] Adds Tappable and Conditionable to Relation class - #50124

Merged
taylorotwell merged 3 commits into
laravel:10.xfrom
DarkGhostHunter:fix/relationship-conditionable
Feb 19, 2024
Merged

[10.x] Adds Tappable and Conditionable to Relation class#50124
taylorotwell merged 3 commits into
laravel:10.xfrom
DarkGhostHunter:fix/relationship-conditionable

Conversation

@DarkGhostHunter

@DarkGhostHunter DarkGhostHunter commented Feb 17, 2024

Copy link
Copy Markdown
Contributor

Fixes #50123

Since the Relation class forwards calls to the underlying Query instance, using tap, when and unless methods do not inherit the Relation methods.

For example, calling wherePivot('is_cool', true) after these methods will make the query set WHERE "table"."pivot" = 'cool" instead of using the wherePivot() method of the relation. This produces the Query instance to use its helpers disregarding anything else.

$tag->posts()->when(true)->wherePivot('is_cool', true)->get();
// Before
SELECT * FROM "posts" INNER JOIN "post_tag" ... WHERE "post_tag"."pivot" = 'is_cool'

// After
SELECT * FROM "posts" INNER JOIN "post_tag" ... WHERE "post_tag"."is_cool" = 1

By adding these traits directly into the Relation class itself, it fixes that problem.

Breaking changes?

None that is documented and could collision with this change.

The only edge case I can think of is someone receiving the Builder instance inside these methods callbacks, but that was deprecated in favor of the BuilderContract, so its a far stretch.

use Illuminate\Database\Eloquent\Builder;
// use Illuminate\Contracts\Database\Eloquent\Builder; <-- Should be using this instead.

$tag->post()->when(true, function (Builder $query) {
    // ...
});

@taylorotwell

Copy link
Copy Markdown
Member

Reverted due to multiple breaking changes.

@RTippin

RTippin commented Feb 21, 2024

Copy link
Copy Markdown

@taylorotwell Was just commenting to alert you all. My packages had their tests fail last night due to this change, as I rely on use Illuminate\Database\Eloquent\Builder; and was not aware of any deprecations in that regard or to use the interface as stated above, so long as we are still in 10.x

@DarkGhostHunter

Copy link
Copy Markdown
Contributor Author

Since all test passed, I thought it was OK. My mistake, but couldn't foresee the breaks. Except on the Builder contract, of course (which should be used when dealing with relation queries).

itinerare pushed a commit to itinerare/Alcyone that referenced this pull request Feb 26, 2024
This PR contains the following updates:

| Package | Type | Update | Change |
|---|---|---|---|
| [laravel/framework](https://laravel.com) ([source](https://github.com/laravel/framework)) | require | minor | `10.44.0` -> `10.45.1` |

---

### Release Notes

<details>
<summary>laravel/framework (laravel/framework)</summary>

### [`v10.45.1`](https://github.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10451---2024-02-21)

[Compare Source](laravel/framework@v10.45.0...v10.45.1)

-   Fix typehint for ResetPassword::toMailUsing() by [@&#8203;KKSzymanowski](https://github.com/KKSzymanowski) in laravel/framework#50163
-   \[10.x] Fix Process::fake() never matching multi-line commands by [@&#8203;SjorsO](https://github.com/SjorsO) in laravel/framework#50164

### [`v10.45.0`](https://github.com/laravel/framework/blob/HEAD/CHANGELOG.md#v10450---2024-02-20)

[Compare Source](laravel/framework@v10.44.0...v10.45.0)

-   \[10.x] Update `Stringable` phpdoc by [@&#8203;milwad-dev](https://github.com/milwad-dev) in laravel/framework#50075
-   \[10.x] Allow `Collection::select()` to work on `ArrayAccess` by [@&#8203;axlon](https://github.com/axlon) in laravel/framework#50072
-   \[10.x] Add `before` to the `PendingBatch` by [@&#8203;xiCO2k](https://github.com/xiCO2k) in laravel/framework#50058
-   \[10.x] Adjust rules call sequence by [@&#8203;driesvints](https://github.com/driesvints) in laravel/framework#50084
-   \[10.x] Fixes `Illuminate\Support\Str::fromBase64()` return type by [@&#8203;SamAsEnd](https://github.com/SamAsEnd) in laravel/framework#50108
-   \[10.x] Actually fix fromBase64 return type by [@&#8203;GrahamCampbell](https://github.com/GrahamCampbell) in laravel/framework#50113
-   \[10.x] Fix warning and deprecation for Str::api by [@&#8203;driesvints](https://github.com/driesvints) in laravel/framework#50114
-   \[10.x] Mark model instanse as not exists on deleting MorphPivot relation. by [@&#8203;dkulyk](https://github.com/dkulyk) in laravel/framework#50135
-   \[10.x] Adds Tappable and Conditionable to Relation class by [@&#8203;DarkGhostHunter](https://github.com/DarkGhostHunter) in laravel/framework#50124
-   \[10.x] Added getQualifiedMorphTypeName to MorphToMany by [@&#8203;dkulyk](https://github.com/dkulyk) in laravel/framework#50153

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box

---

This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy4yMTQuMCIsInVwZGF0ZWRJblZlciI6IjM3LjIxNC4wIiwidGFyZ2V0QnJhbmNoIjoiZGV2ZWxvcCJ9-->

Reviewed-on: https://code.itinerare.net/itinerare/Alcyone/pulls/5
Co-authored-by: Amadeus[bot] <amadeus@itinerare.net>
Co-committed-by: Amadeus[bot] <amadeus@itinerare.net>
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.

[10.x] Conditionable on a Belongs-To-Many query breaks it

3 participants