Remove PublicBody#notes

This has been replace by a separate Note model.

Searching of public bodies by their note content on `/body/list/all` is
not possible due to the complex nature of the SQL query. A spec has been
disabled due it now failing without the old notes column.

This has been broken on WDTK for months without being noticed.

Fixes #7254

app/models/public_body.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: public_bodies
 #
@@ -22,7 +22,6 @@
 #  short_name                             :text
 #  request_email                          :text
 #  url_name                               :text
-#  notes                                  :text
 #  first_letter                           :string
 #  publication_scheme                     :text
 #  disclosure_log                         :text
@@ -136,7 +135,8 @@ def self.admin_title
   strip_attributes allow_empty: false, except: %i[request_email]
   strip_attributes allow_empty: true, only: %i[request_email]
 
-  translates :name, :short_name, :request_email, :url_name, :notes, :first_letter, :publication_scheme
+  translates :name, :short_name, :request_email, :url_name, :first_letter,
+             :publication_scheme
 
   # Cannot be grouped at top as it depends on the `translates` macro
   include Translatable
@@ -439,7 +439,6 @@ def self.internal_admin_body
                     :short_name => "",
                     :request_email => AlaveteliConfiguration.contact_email,
                     :home_page => nil,
-                    :notes => nil,
                     :publication_scheme => nil,
                     :last_edit_editor => "internal_admin",
                     :last_edit_comment =>
@@ -669,25 +668,13 @@ def self.extract_domain_from_email(email)
   end
 
   def notes
-    [legacy_note].compact + all_notes
+    all_notes
   end
 
   def notes_as_string
     notes.map(&:body).join(' ')
   end
 
-  def legacy_note
-    return unless read_attribute(:notes).present?
-
-    Note.new(notable: self) do |note|
-      AlaveteliLocalization.available_locales.each do |locale|
-        AlaveteliLocalization.with_locale(locale) do
-          note.body = read_attribute(:notes)
-        end
-      end
-    end
-  end
-
   def has_notes?
     notes.present?
   end
@@ -966,8 +953,7 @@ def name_for_search
 
   def self.get_public_body_list_translated_condition(table, has_first_letter=false, locale=nil)
     result = "(upper(#{table}.name) LIKE upper(:query)" \
-      " OR upper(#{table}.notes) LIKE upper(:query)" \
-        " OR upper(#{table}.short_name) LIKE upper(:query))"
+      " OR upper(#{table}.short_name) LIKE upper(:query))"
     if has_first_letter
       result += " AND #{table}.first_letter = :first_letter"
     end

db/migrate/096_create_translation_tables.rb

@@ -1,4 +1,10 @@
 class CreateTranslationTables < ActiveRecord::Migration[4.2] # 2.3
