search

solidusio / solidus_auth_devise

🔑 Devise authentication for your Solidus store.
http://solidus.io
BSD 3-Clause "New" or "Revised" License
52 stars 124 forks source link

Fix migration `20101026184950` `down` method #144

Closed spaghetticode closed 5 years ago

spaghetticode commented 5 years ago

When the up method of migration 20101026184950 runs after the rollback, the column unlock_token already exists as it's added by the down method.

There's no apparent reason to keep adding that column in the down method, so this commit removes it.

kennyadsl commented 5 years ago

Is it normal that both up and down create the same column? Maybe there's an error there?

spaghetticode commented 5 years ago

I may well be... I'm not 100% sure about that. Since my main goal was to be able to complete Solidus migrations rollback without generating errors, I did the most conservative choice.

This migration is still present in the original form also in the spree version of the gem: https://github.com/spree/spree_auth_devise/blob/master/db/migrate/20101026184950_rename_columns_for_devise.rb

Now I tried to create a sample app without add_column :spree_users, :unlock_token, :string in the down method and both migrations and rollbacks are still fine. If you agree I can change the down instead of the up method.

kennyadsl commented 5 years ago

Yes I think it's better. I think that line is just a mistake since that same column is removed at line 25 of that migration.

kennyadsl commented 5 years ago

@spaghetticode can you please rebase? I removed the CircleCI config

kennyadsl commented 5 years ago

Thanks @spaghetticode !