mysociety / alaveteli

Provide a Freedom of Information request system for your jurisdiction
https://alaveteli.org
Other
389 stars 195 forks source link

Variance in WDTK production DB columns and what is generated by db:migrate #5246

Open gbp opened 5 years ago

gbp commented 5 years ago

This came up when annotating models.

One issue is the ordering which could be fixed by using the --sort or --classified-sort when running annotate. For now we can easily ignore these and not stage them.

The other appears we're missing NOT NULL on production DB columns. We should consider adding a new migration for these.

Remove: db:migrate, Added: production

diff --git a/app/models/info_request.rb b/app/models/info_request.rb
index aeeb8b683..410f2499f 100644
--- a/app/models/info_request.rb
+++ b/app/models/info_request.rb
@@ -17,9 +17,9 @@
 #  allow_new_responses_from              :string           default("anybody"), not null
 #  handle_rejected_responses             :string           default("bounce"), not null
 #  idhash                                :string           not null
+#  attention_requested                   :boolean          default(FALSE)
 #  external_user_name                    :string
 #  external_url                          :string
-#  attention_requested                   :boolean          default(FALSE)
 #  comments_allowed                      :boolean          default(TRUE), not null
 #  info_request_batch_id                 :integer
 #  last_public_response_at               :datetime
diff --git a/app/models/public_body.rb b/app/models/public_body.rb
index 0482b990d..b64edc6bd 100644
--- a/app/models/public_body.rb
+++ b/app/models/public_body.rb
@@ -10,7 +10,7 @@
 #  created_at                             :datetime         not null
 #  updated_at                             :datetime         not null
 #  home_page                              :text
-#  api_key                                :string           not null
+#  api_key                                :string
 #  info_requests_count                    :integer          default(0), not null
 #  disclosure_log                         :text
 #  info_requests_successful_count         :integer
diff --git a/app/models/request_classification.rb b/app/models/request_classification.rb
index e3eac53d7..ef3848cfc 100644
--- a/app/models/request_classification.rb
+++ b/app/models/request_classification.rb
@@ -6,8 +6,8 @@
 #  id                    :integer          not null, primary key
 #  user_id               :integer
 #  info_request_event_id :integer
-#  created_at            :datetime         not null
-#  updated_at            :datetime         not null
+#  created_at            :datetime
+#  updated_at            :datetime
 #

 class RequestClassification < ApplicationRecord
diff --git a/app/models/user.rb b/app/models/user.rb
index a1b13c3e9..80ea16a13 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -15,9 +15,9 @@
 #  last_daily_track_email            :datetime         default(Sat, 01 Jan 2000 00:00:00 GMT +00:00)
 #  ban_text                          :text             default(""), not null
 #  about_me                          :text             default(""), not null
-#  locale                            :string
 #  email_bounced_at                  :datetime
 #  email_bounce_message              :text             default(""), not null
+#  locale                            :string
 #  no_limit                          :boolean          default(FALSE), not null
 #  receive_email_alerts              :boolean          default(TRUE), not null
 #  can_make_batch_requests           :boolean          default(FALSE), not null
garethrees commented 5 years ago

Looks like there are two cases:

  1. PublicBody#api_key

Should have been set by https://github.com/mysociety/alaveteli/blob/develop/db/migrate/112_add_api_key_to_public_bodies.rb#L54

In this case it looks like the migration may have been incorrectly added initially (4dec55f3f3672e806b24aef99f6211034c49e9d6), run, and then correctly applied to the same migration in 1d2ceb988b910acdcd60910398ccd05de1688334. This probably wasn't manually fixed on the prod database.

  1. RequestClassification timestamps

Should have been set by https://github.com/mysociety/alaveteli/blob/develop/db/migrate/20120910153022_create_request_classifications.rb#L7.

Similarly, 9478c601a5557902f605dc2fa559e7045cbd94e4 was added after the migration had been initially run, so it wouldn't have "fixed" existing databases.

9478c601a5557902f605dc2fa559e7045cbd94e4 got added in https://github.com/mysociety/alaveteli/pull/3768, which was prep for https://github.com/mysociety/alaveteli/issues/2968. Annoyingly I can't find much information about it, and my poor commit message doesn't shed any light on the situation.

garethrees commented 5 years ago

Just linking this against https://github.com/mysociety/alaveteli/issues/4614, as they're similar issues that might want to get fixed at the same time.