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

Enable the usage of an alias for a column name when using Sorts #164

Merged
merged 5 commits into from
Feb 27, 2019

Conversation

dominikb
Copy link
Contributor

This PR addresses #163

I'm not too fond of the solution here

if (is_string($alias)) {
    return Sort::field(ltrim($alias, '-'), ltrim($sort, '-'));
}
return Sort::field(ltrim($sort, '-'));

Maybe someone can come up with a more readable solution?

@AlexVanderbist
Copy link
Member

Hey, thanks for another PR! Am I missing something or are there some changes to the Sort class missing?

@AlexVanderbist
Copy link
Member

I've added the missing code in Sort.

About that is_string clause that you don't like; I wouldn't mind just dropping the allowedSorts(['sort' => 'alias']) array notation.
It's not supported for filters either. If you want to use any aliases you'll have to use the Filter and Sort classes which will be type hinted. This way there's no "magic" array syntax where you're unsure about whether the key or the value will be used as an alias, etc...

Lmk what you think :)

@dominikb
Copy link
Contributor Author

Hey, thanks for the help there. I somehow forgot to commit half of my files 🤕

I like it better to use the actual Sort classes when aliasing a column. As you mentioned, it is more expressive in what it actually does and can be comprehended easier.

Thus I've decided to drop the array syntax and remove tests for it.

I've also updated the documentation to reflect theses adjustments.

@AlexVanderbist AlexVanderbist merged commit ead49ef into spatie:master Feb 27, 2019
AlexVanderbist pushed a commit that referenced this pull request Feb 27, 2019
* allow for alias on sort

* Fix tests and add column name to sort class

* drop array syntax for aliasing allowed sorts
adjust documentation

* drop test for array syntax

* add hyperlink to readme
@dominikb dominikb deleted the allow-alias-on-sort branch February 17, 2022 09:07
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.

2 participants