thedevdojo / voyager

Voyager - The Missing Laravel Admin
https://voyager.devdojo.com
MIT License
11.78k stars 2.67k forks source link

Issue with Relationships with column names not lowercase #4850

Open johncarlson21 opened 4 years ago

johncarlson21 commented 4 years ago

Version information

Description

Trying to create a hasMany Relationship. choose all the correct fields, but showing now results. Tables: orders, order_items Columns: OrderID, OrderID

Steps To Reproduce

Steps to reproduce the behavior:

  1. Go to Database
  2. Edit Browse
  3. Create hasMany relationship selecting
  4. Save Relationship
  5. View Bread for an Order

Expected behavior

The View should show order items under the order

Additional context

The problem here is that when it tries to get the data in the view for the relationship, all the column names are lowercase, so when it tries to create the query, it is using orderid as the column name instead of the actual column name of OrderID

example of relationship data from DB: { "model": "App\OrderItem", "table": "order_items", "type": "hasMany", "column": "orderid", "key": "orderid", "label": "sku", "pivot_table": "contents", "pivot": "0", "taggable": "0" }

MrCrayon commented 4 years ago

You could probably rename columns since database columns are case insensitive and it's usually a terrible idea to use uppercase letters, but I understand if it's an existent project might be an hassle. Not sure where and if we can keep the original column name, I would have to check it.

As work around you can convert the name in your model:

public function __get($name)
{
    if ($name == "orderid") {
        $name = "OrderID";
    }

    return parent::__get($name);
}
johncarlson21 commented 4 years ago

I would have to do this with like a hundred columns over about 4 tables. as I said I would have to change all the column names so this would be a process either way to do this. as for your statement of columns being case insensitive, obviously that is not a true statement as this is the problem I'm having. somewhere in your code, it is changing the column names to lowercase. I haven't had any issues with my actual models because I'm telling it what I'm looking for. so somewhere in the voyager code it is converting the column names when it creates the bread.

MrCrayon commented 4 years ago

Database columns are case insensitive, object properties and methods aren't. That's why is bad practice to use capital letters in databases since it doesn't change anything but can cause other problems.

I don't think Voyager lowecase anything it might be a problem with Doctrine\DBAL that is used to get column names.

You could change all columns in database to lowercase with a simple query:

SELECT
CONCAT('ALTER TABLE ', TABLE_NAME, ' CHANGE `', COLUMN_NAME, '` `', LOWER(COLUMN_NAME), '` ', COLUMN_TYPE, ';') 
AS columnNameToLower 
FROM INFORMATION_SCHEMA.COLUMNS
WHERE
TABLE_SCHEMA = 'your-database-name'
AND COLUMN_NAME REGEXP BINARY '[A-Z]+';

This query is not going to change anything but will output the queries you need to run to change columns to lowercase.

Of course if you want to try make a backup of your database first.

MrCrayon commented 4 years ago

Or you can add in database config file:

PDO::ATTR_CASE => PDO::CASE_LOWER,

to options in mysql connection.

adrianloh128 commented 4 years ago

same here, stuck for a day because couldn't find the bug. The column under relationship section is all lowered case. any fix on the BREAD other than update the column name on database?

bhaskarkishor commented 4 years ago

Any update here, same issue as voyager is converting the column name to lowercase while creating the relationship....

MrCrayon commented 4 years ago

@adrianloh128

The column under relationship section is all lowered case. any fix on the BREAD other than update the column name on database?

Right in the comment before yours

tunapiq commented 3 years ago

There might be some cases where it makes sense to name a single column all in uppercase, moreover, it feels a bit restrictive to not be able to use uppercase attribute names in your migration.

-Part 1 I found the problem to be linked to this: TCG\Voyager\Database\Schema\SchemaManager::describeTable($tableName)

-Part 2 There is a call to $table->columns in the body of that function and what it does is basically this if you trace the code: DB::connection()->getDoctrineSchemaManager()->listTableColumns($tableName)

The code in part 2 above seems to always return the database column names in lower case.

-Part 3 The voyager source code does not take this into consideration. For example, in TCG\Voyager\Http\Controllers\VoyagerBaseController::relation(Request $request) there is a line that compares the attributes of a model i.e. its columns (after it has already been converted to a collection) to the attributes generated by the code in part 2.

If a model has an attribute that is all uppercase and you convert the model to a collection, the attributes of the collection will be uppercase and will be case sensitive if you try to access it.

Here is a snippet from the body of the function in part 3:

foreach ($relationshipOptions as $relationshipOption) {
    $results[] = [
        'id'   => $relationshipOption->{$options->key},
        'text' => $relationshipOption->{$options->label},
    ];
}

$relationshipOptions is a collection that possibly contains items with uppercased attributes, $options->key and $options->label are values derived from the code in part 2 (if you trace where they are coming from).

Hopefully, this helps whoever might want to implement a fix for this locally.