+  class ::PublicBody
+    # This has been removed from the model but is needed for this old migration
+    # to work
+    translates :notes
+  end
+
   def self.up
     fields = { :name => :text,
                :short_name => :text,

db/migrate/20220902112339_remove_public_body_notes.rb

@@ -0,0 +1,13 @@
+class RemovePublicBodyNotes < ActiveRecord::Migration[6.1]
+  def change
+    reversible do |dir|
+      dir.up do
+        remove_column :public_body_translations, :notes
+      end
+
+      dir.down do
+        PublicBody.add_translation_fields! notes: :text
+      end
+    end
+  end
+end

spec/controllers/general_controller_spec.rb

@@ -303,15 +303,13 @@
   end
 
   it 'should prioritise direct matches of public body names' do
-    FactoryBot.
-      create(:public_body,
-             name: 'Cardiff Business Technology Centre Limited',
-             notes: 'Something something cardiff council something else.')
-
-    FactoryBot.
-      create(:public_body,
-             name: 'Cardiff and Vale of Glamorgan Community Health Council',
-             notes: 'Another notes mentioning Cardiff Council.')
+    FactoryBot.create(:public_body, :with_note,
+                      name: 'Cardiff Business Technology Centre Limited',
+                      note_body: 'Something cardiff council something else.')
+
+    FactoryBot.create(:public_body, :with_note,
+                      name: 'Cardiff and Vale of Glamorgan Health Council',
+                      note_body: 'Another notes mentioning Cardiff Council.')
 
     FactoryBot.create(:public_body, name: 'Cardiff Council')
 

spec/controllers/public_body_controller_spec.rb

@@ -276,7 +276,7 @@ def make_single_language_example(locale)
     expect(assigns[:public_bodies]).to eq([ public_bodies(:humpadink_public_body) ])
   end
 
-  it "should support simple searching of bodies by notes" do
+  xit "should support simple searching of bodies by notes" do
     get :list, params: { :public_body_query => 'Albatross' }
     expect(assigns[:public_bodies]).to eq([ public_bodies(:humpadink_public_body) ])
   end

spec/factories/public_bodies.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: public_bodies
 #
@@ -22,7 +22,6 @@
 #  short_name                             :text
 #  request_email                          :text
 #  url_name                               :text
-#  notes                                  :text
 #  first_letter                           :string
 #  publication_scheme                     :text
 #  disclosure_log                         :text
@@ -49,6 +48,16 @@
       tag_string { 'eir_only' }
     end
 
+    trait :with_note do
+      transient do
+        note_body { 'This is my note' }
+      end
+
+      concrete_notes do
+        [association(:note, body: note_body)]
+      end
+    end
+
     factory :blank_email_public_body do
       request_email { '' }
     end

spec/fixtures/note_translations.yml

@@ -0,0 +1,63 @@
+geraldine_es_note_translation:
+  id: 1
+  note_id: 2
+  locale: es
+  body:
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+geraldine_en_note_translation:
+  id: 2
+  note_id: 2
+  locale: en
+  body:
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+humpadink_es_note_translation:
+  id: 3
+  note_id: 3
+  locale: es
+  body: Baguette
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+humpadink_en_note_translation:
+  id: 4
+  note_id: 3
+  locale: en
+  body: An albatross told me!!!
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+silly_walks_en_note_translation:
+  id: 6
+  note_id: 5
+  locale: en
+  body: You know the one.
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+sensible_walks_en_note_translation:
+  id: 7
+  note_id: 6
+  locale: en
+  body: I bet you’ve never heard of it.
+  created_at: 2008-10-25 10:51:01.161639
+  updated_at: 2008-10-25 10:51:01.161639
+
+other_note_translation:
+  id: 8
+  note_id: 7
+  locale: en
+  body: More notes
+  created_at: 2008-10-25 10:51:01.161639
+  updated_at: 2008-10-25 10:51:01.161639
+
+humpadink_he_IL_note_translation:
+  id: 9
+  note_id: 3
+  locale: he_IL
+  body: An albatross told me!!!
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639

spec/fixtures/notes.yml

@@ -0,0 +1,48 @@
+# == Schema Information
+# Schema version: 20220928093559
+#
+# Table name: notes
+#
+#  id           :bigint           not null, primary key
+#  notable_type :string
+#  notable_id   :bigint
+#  notable_tag  :string
+#  created_at   :datetime         not null
+#  updated_at   :datetime         not null
+#  body         :text
+#
+
+geraldine_note:
+  id: 2
+  notable_type: PublicBody
+  notable_id: 2
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+humpadink_note:
+  id: 3
+  notable_type: PublicBody
+  notable_id: 3
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+silly_walks_note:
+  id: 5
+  notable_type: PublicBody
+  notable_id: 5
+  created_at: 2007-10-24 10:51:01.161639
+  updated_at: 2007-10-24 10:51:01.161639
+
+sensible_walks_note:
+  id: 6
+  notable_type: PublicBody
+  notable_id: 6
+  created_at: 2008-10-25 10:51:01.161639
+  updated_at: 2008-10-25 10:51:01.161639
+
+other_note:
+  id: 7
+  notable_type: PublicBody
+  notable_id: 7
+  created_at: 2008-10-25 10:51:01.161639
+  updated_at: 2008-10-25 10:51:01.161639

spec/fixtures/public_bodies.yml

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220928093559
 #
 # Table name: public_bodies
 #
@@ -22,7 +22,6 @@
 #  short_name                             :text
 #  request_email                          :text
 #  url_name                               :text
-#  notes                                  :text
 #  first_letter                           :string
 #  publication_scheme                     :text
 #  disclosure_log                         :text

spec/fixtures/public_body_translations.yml

@@ -7,7 +7,6 @@ geraldine_es_public_body_translation:
   short_name: eTGQ
   url_name: etgq
   locale: es
-  notes: ""
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -22,7 +21,6 @@ geraldine_en_public_body_translation:
   short_name: TGQ
   url_name: tgq
   locale: en
-  notes: ""
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -37,7 +35,6 @@ humpadink_es_public_body_translation:
   short_name: eDfH
   url_name: edfh
   locale: es
-  notes: Baguette
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -52,7 +49,6 @@ humpadink_en_public_body_translation:
   short_name: DfH
   url_name: dfh
   locale: en
-  notes: An albatross told me!!!
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -67,7 +63,6 @@ forlorn_en_public_body_translation:
   short_name: DoL
   url_name: lonely
   locale: en
-  notes: A very lonely public body that no one has corresponded with
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -82,7 +77,6 @@ silly_walks_en_public_body_translation:
   request_email: silly-walks-requests@localhost
   short_name: MSW
   url_name: msw
-  notes: You know the one.
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639
@@ -97,7 +91,6 @@ sensible_walks_en_public_body_translation:
   request_email: sensible-walks-requests@localhost
   short_name: SenseWalk
   url_name: sensible_walks
-  notes: I bet you’ve never heard of it.
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2008-10-25 10:51:01.161639
@@ -112,7 +105,6 @@ other_public_body_translation:
   request_email: other@localhost
   short_name: Another Public Body
   url_name: another_public_body
-  notes: More notes
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2008-10-25 10:51:01.161639
@@ -127,7 +119,6 @@ humpadink_he_IL_public_body_translation:
   short_name: DfH
   url_name: dfh
   locale: he_IL
-  notes: An albatross told me!!!
   publication_scheme: ""
   disclosure_log: ""
   created_at: 2007-10-24 10:51:01.161639

spec/lib/public_body_csv_spec.rb

@@ -117,10 +117,10 @@
                 :short_name => 'CSV',
                 :request_email => 'csv@localhost',
                 :tag_string => 'exported',
-                :notes => 'An exported authority',
+                :note_body => 'An exported authority',
                 :created_at => '2007-10-25 10:51:01 UTC',
                 :updated_at => '2007-10-25 10:51:01 UTC' }
