Merge branch '387-notes-by-tag' into develop

app/helpers/notes_helper.rb

@@ -2,8 +2,15 @@ module NotesHelper
   def render_notes(notes, batch: false, **options)
     allowed_tags = batch ? batch_notes_allowed_tags : notes_allowed_tags
 
-    tag.p options do
-      sanitize(notes, tags: allowed_tags)
+    tag.aside options.merge(id: 'notes') do
+      notes.each do |note|
+        note_classes = ['note']
+        note_classes << "tag-#{note.notable_tag}" if note.notable_tag
+
+        concat tag.article sanitize(note.body, tags: allowed_tags),
+                           id: dom_id(note),
+                           class: note_classes
+      end
     end
   end
 

app/models/public_body.rb

@@ -667,12 +667,28 @@ def self.extract_domain_from_email(email)
     $1.nil? ? nil : $1.downcase
   end
 
-  def has_notes?
-    notes.present?
+  def notes
+    [legacy_note].compact + all_notes
   end
 
   def notes_as_string
-    notes.to_s
+    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
 
   def json_for_api

spec/helpers/notes_helper_spec.rb

@@ -4,19 +4,33 @@
   include NotesHelper
 
   describe '#render_notes' do
+    let(:note) { FactoryBot.build(:note, body: '<h1>title</h1>') }
+
     context 'when not a batch' do
-      subject { render_notes('<h1>title</h1>', class: 'note') }
+      subject { render_notes([note], class: 'notes') }
 
       it 'allows more tags' do
-        is_expected.to eq('<p class="note"><h1>title</h1></p>')
+        is_expected.to eq(
+          '<aside class="notes" id="notes">' \
+            '<article id="new_note" class="note tag-some_tag">' \
+              '<h1>title</h1>' \
+            '</article>' \
+          '</aside>'
+        )
       end
     end
 
     context 'when batch' do
-      subject { render_notes('<h1>title</h1>', batch: true, class: 'note') }
+      subject { render_notes([note], batch: true, class: 'notes') }
 
       it 'removes more tags' do
-        is_expected.to eq('<p class="note">title</p>')
+        is_expected.to eq(
+          '<aside class="notes" id="notes">' \
+            '<article id="new_note" class="note tag-some_tag">' \
+              'title' \
+            '</article>' \
+          '</aside>'
+        )
       end
     end
   end

spec/models/public_body_spec.rb

@@ -564,18 +564,82 @@
   end
 
   describe '#notes' do
+    subject(:notes) { public_body.notes }
 
-    it 'is valid when nil' do
-      subject = PublicBody.new(:notes => nil)
-      subject.valid?
-      expect(subject.errors[:notes]).to be_empty
+    let(:public_body) do
+      FactoryBot.build(:public_body, notes: 'foo', tag_string: 'important')
     end
 
-    it 'strips blank attributes' do
-      subject = FactoryBot.create(:public_body, :notes => '')
-      expect(subject.notes).to be_nil
+    let!(:concrete_note) do
+      FactoryBot.create(:note, :for_public_body, notable: public_body)
+    end
+
+    let!(:tagged_note) do
+      FactoryBot.create(:note, :tagged, notable_tag: 'important')
+    end
+
+    it 'returns an array' do
+      is_expected.to be_an Array
+      expect(notes.count).to eq 3
+    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
+    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')
+    end
+
+    let!(:concrete_note) do
+      FactoryBot.create(:note, :for_public_body,
+                        body: 'bar', notable: public_body)
+    end
+
+    let!(:tagged_note) do
+      FactoryBot.create(:note, :tagged, body: 'baz', notable_tag: 'important')
     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
+    end
   end
 
   describe '#has_notes?' do