Merge branch '387-notes-by-tag' into 6912-request-notes

app/controllers/admin/notes_controller.rb

@@ -0,0 +1,57 @@
+class Admin::NotesController < AdminController
+  include Admin::TagHelper
+  include TranslatableParams
+
+  def new
+    @note = scope.build
+    @note.build_all_translations
+  end
+
+  def create
+    @note = scope.build(note_params)
+    if @note.save
+      notice = 'Note successfully created.'
+      redirect_to admin_note_parent_path(@note), notice: notice
+    else
+      @note.build_all_translations
+      render :new
+    end
+  end
+
+  def edit
+    @note = scope.find(params[:id])
+    @note.build_all_translations
+  end
+
+  def update
+    @note = scope.find(params[:id])
+    if @note.update(note_params)
+      notice = 'Note successfully updated.'
+      redirect_to admin_note_parent_path(@note), notice: notice
+    else
+      @note.build_all_translations
+      render :edit
+    end
+  end
+
+  def destroy
+    @note = Note.find(params[:id])
+    @note.destroy
+    notice = 'Note successfully destroyed.'
+    redirect_to admin_note_parent_path(@note), notice: notice
+  end
+
+  private
+
+  def scope
+    Note.where(params.slice(:notable_tag, :notable_id, :notable_type).permit!)
+  end
+
+  def note_params
+    translatable_params(
+      params.require(:note),
+      translated_keys: [:locale, :body],
+      general_keys: [:notable_tag, :notable_id, :notable_type]
+    )
+  end
+end

app/controllers/admin/tags_controller.rb

@@ -22,6 +22,9 @@ def show
       @tag
     )
 
+    @notes = Note.distinct.where(notable_tag: @tag).
+      paginate(page: params[:page], per_page: 50)
+
     @taggings = current_klass.with_tag(@tag).distinct.
       joins(:tags).merge(
         apply_filters(HasTagString::HasTagStringTag.all)

app/helpers/admin/tag_helper.rb

@@ -9,6 +9,10 @@ def render_tags(tags)
 
   def render_tag(record_tag)
     tag.span class: 'label label-info tag' do
+      if record_tag.is_a?(String)
+        record_tag = HasTagString::HasTagStringTag.from_string(record_tag)
+      end
+
       render_tag_href(record_tag)
     end
   end

app/helpers/alaveteli_pro/batch_request_authority_searches_helper.rb

@@ -1,11 +1,6 @@
 module AlaveteliPro
   # Helper methods for the batch builder
   module BatchRequestAuthoritySearchesHelper
-    def batch_notes_allowed_tags
-      Alaveteli::Application.config.action_view.sanitized_allowed_tags -
-        %w(pre h1 h2 h3 h4 h5 h6 img blockquote html head body style)
-    end
-
     def batch_authority_count
       count = @draft_batch_request.public_bodies.count
 

app/helpers/alaveteli_pro/public_bodies_helper.rb

@@ -4,7 +4,7 @@ def public_body_search_attributes(body)
       id: body.id,
       name: body.name,
       short_name: body.short_name,
-      notes: body.notes,
+      notes: body.notes_as_string,
       info_requests_visible_count: body.info_requests_visible_count,
       about: _('About {{public_body_name}}', public_body_name: body.name)
     }

app/helpers/notes_helper.rb

@@ -0,0 +1,26 @@
+module NotesHelper
+  def render_notes(notes, batch: false, **options)
+    allowed_tags = batch ? batch_notes_allowed_tags : notes_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
+
+  def notes_allowed_tags
+    Alaveteli::Application.config.action_view.sanitized_allowed_tags +
+      %w(th time u font iframe) -
+      %w(html head body style)
+  end
+
+  def batch_notes_allowed_tags
+    notes_allowed_tags - %w(pre h1 h2 h3 h4 h5 h6 img blockquote font iframe)
+  end
+end

app/models/concerns/notable.rb

@@ -0,0 +1,25 @@
+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 + tagged_notes.with_translations
+  end
+
+  def tagged_notes
+    Note.where(notable_tag: notable_tags)
+  end
+
+  private
+
+  def notable_tags
+    tags.map(&:name_and_value)
+  end
+end

app/models/note.rb

@@ -0,0 +1,31 @@
+# == Schema Information
+# Schema version: 20220720085105
+#
+# 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
+#
+
+class Note < ApplicationRecord
+  include AdminColumn
+
+  translates :body
+  include Translatable
+
+  belongs_to :notable, polymorphic: true
+
+  validates :body, presence: true
+  validates :notable_or_notable_tag, presence: true
+
+  private
+
+  def notable_or_notable_tag
+    notable || notable_tag
+  end
+end

app/models/public_body.rb

@@ -36,6 +36,7 @@
 class PublicBody < ApplicationRecord
   include AdminColumn
   include Taggable
+  include Notable
 
   class ImportCSVDryRun < StandardError; end
 
@@ -120,7 +121,7 @@ def self.admin_title
   scope :visible, -> { where("public_bodies.id <> #{ PublicBody.internal_admin_body.id }") }
 
   acts_as_versioned
