laravel-ide-helper icon indicating copy to clipboard operation
laravel-ide-helper copied to clipboard

Ability to save Eloquent model's database columns in alphabetical order

Open lukrak opened this issue 4 years ago • 3 comments

Summary

Currently in ModelsCommand.php when getting properties from table, it's adding columns in same order that columns are saved in database.

src/Console/ModelsCommand.php
protected function getPropertiesFromTable($model)
{
    ...
    $columns = $schema->listTableColumns($table, $database);
    ...
}

This means that if there are multiple people working on the same project, it's quite likely that each person's database might have slightly different column order in its tables. This happens if two people are adding new columns to the same table and neither of them are using ->after('column_name') when creating migrations.

This eventually leads to constants merge conflicts in Eloquent models PHPDocs, which would be prevented if there was a config setting to force properties from table to be sorted alphabetically for example. Simple ksort() should do the work I think.

src/Console/ModelsCommand.php
protected function getPropertiesFromTable($model)
{
    ...
    $columns = $schema->listTableColumns($table, $database);
    ksort($columns);
    ...
}

lukrak avatar Feb 25 '21 10:02 lukrak

Sounds good to me, but I think such solution should be optional (aka config setting probably).

  • For one, there are databases which don't even have ->after(). For example in Postgres you can't change the order of columns
  • Since for such cases, I cannot change the database structure, there is value having the order the same as in the database (thus my suggestion it being optional)
  • Also I just find it useful to immediately see if a model has an id (which is usually the first column in 99.99% cases) and not having to go through the phpdoc to find it.

Does this make sense?

mfn avatar Feb 25 '21 15:02 mfn

Sorry for a delay. Yes! Definitely agree that it should be optional (turned off by default since that's the way it currently works), but you can change it in config :)

lukrak avatar Mar 05 '21 15:03 lukrak

I think there is a little bit of value in keeping the docblock order similar to what's in the DB. But I find it problematic that smart reset wants to constantly reorder some properties just so it would match what's in my local DB.

Here is the how we got to the problem: we sometimes work off of a database dump of the dev or staging DB. Most times when we create migrations, we would deploy them to the dev environment first. But sometimes we are in a hurry with a new feature and deploy it straight to staging first. In that case there may be a change on the dev environment that's not in staging yet that also adds a new column, and the result is that the order of the columns on dev and staging become slightly different...

And then comes the problem that some colleagues work with the latest dev database dump and some with staging's. And when we run php artisan ide-helper:models --write --smart-reset, we constantly get annoyed with diffs like this happening:

image

And it's futile to commit those changes, because then for the next colleague their ide-helper is going to want to change it back.

I would love for an option to have properties sorted alphabetically. But it would be even better if smart reset simply didn't move any pre-existing properties around; only new properties.

amcsi avatar Jul 08 '21 09:07 amcsi