vlado / earthquake-croatia

Code for app hosted on https://www.potres-petrinja.hr/ used for helping people in need during the earthquake crisis in Croatia
15 stars 13 forks source link

Add missing index on Ad.kind; Fix db:seeds #104

Closed vfonic closed 3 years ago

vfonic commented 3 years ago

@vlado we lost the index on kind due to this migration: https://github.com/vlado/earthquake-croatia/blob/master/db/migrate/20210101194804_rename_kind_to_service_on_ads.rb#L10

I'm adding it back in this PR as we're using that column to query the db when filtering: https://github.com/vlado/earthquake-croatia/blob/master/app/controllers/ads_controller.rb#L8

vfonic commented 3 years ago

Sh**! :)

What happened here was: I knew that rails will execute all the migrations even if migration timestamps are lower than the last executed migration. So I didn't recreate the migrations from "normalize city names" PR to get higher migration timestamp number. This in turn made my add_index :ads, :kind migration have lower timestamp than your migration that does remove_column :ads, :kind. Therefore, if you run rails db:drop db:create db:migrate now, you'll get schema changes:

# db/schema.rb
    t.index ["category"], name: "index_ads_on_category"
    t.index ["city"], name: "index_ads_on_city"
    t.index ["created_at"], name: "index_ads_on_created_at"
-    t.index ["kind"], name: "index_ads_on_kind"

There are two solutions I can think of right now: 1) Add add_index migration w/ add_index if... 2) Replace all db:migrate in documentation and elsewhere (not needed to change the release: phase), with db:schema:load

I'll make the 1) change now so nothing else will have to change.

Fun fact: if_not_exists for add_index has been added only in Rails 6.1. And we're using Rails 6.1. :) add_index :ads, :kind, if_not_exists: true

vfonic commented 3 years ago

@vlado done. This should work now.

vlado commented 3 years ago

@vfonic Some detective work early in the morning :) Everything worked ok now 👍