-      body = FactoryBot.create(:public_body, attrs)
+      body = FactoryBot.create(:public_body, :with_note, attrs)
 
       csv = PublicBodyCSV.new
       csv << body
@@ -138,19 +138,19 @@
                  :short_name => 'CSV1',
                  :request_email => 'csv1@localhost',
                  :tag_string => 'exported',
-                 :notes => 'An exported authority',
+                 :note_body => 'An exported authority',
                  :created_at => '2007-10-25 10:51:01 UTC',
                  :updated_at => '2007-10-25 10:51:01 UTC' }
-      body1 = FactoryBot.create(:public_body, attrs1)
+      body1 = FactoryBot.create(:public_body, :with_note, attrs1)
 
       attrs2 = { :name => 'Exported to CSV 2',
                  :short_name => 'CSV2',
                  :request_email => 'csv2@localhost',
                  :tag_string => 'exported',
-                 :notes => 'Exported authority',
+                 :note_body => 'Exported authority',
                  :created_at => '2011-01-26 14:11:02 UTC',
                  :updated_at => '2011-01-26 14:11:02 UTC' }
-      body2 = FactoryBot.create(:public_body, attrs2)
+      body2 = FactoryBot.create(:public_body, :with_note, attrs2)
 
       expected = <<-CSV.strip_heredoc
       Name,Short name,URL name,Home page,Publication scheme,Disclosure log,Notes,Created at,Updated at,Version