cakephp / migrations

CakePHP database migrations plugin
Other
137 stars 116 forks source link

Replace save() with update() #735

Closed davidspeijer closed 3 months ago

davidspeijer commented 3 months ago

Update() should be used instead of save().

LordSimal commented 3 months ago

->save() is the recommended method to use since it automatically calls either $this->update(); or $this->create(); depending on if a table needs to be created or updated.

or is there a specific reason why you think this is better?

davidspeijer commented 3 months ago

For deleting columns the ->save() wasn't working for the migration I created. When I recplaced it with ->update() it did. When I bake a migration for deletion it also uses ->update().

LordSimal commented 3 months ago

What does "was not working for me" mean? Did you get an error? Did it just apply the migration but nothing happened?

I just tested it with latest CakePHP 5 and it works fine with ->save();

davidspeijer commented 3 months ago

When i tried it in a cakephp 4.5.6 project it did not remove the field.

My code;

public function up(): void
{
    $table = $this->table('table');
    $table->removeColumn('field');
    $table->save();
}
LordSimal commented 3 months ago

I have an example app with exactly that version and your code works fine for me.

Are you certain you have the correct datasource set and looking at the correct database?

davidspeijer commented 3 months ago

Yes, I am. In the same migration I could update fieldnames. But the removal didn't work. Maybe there was an other error. If you are sure the code from the book is correct, you can ignore this commit.

LordSimal commented 3 months ago

can you post your whole example? maybe there is a problem if multiple operations are queued up.

davidspeijer commented 3 months ago

I will remove my code later. I think my code was like this (altered it until it worked for me).

<?php

declare(strict_types=1);

use Migrations\AbstractMigration;

class Migration01 extends AbstractMigration
{

    public function up(): void
    {
        $table = $this->table('table');
        $table->removeColumn('field');
        $table->save();
    }

    /**
     * Change Method.
     *
     * More information on this method is available here:
     * https://book.cakephp.org/phinx/0/en/migrations.html#the-change-method
     *
     * @return void
     */
    public function change(): void
    {
        $table = $this->table('table');

        $table
            ->renameColumn('xx', 'yy');

        $table->update();
    }
}
LordSimal commented 3 months ago

i am pretty sure you can't mix change() and up() methods 😅

either do only change() or do both up() and down() respectivly

davidspeijer commented 3 months ago

Thanks for the reply. Maybe we can add it to the docs not to do that 😄