aarondfrancis / fast-paginate

A fast implementation of offset/limit pagination for Laravel.
MIT License
1.21k stars 56 forks source link

Wrap column when looking for innerSelectColumns #22

Closed j3j5 closed 2 years ago

j3j5 commented 2 years ago

This fixes #21

aarondfrancis commented 2 years ago

@j3j5 lovely, thank you so much! Could you add a test to make sure I never undo this 🙈

j3j5 commented 2 years ago

:thinking: Interesting, apparently this breaks the count. Let me give it another go later to see why this breaks but it doesn't take the other column. I'll try to add an extra test for this.

j3j5 commented 2 years ago

Ok, found the issue, I started by wrapping the column name on the str_contains function but actually, when the column is an instance of the Expression class, the call to getValue() returns a string tha is already wrapped, so it needed to be above. I've added a few tests but please, review them to make sure I didn't mess up anything on there.

One more thing, I saw that the github actions did not include PHP 8.1, is there a reason? I run all the tests on PHP 8.1

aarondfrancis commented 2 years ago

Wonderful job! And thank you for writing all of those tests, that's extremely helpful.

One more thing, I saw that the github actions did not include PHP 8.1, is there a reason? I run all the tests on PHP 8.1

Nope, just an oversight. Will add it!

j3j5 commented 2 years ago

Awesome! Glad to be of help!