-  acts_as_xapian :texts => [:name, :short_name, :notes],
+  acts_as_xapian :texts => [:name, :short_name, :notes_as_string],
                  :values => [
                    # for sorting
                    [:created_at_numeric, 1, "created_at", :number]
@@ -667,26 +668,28 @@ def self.extract_domain_from_email(email)
     $1.nil? ? nil : $1.downcase
   end
 
-  def has_notes?(opts = {})
-    tag = opts[:tag]
+  def notes
+    [legacy_note].compact + all_notes
+  end
 
-    if tag
-      notes.present? && has_tag?(tag)
-    else
-      notes.present?
-    end
+  def notes_as_string
+    notes.map(&:body).join(' ')
   end
 
-  # TODO: Deprecate this method. Its only used in a couple of views so easy to
-  # update to just call PublicBody#notes
-  def notes_as_html
-    notes
+  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 notes_without_html
-    # assume notes are reasonably behaved HTML, so just use simple regexp
-    # on this
-    @notes_without_html ||= (notes.nil? ? '' : notes.gsub(/<\/?[^>]*>/, ""))
+  def has_notes?
+    notes.present?
   end
 
   def json_for_api
@@ -703,7 +706,7 @@ def json_for_api
       # information
       # :version, :last_edit_editor, :last_edit_comment
       :home_page => calculated_home_page,
-      :notes => notes.to_s,
+      :notes => notes_as_string,
       :publication_scheme => publication_scheme.to_s,
       :tags => tag_array,
       :info => {

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

@@ -0,0 +1,38 @@
+<%= foi_error_messages_for :note %>
+
+<div class="well">
+  <% if @note.notable %>
+    Applies to <%= both_links(@note.notable) %>
+    <%= f.hidden_field :notable_id %>
+    <%= f.hidden_field :notable_type %>
+  <% else %>
+    Applies to objects tagged with <%= render_tag @note.notable_tag %>
+    <%= f.hidden_field :notable_tag %>
+  <% end %>
+</div>
+
+<div id="div-locales">
+  <ul class="locales nav nav-tabs">
+    <% @note.ordered_translations.each do |translation| %>
+      <li>
+        <a href="#div-locale-<%= translation.locale.to_s %>" data-toggle="tab" >
+          <%= locale_name(translation.locale.to_s) || translation.locale.to_s %>
+        </a>
+      </li>
+    <% end %>
+  </ul>
+
+  <div class="tab-content">
+    <% @note.ordered_translations.each do |translation| %>
+      <% if AlaveteliLocalization.default_locale?(translation.locale) %>
+        <%= fields_for('note', @note) do |t| %>
+          <%= render partial: 'locale_fields', locals: { t: t, locale: translation.locale } %>
+        <% end %>
+      <% else %>
+        <%= f.fields_for(:translations, translation, child_index: translation.locale) do |t| %>
+          <%= render partial: 'locale_fields', locals: { t: t, locale: translation.locale } %>
+        <% end %>
+      <% end %>
+    <% end %>
+  </div>
+</div>

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

@@ -0,0 +1,16 @@
+<div class="tab-pane" id="div-locale-<%= locale %>">
+<div class="control-group">
+  <%= t.hidden_field :locale, :value => locale %>
+
+  <%= t.label :body, class: 'control-label' %>
+  <div class="controls">
+    <% if AlaveteliLocalization.default_locale?(locale) && t.object.errors[:body].any? %>
+      <span class="fieldWithErrors">
+    <% end %>
+    <%= t.text_area :body, class: 'span6', rows: 10 %>
+    <% if AlaveteliLocalization.default_locale?(locale) && t.object.errors[:body].any? %>
+      </span>
+    <%end %>
+  </div>
+</div>
+</div>

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

@@ -0,0 +1,24 @@
+<div class="accordion-group">
+  <div class="accordion-heading accordion-toggle row">
+    <span class="item-title span6">
+      <%= link_to chevron_right, "##{dom_id(note)}", data: { toggle: 'collapse', parent: 'notes' } %>
+      <%= link_to(note.body, edit_admin_note_path(note), title: 'view full details') %>
+    </span>
+
+    <span class="item-metadata span6">
+      <%= render_tag note.notable_tag if note.notable_tag %>
+    </span>
+  </div>
+  <%= tag.div id: dom_id(note), class: 'item-detail accordion-body collapse row' do %>
+    <% note.for_admin_column do |name, value, type| %>
+      <div>
+        <span class="span6">
+          <b><%= name %></b>
+        </span>
+        <span class="span6">
+          <%= admin_value(value) %>
+        </span>
+      </div>
+    <% end %>
+  <% end %>
+</div>

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/notes/edit.erb

@@ -0,0 +1,21 @@
+<div class="row">
+  <div class="span12">
+    <div class="page-header">
+      <h1><%= @title = 'Edit note' %></h1>
+    </div>
+  </div>
+</div>
+
+<%= form_for [:admin, @note], class: 'form form-horizontal' do |f| %>
+  <%= render partial: 'form', locals: { f: f } %>
+  <div class="form-actions">
+    <%= submit_tag 'Save', class: 'btn btn-success' %>
+  </div>
+<% end %>
+
+<%= form_tag [:admin, @note], class: 'form form-inline', method: 'delete' do %>
+  <%= submit_tag 'Destroy note',
+                 class: 'btn btn-danger',
+                 data: { confirm: 'Are you sure? This is irreversible.' } %>
+  (this is permanent!)
+<% end %>