Kyslik / column-sortable

Package for handling column sorting in Laravel 5/6/7/8
MIT License
644 stars 105 forks source link

camelCased model relationship methods fail #90

Closed kyleweishaupt closed 6 years ago

kyleweishaupt commented 6 years ago

I'm having a small issue where the query order builder is attempting to check for a method in snake case when my model's relationships are defined in camel case. I'm new to Laravel but I believe that's the standard naming convention.

If I define a relationship using like clientType(), I get exception code 2 saying that 'client_type' is not a hasOne/belongsTo relation. Changing the method to client_type() works for column-sortable, but breaks the rest of my application. Since I'm using Boostrap-Vue and their table components, I don't have much of a sortable key choice, so I'm kind of stuck sorting by client_type.name for example.

I did manage to overwrite the trait, but wanted to share the issue here in case anyone else is experiencing this:

<?php

namespace App\Traits;

use Kyslik\ColumnSortable\Exceptions\ColumnSortableException;
use Kyslik\ColumnSortable\Sortable as ParentSortable;

/**
 * Sortable trait.
 */
trait Sortable
{
    use ParentSortable {
        ParentSortable::queryOrderBuilder as parentQueryOrderBuilder;
    }

    /*
     * Override query order builder to support camel case convention relationship names.
     */
    private function queryOrderBuilder($query, array $sortParameters)
    {
        // Convert the first part of a multi-part key to camel case
        if (!empty($sortParameters['sort'])) {
            $sortKey = explode('.', $sortParameters['sort']);

            if (count($sortKey) > 1) { 
                $sortKey[0] = camel_case($sortKey[0]);
                $sortParameters['sort'] = implode('.', $sortKey);
            }
        }

        $this->parentQueryOrderBuilder($query, $sortParameters);
    }
}

Not sure if this was ever intended but I'd appreciate being able to use both casing conventions without a wrapper.

Thanks for the excellent plugin by the way!

Kyslik commented 6 years ago

@kyleweishaupt I can not reproduce. Please see this diff where I changed the relationship to camelCase and it works as expected:

https://github.com/Kyslik/column-sortable-example/commit/3eecd7e160bfb42596f99b05b57f18cf666b5a74

Kyslik commented 6 years ago

@kyleweishaupt reopen if you want to discuss more.

kyleweishaupt commented 6 years ago

@Kyslik Thanks for taking a look into this, but to clarify, I had to use snake-casing for the input. client_type.name as opposed to clientType.name. (I'm trying to override the other plugin I'm using in order to provide camel case input to column-sortable)

Kyslik commented 6 years ago

I see now, I will do some more testing on this later this week, for now just use the solution you have :)

Kyslik commented 6 years ago

@kyleweishaupt I can not wrap my head around this one, If I declare my relationships camelCased package wants camelCased input, same goes for snake_cased relationship.

Whatever the name of relation is, package expects to be in input the name of the relationship, otherwise it will fail.

kyleweishaupt commented 6 years ago

That's alright - I'm working out an issue with the table plugin that's causing me to have a difference in the sorting key and the key for the relationship.

Thanks for looking into this! 👍