Remove PublicBody#notes

This has been replace by a separate Note model.

Fixes #7254

app/models/public_body.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220902112339
 #
 # 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,7 @@ 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 +438,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 +667,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 +952,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/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

@@ -305,13 +305,11 @@
   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.')
+             name: 'Cardiff Business Technology Centre Limited')
 
     FactoryBot.
       create(:public_body,
-             name: 'Cardiff and Vale of Glamorgan Community Health Council',
-             notes: 'Another notes mentioning Cardiff Council.')
+             name: 'Cardiff and Vale of Glamorgan Community Health Council')
 
     FactoryBot.create(:public_body, name: 'Cardiff Council')
 

spec/factories/public_bodies.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220902112339
 #
 # 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_bodies.yml

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220902112339
 #
 # 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',
                 :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.notes.create(body: 'An exported authority')
 
       csv = PublicBodyCSV.new
       csv << body
@@ -138,10 +138,10 @@
                  :short_name => 'CSV1',
                  :request_email => 'csv1@localhost',
                  :tag_string => 'exported',
-                 :notes => '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.notes.create(body: 'An exported authority')
 
       attrs2 = { :name => 'Exported to CSV 2',
                  :short_name => 'CSV2',
@@ -151,6 +151,7 @@
                  :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.notes.create(body: 'Exported authority')
 
       expected = <<-CSV.strip_heredoc
       Name,Short name,URL name,Home page,Publication scheme,Disclosure log,Notes,Created at,Updated at,Version

spec/models/public_body_spec.rb

@@ -1,5 +1,5 @@
 # == Schema Information
-# Schema version: 20220210114052
+# Schema version: 20220902112339
 #
 # 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
@@ -544,9 +543,7 @@
   describe '#notes' do
     subject(:notes) { public_body.notes }
 
-    let(:public_body) do
-      FactoryBot.build(:public_body, notes: 'foo', tag_string: 'important')
-    end
+    let(:public_body) { FactoryBot.build(:public_body) }
 
     let!(:concrete_note) do
       FactoryBot.create(:note, :for_public_body, notable: public_body)
@@ -558,21 +555,20 @@
 
     it 'returns an array' do
       is_expected.to be_an Array
-      expect(notes.count).to eq 3
+      expect(notes.count).to eq 2
     end
 
     it 'combined notable notes with legacy note' do
-      expect(notes[0].body).to eq 'foo'
-      expect(notes[1]).to eq concrete_note
-      expect(notes[2]).to eq tagged_note
+      expect(notes[0]).to eq concrete_note
+      expect(notes[1]).to eq tagged_note
     end
   end
 
   describe '#notes_as_string' do
     subject(:notes) { public_body.notes_as_string }
 
     let(:public_body) do
-      FactoryBot.build(:public_body, notes: 'foo', tag_string: 'important')
+      FactoryBot.build(:public_body, tag_string: 'important')
     end
 
     let!(:concrete_note) do
@@ -585,56 +581,28 @@
     end
 
     it 'concaterates note bodies' do
-      is_expected.to eq('foo bar baz')
-    end
-  end
-
-  describe '#legacy_note' do
-    subject(:legacy_note) { public_body.legacy_note }
-
-    context 'without legacy translated attributes' do
-      let(:public_body) { FactoryBot.build(:public_body) }
-      it { is_expected.to be_nil }
-    end
-
-    context 'with legacy translated attributes' do
-      let(:public_body) do
-        FactoryBot.build(
-          :public_body,
-          notes: 'foo',
-          translations_attributes: { es: { locale: 'es', notes: 'bar' } }
-        )
-      end
-
-      it 'builds new note instance' do
-        is_expected.to be_a Note
-        expect(legacy_note.body).to eq 'foo'
-        AlaveteliLocalization.with_locale('es') do
-          expect(legacy_note.body).to eq 'bar'
-        end
-      end
-
-      it 'assigns body as notable' do
-        expect(legacy_note.notable).to eq public_body
-      end
+      is_expected.to eq('bar baz')
     end
   end
 
   describe '#has_notes?' do
 
+    subject { public_body.has_notes? }
+    let(:public_body) { PublicBody.new }
+
     it 'returns false if notes is nil' do
-      subject = PublicBody.new(:notes => nil)
-      expect(subject.has_notes?).to eq(false)
+      allow(public_body).to receive(:notes).and_return(nil)
+      is_expected.to eq(false)
     end
 
-    it 'returns false if notes is blank' do
-      subject = PublicBody.new(:notes => '')
-      expect(subject.has_notes?).to eq(false)
+    it 'returns false if notes is empty' do
+      allow(public_body).to receive(:notes).and_return([])
+      is_expected.to eq(false)
     end
 
     it 'returns true if notes are present' do
-      subject = PublicBody.new(:notes => 'x')
-      expect(subject.has_notes?).to eq(true)
+      allow(public_body).to receive(:notes).and_return([double(:note)])
+      is_expected.to eq(true)
     end
 
   end
@@ -1234,7 +1202,7 @@ def set_default_attributes(public_body)
 
     ActsAsXapian::ActsAsXapianJob.destroy_all
 
-    body.update!(notes: 'test')
+    body.update!(request_email: 'other@localhost')
 
     expected_events =
       ActsAsXapian::ActsAsXapianJob.