[5.8] Fix chunkById when used after orWhere

[rgehan]

This PR fixes #29655

When using chunkById after orWhere, the condition is invalid, as it generates something like WHERE a OR b AND id > N, while it should be WHERE (a OR b) AND id > N.

---

This fix feels a tad inelegant, as I don't have much experience with how the query builder work internally.

I've tried using cloneWithout(['wheres']) to generate a new query without the original where clause, but it didn't work, and I haven't been able to find out why.

@staudenmeir you might have some ideas about how we could make that more elegant

[staudenmeir]

@morloderex I think the user shouldn't have to think about the constraints a method chunkById() adds to the query. Laravel already "fixes" queries with orWhere() when applying scopes.

[staudenmeir]

@rgehan I would leave it like this for now.

Eloquent uses a slightly different approach when applying scopes and we could share the code, but that would require breaking changes. I'll keep it in mind.

[rgehan]

@staudenmeir Can you point me to the logic that fixes ˋorWhereˋ when used with scopes?

I'm curious about how it's done

[taylorotwell]

I'm hesitant on this. It does not absolve the user of all responsibility to think about the query they are constructing. For example:

EloquentTestUser::where('id', 4)->orWhere('id', 1)->forPageBeforeId(15, 3)->orWhere('id', 3);

Even after this PR, this will still construct a query with probably unintended logic.

Without this PR, the proper query can be constructed by properly using logic grouping with Eloquent's existing features:

EloquentTestUser::where(function ($query) {
      $query->where('id', 4)->orWhere('id', 1);
})->forPageBeforeId(15, 3);

I think we do already force the user to think about this. We don't protect them from every logic error they can possibly make when building where clauses.

[rgehan]

@taylorotwell While I agree this might be confusing for ˋforPageAfterIdˋ, it also fixes the behavior of ˋchunkByIdˋ, which you wouldn't expect to be used between where/orWhere, but rather on its own, at the end of a call chain.

Don't you think this might be worth fixing?

[staudenmeir]

@rgehan I was referring to Eloquent\Builder::groupWhereSliceForScope() and createNestedWhere().

[halaei]

Don't` you think this might be worth fixing?

@rgehan I think it worth fixing that way. Just my opinion.