Skip to content

Commit

Permalink
Add tagged notes
Browse files Browse the repository at this point in the history
Allow notes to be created with an association to tags via a
`Note#notable_tag` column.

This updates `Notable#all_notes` to combine concrete and tagged notes.

Fixes #6994
  • Loading branch information
gbp committed Aug 30, 2022
1 parent 9cbb844 commit 1b19d18
Show file tree
Hide file tree
Showing 15 changed files with 142 additions and 9 deletions.
4 changes: 2 additions & 2 deletions app/controllers/admin/notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -44,14 +44,14 @@ def destroy
private

def scope
Note.where(params.slice(:notable_id, :notable_type).permit!)
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_id, :notable_type]
general_keys: [:notable_tag, :notable_id, :notable_type]
)
end
end
3 changes: 3 additions & 0 deletions app/controllers/admin/tags_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
12 changes: 11 additions & 1 deletion app/models/concerns/notable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,16 @@ module Notable
end

def all_notes
concrete_notes.with_translations
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
8 changes: 7 additions & 1 deletion app/models/note.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,5 +21,11 @@ class Note < ApplicationRecord
belongs_to :notable, polymorphic: true

validates :body, presence: true
validates :notable, presence: true
validates :notable_or_notable_tag, presence: true

private

def notable_or_notable_tag
notable || notable_tag
end
end
11 changes: 8 additions & 3 deletions app/views/admin/notes/_form.html.erb
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
<%= foi_error_messages_for :note %>

<div class="well">
Applies to <%= both_links(@note.notable) %>
<%= f.hidden_field :notable_id %>
<%= f.hidden_field :notable_type %>
<% 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">
Expand Down
13 changes: 13 additions & 0 deletions app/views/admin/tags/_notes.html.erb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<h2>Notes</h2>

<div class="btn-toolbar">
<div class="btn-group">
<%= link_to 'New note', new_admin_note_path(notable_tag: @tag), class: 'btn' %>
</div>
</div>

<% if @notes.empty? %>
<p>No notes.</p>
<% else %>
<%= render partial: 'tagging', collection: @notes %>
<% end %>
5 changes: 5 additions & 0 deletions app/views/admin/tags/_tagging.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@
snippet: tagging
} %>

<% when Note %>
<%= render partial: 'admin/notes/note', locals: {
note: tagging
} %>

<% else %>
<pre><%= tagging.to_json %></pre>
<% end %>
2 changes: 2 additions & 0 deletions app/views/admin/tags/show.html.erb
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
<h1><%= @title = "Tag – #{@tag}" %></h1>

<%= render partial: 'notes', locals: { notes: @notes } %>

<div class="row">
<div class="span12">
<h2>Taggings</h2>
Expand Down
4 changes: 3 additions & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -491,7 +491,9 @@
end

direct :admin_note_parent do |note|
if note.notable
if note.notable_tag
admin_tag_path(tag: note.notable_tag)
elsif note.notable
url_for([:admin, note.notable])
else
admin_general_index_path
Expand Down
46 changes: 46 additions & 0 deletions spec/controllers/admin/notes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,24 @@
end
end

context 'on a successful create of tagged note' do
include_context 'successful create'

let!(:note) { FactoryBot.create(:note, :tagged) }
let(:tag) { note.notable_tag }

let(:params) do
{
id: note.id,
note: { body: 'New body', notable_tag: tag }
}
end

it 'redirects to the tag admin' do
expect(response).to redirect_to(admin_tag_path(tag))
end
end

context 'on an unsuccessful create' do
let(:params) do
{ note: { body: '' } }
Expand Down Expand Up @@ -140,6 +158,24 @@
end
end

context 'on a successful update of tagged note' do
include_context 'successful update'

let!(:note) { FactoryBot.create(:note, :tagged) }
let(:tag) { note.notable_tag }

let(:params) do
{
id: note.id,
note: { body: 'New body', notable_tag: tag }
}
end

it 'redirects to the tag admin' do
expect(response).to redirect_to(admin_tag_path(tag))
end
end

context 'on an unsuccessful update' do
let(:params) do
{ id: note.id, note: { body: '' } }
Expand Down Expand Up @@ -182,5 +218,15 @@
expect(response).to redirect_to(admin_public_body_path(public_body))
end
end

context 'when tagged note' do
let!(:note) { FactoryBot.create(:note, :tagged) }
let(:tag) { note.notable_tag }

it 'redirects to the public body admin' do
delete :destroy, params: { id: note.id }
expect(response).to redirect_to(admin_tag_path(tag))
end
end
end
end
9 changes: 9 additions & 0 deletions spec/controllers/admin/tags_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,15 @@ def tags
)
end

it 'loads notes' do
note = FactoryBot.create(:note, notable_tag: 'foo')
other_note = FactoryBot.create(:note, notable_tag: 'bar')

get :show, params: { tag: 'foo' }
expect(assigns[:notes]).to include(note).once
expect(assigns[:notes]).to_not include(other_note)
end

def taggings
assigns[:taggings]
end
Expand Down
7 changes: 7 additions & 0 deletions spec/factories/notes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,16 @@
factory :note do
body { 'Test note' }
association :notable, factory: :public_body
notable_tag { 'some_tag' }

trait :for_public_body do
association :notable, factory: :public_body
notable_tag { nil }
end

trait :tagged do
notable { nil }
notable_tag { 'foo' }
end
end
end
13 changes: 13 additions & 0 deletions spec/models/concerns/notable_and_taggable.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
RSpec.shared_examples 'concerns/notable_and_taggable' do |record|
describe '#all_notes' do
subject { record.all_notes }

before { record.tag_string = 'foo' }

let!(:tagged_note) { FactoryBot.create(:note, notable_tag: 'foo') }
let!(:other_tagged_note) { FactoryBot.create(:note, notable_tag: 'bar') }

it { is_expected.to include tagged_note }
it { is_expected.to_not include other_tagged_note }
end
end
11 changes: 10 additions & 1 deletion spec/models/note_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,18 @@
expect(note).not_to be_valid
end

it 'requires notable' do
it 'requires notable or notable_tag' do
note.notable = nil
note.notable_tag = nil
expect(note).not_to be_valid

note.notable = nil
note.notable_tag = 'foo'
expect(note).to be_valid

note.notable = PublicBody.first
note.notable_tag = nil
expect(note).to be_valid
end
end

Expand Down
3 changes: 3 additions & 0 deletions spec/models/public_body_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,12 @@

require 'spec_helper'
require 'models/concerns/notable'
require 'models/concerns/notable_and_taggable'

RSpec.describe PublicBody do
it_behaves_like 'concerns/notable', FactoryBot.build(:public_body)
it_behaves_like 'concerns/notable_and_taggable',
FactoryBot.build(:public_body)

describe <<-EOF.squish do
temporary tests for Globalize::ActiveRecord::InstanceMethods#read_attribute
Expand Down

0 comments on commit 1b19d18

Please sign in to comment.