Regarding Timestamps for Models

[jhavenz]

1st off, thank you for the great package!

I wanted to mention that I just spent a little time getting an error when trying to save an orbit model which was related to having a duplicate 'created_at' column. Which was this:

Illuminate\Database\QueryException: SQLSTATE[HY000]: General error: 1 duplicate column name: created_at (Connection: orbit, SQL: create table "articles" ("slug" varchar not null, "created_at" datetime, "updated_at" datetime, "author_id" varchar, "title" varchar not null, "category" varchar not null, "published_at" datetime, "excerpt" varchar, "content" text, "created_at" datetime, "updated_at" datetime, primary key ("slug")))

In my model's schema, I was declaring $table->timestamps() (which you're also doing in one of your examples in the read me), and after having spent a bit of time source-diving this package, came to see why I was getting the error.

The Orbital::migrate() method:

<?php

namespace Orbit\Concerns;

trait Orbital 
{
     ... other methods

     public function migrate() 
     {
          // ... other logic

          static::resolveConnection()->getSchemaBuilder()->create($table, function (Blueprint $table) use (&$blueprint) {
            static::schema($table);

            $this->callTraitMethod('schema', $table);

            $driver = Orbit::driver(static::getOrbitalDriver());

            if (method_exists($driver, 'schema')) {
                $driver->schema($table);
            }

            # -------------------------------------------------------------------------
            # This isn't checking if the timestamp columns have already been declared
            # in the model's `public static function schema(Blueprint $table)` method
            # -------------------------------------------------------------------------
            if ($this->usesTimestamps()) {
                $table->timestamps();
            }

            $blueprint = $table;
        });

        // ...other logic
     }
}

...thus producing the error mentioned above.

So adding mention of this in the readme, or logic in the trait, seems like it'd prevent this for any future users.

Cheers.

[ryangjchandler]

I can't see any reference to ->timestamps() in the README – there's only a single timestamp() call for a custom field.

[jhavenz]

Right, but my primary point is that: should one add ->timestamps() to their schema declaration, or want to use custom timestamp columns, this package isn't accounting for it.

The mention of the readme is just a reference to what would be a good place to note this behavior.

[ryangjchandler]

Okay, this will be made clearer in the next release. I have a v2.0 in beta right now almost ready for stable release.

[jhavenz]

sounds good man 👍