`lastRun` has incorrect value when migrating down

[stalniy]

I'm trying to create a custom store for migrations, a table in SQL db:

CREATE TABLE IF NOT EXISTS "schema_migrations" (version character varying NOT NULL PRIMARY KEY)

All works good for up migrations but for down migrations lastRun in save method of my custom store has invalid value. That value actually points to the next migration not that one which was run.

if you do console.log(migration.title, migration.timestamp, set.lastRun) at https://github.com/tj/node-migrate/blob/master/lib/migrate.js#L67 you will see that migration.title and lastRun have different values.

So, then I don't know which migration to remove from schema_migrations table

[wesleytodd]

Interesting, I thought we had a test for this, but I must be wrong. In the last major refactor I removed the use of lastRun and instead use the timestamp as in indication of which were run. I only left that flag as an indicator for people like you to use. Maybe because of this it just never was done correctly.

If you could make a reduced test case, or even a failing test in our test suite that would be super helpful!

[stalniy]

The issue is in this test https://github.com/tj/node-migrate/blob/master/test/basic.js#L172

It expects that after migrating down lastRun should equal 1-add-guy-ferrets.js but it's actually next migration to run (similar indication to how pos parameter was implemented in earlier versions) not the last one. The correct one must be 2-add-girl-ferrets.js

lastRun must always equal to migration title which was run the last and timestamp field can be used to determine whether the migration direction was up or down

[wesleytodd]

Yep! This would be a great little PR to open. If you don't have the time I can try to look at it this weekend.

[wesleytodd]

@stalniy have you had time to look at this?

[stalniy]

Sorry, I didn’t have time. Also I switched to sequelize cli, so it’s not an issue for me anymore

[wesleytodd]

Ok, thanks. I will make the update myself then.