Add Notable concern to PublicBody

Adds an association as `Notable#concrete_notes` to avoid clashing with
the current `PublicBody#notes` column.

This uses a polymorphic association so any model can be `Notable`.

app/controllers/admin/notes_controller.rb

@@ -3,12 +3,12 @@ class Admin::NotesController < AdminController
   include TranslatableParams
 
   def new
-    @note = Note.new
+    @note = scope.build
     @note.build_all_translations
   end
 
   def create
-    @note = Note.new(note_params)
+    @note = scope.build(note_params)
     if @note.save
       notice = 'Note successfully created.'
       redirect_to admin_note_parent_path(@note), notice: notice
@@ -19,12 +19,12 @@ def create
   end
 
   def edit
-    @note = Note.find(params[:id])
+    @note = scope.find(params[:id])
     @note.build_all_translations
   end
 
   def update
-    @note = Note.find(params[:id])
+    @note = scope.find(params[:id])
     if @note.update(note_params)
       notice = 'Note successfully updated.'
       redirect_to admin_note_parent_path(@note), notice: notice
@@ -43,6 +43,10 @@ def destroy
 
   private
 
+  def scope
+    Note.where(params.slice(:notable_id, :notable_type).permit!)
+  end
+
   def note_params
     translatable_params(
       params.require(:note),

app/models/concerns/notable.rb

@@ -0,0 +1,15 @@
+module Notable
+  extend ActiveSupport::Concern
+
+  included do
+    has_many :concrete_notes,
+             class_name: 'Note',
+             as: :notable,
+             inverse_of: :notable,
+             dependent: :destroy
+  end
+
+  def all_notes
+    concrete_notes.with_translations
+  end
+end

app/models/note.rb

@@ -21,4 +21,5 @@ class Note < ApplicationRecord
   belongs_to :notable, polymorphic: true
 
   validates :body, presence: true
+  validates :notable, presence: true
 end

app/models/public_body.rb

@@ -36,6 +36,7 @@
 class PublicBody < ApplicationRecord
   include AdminColumn
   include Taggable
+  include Notable
 
   class ImportCSVDryRun < StandardError; end
 

app/views/admin/notes/_form.html.erb

@@ -1,5 +1,11 @@
 <%= foi_error_messages_for :note %>
 
+<div class="well">
+  Applies to <%= both_links(@note.notable) %>
+  <%= f.hidden_field :notable_id %>
+  <%= f.hidden_field :notable_type %>
+</div>
+
 <div id="div-locales">
   <ul class="locales nav nav-tabs">
     <% @note.ordered_translations.each do |translation| %>

app/views/admin/notes/_show.html.erb

@@ -0,0 +1,31 @@
+<div class="row">
+  <% if notes.size > 0 %>
+    <table class="table table-condensed table-hover span12 censor-rule-list">
+      <tr>
+        <th>ID</th>
+        <th>Notable ID</th>
+        <th>Notable type</th>
+        <th>Notable tag</th>
+        <th>Actions</th>
+      </tr>
+
+      <% notes.each do |note| %>
+        <tr class="<%= cycle('odd', 'even') %>">
+          <td class="id"><%= h note.id %></td>
+          <td class="notable_id"><%= h note.notable_id %></td>
+          <td class="notable_type"><%= h note.notable_type %></td>
+          <td class="notable_tag"><%= h note.notable_tag %></td>
+          <td><%= link_to "Edit", edit_admin_note_path(note) %></td>
+        </tr>
+      <% end %>
+    </table>
+  <% else %>
+    <p class="span12">None yet.</p>
+  <% end %>
+</div>
+
+<div class="row">
+  <p class="span12">
+    <%= link_to "New note", new_admin_note_path(notable_type: notable.class, notable_id: notable), class: "btn btn-info" %>
+  </p>
+</div>

app/views/admin_public_body/show.html.erb

@@ -131,3 +131,11 @@
 <h2>Censor rules</h2>
 
 <%= render :partial => 'admin_censor_rule/show', :locals => { :censor_rules => @public_body.censor_rules, :public_body => @public_body } %>
+
+<hr>
+
+<h2>Notes</h2>
+
+<%= render partial: 'admin/notes/show',
+  locals: { notes: @public_body.all_notes,
+            notable: @public_body } %>

config/routes.rb

@@ -491,7 +491,11 @@
   end
 
   direct :admin_note_parent do |note|
-    admin_general_index_path
+    if note.notable
+      url_for([:admin, note.notable])
+    else
+      admin_general_index_path
+    end
   end
   ####
 

spec/controllers/admin/notes_controller_spec.rb

@@ -24,11 +24,7 @@
       post :create, params: params
     end
 
-    context 'on a successful create' do
-      let(:params) do
-        { note: { body: 'New body' } }
-      end
-
+    shared_context 'successful create' do
       it 'assigns the note' do
         expect(assigns[:note]).to be_a(Note)
       end
@@ -40,9 +36,27 @@
       it 'sets a notice' do
         expect(flash[:notice]).to eq('Note successfully created.')
       end
+    end
+
+    context 'on a successful create of concrete note' do
+      include_context 'successful create'
+
+      let!(:note) { FactoryBot.create(:note, :for_public_body) }
+      let(:public_body) { note.notable }
 
-      it 'redirects to the general index' do
-        expect(response).to redirect_to(admin_general_index_path)
+      let(:params) do
+        {
+          id: note.id,
+          note: {
+            body: 'New body',
+            notable_id: public_body.id,
+            notable_type: public_body.class.name
+          }
+        }
+      end
+
+      it 'redirects to the public body admin' do
+        expect(response).to redirect_to(admin_public_body_path(public_body))
       end
     end
 
@@ -90,11 +104,7 @@
       patch :update, params: params
     end
 
-    context 'on a successful update' do
-      let(:params) do
-        { id: note.id, note: { body: 'New body' } }
-      end
-
+    shared_context 'successful update' do
       it 'assigns the note' do
         expect(assigns[:note]).to eq(note)
       end
@@ -106,9 +116,27 @@
       it 'sets a notice' do
         expect(flash[:notice]).to eq('Note successfully updated.')
       end
+    end
+
+    context 'on a successful update of concrete note' do
+      include_context 'successful update'
+
+      let!(:note) { FactoryBot.create(:note, :for_public_body) }
+      let(:public_body) { note.notable }
+
+      let(:params) do
+        {
+          id: note.id,
+          note: {
+            body: 'New body',
+            notable_id: public_body.id,
+            notable_type: public_body.class.name
+          }
+        }
+      end
 
-      it 'redirects to the general index' do
-        expect(response).to redirect_to(admin_general_index_path)
+      it 'redirects to the public body admin' do
+        expect(response).to redirect_to(admin_public_body_path(public_body))
       end
     end
 
@@ -145,9 +173,14 @@
       expect(flash[:notice]).to eq('Note successfully destroyed.')
     end
 
-    it 'redirects to the general index' do
-      delete :destroy, params: { id: note.id }
-      expect(response).to redirect_to(admin_general_index_path)
+    context 'when concrete note' do
+      let!(:note) { FactoryBot.create(:note, :for_public_body) }
+      let(:public_body) { note.notable }
+
+      it 'redirects to the public body admin' do
+        delete :destroy, params: { id: note.id }
+        expect(response).to redirect_to(admin_public_body_path(public_body))
+      end
     end
   end
 end

spec/factories/notes.rb

@@ -15,6 +15,7 @@
 FactoryBot.define do
   factory :note do
     body { 'Test note' }
+    association :notable, factory: :public_body
 
     trait :for_public_body do
       association :notable, factory: :public_body

spec/models/concerns/notable.rb

@@ -0,0 +1,11 @@
+RSpec.shared_examples 'concerns/notable' do |record|
+  describe '#all_notes' do
+    subject { record.all_notes }
+
+    let!(:note) { FactoryBot.create(:note, notable: record) }
+    let!(:other_note) { FactoryBot.create(:note) }
+
+    it { is_expected.to include note }
+    it { is_expected.to_not include other_note }
+  end
+end

spec/models/note_spec.rb

@@ -24,6 +24,11 @@
       note.body = nil
       expect(note).not_to be_valid
     end
+
+    it 'requires notable' do
+      note.notable = nil
+      expect(note).not_to be_valid
+    end
   end
 
   describe 'translations' do

spec/models/public_body_spec.rb

@@ -29,8 +29,10 @@
 #
 
 require 'spec_helper'
+require 'models/concerns/notable'
 
 RSpec.describe PublicBody do
+  it_behaves_like 'concerns/notable', FactoryBot.build(:public_body)
 
   describe <<-EOF.squish do
     temporary tests for Globalize::ActiveRecord::InstanceMethods#read_attribute