diff --git a/app/controllers/admin/notes_controller.rb b/app/controllers/admin/notes_controller.rb new file mode 100644 index 0000000000..34d4f5ed93 --- /dev/null +++ b/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 diff --git a/app/controllers/admin/tags_controller.rb b/app/controllers/admin/tags_controller.rb index 9ddef6465f..d1133452ae 100644 --- a/app/controllers/admin/tags_controller.rb +++ b/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) diff --git a/app/helpers/admin/tag_helper.rb b/app/helpers/admin/tag_helper.rb index 1791f69959..136aeef377 100644 --- a/app/helpers/admin/tag_helper.rb +++ b/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 diff --git a/app/models/concerns/notable.rb b/app/models/concerns/notable.rb new file mode 100644 index 0000000000..a9f1739cb7 --- /dev/null +++ b/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 diff --git a/app/models/note.rb b/app/models/note.rb new file mode 100644 index 0000000000..ff8a5bc1b5 --- /dev/null +++ b/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 diff --git a/app/models/public_body.rb b/app/models/public_body.rb index 2eafe753ee..17e31be383 100644 --- a/app/models/public_body.rb +++ b/app/models/public_body.rb @@ -36,6 +36,7 @@ class PublicBody < ApplicationRecord include AdminColumn include Taggable + include Notable class ImportCSVDryRun < StandardError; end diff --git a/app/views/admin/notes/_form.html.erb b/app/views/admin/notes/_form.html.erb new file mode 100644 index 0000000000..48e2bb5098 --- /dev/null +++ b/app/views/admin/notes/_form.html.erb @@ -0,0 +1,38 @@ +<%= foi_error_messages_for :note %> + +
+ <% 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 %> +
+ +
+ + +
+ <% @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 %> +
+
diff --git a/app/views/admin/notes/_locale_fields.html.erb b/app/views/admin/notes/_locale_fields.html.erb new file mode 100644 index 0000000000..ebc0b5dcc1 --- /dev/null +++ b/app/views/admin/notes/_locale_fields.html.erb @@ -0,0 +1,16 @@ +
+
+ <%= t.hidden_field :locale, :value => locale %> + + <%= t.label :body, class: 'control-label' %> +
+ <% if AlaveteliLocalization.default_locale?(locale) && t.object.errors[:body].any? %> + + <% end %> + <%= t.text_area :body, class: 'span6', rows: 10 %> + <% if AlaveteliLocalization.default_locale?(locale) && t.object.errors[:body].any? %> + + <%end %> +
+
+
diff --git a/app/views/admin/notes/_note.html.erb b/app/views/admin/notes/_note.html.erb new file mode 100644 index 0000000000..0607754d93 --- /dev/null +++ b/app/views/admin/notes/_note.html.erb @@ -0,0 +1,24 @@ +
+
+ + <%= 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') %> + + + +
+ <%= tag.div id: dom_id(note), class: 'item-detail accordion-body collapse row' do %> + <% note.for_admin_column do |name, value, type| %> +
+ + <%= name %> + + + <%= admin_value(value) %> + +
+ <% end %> + <% end %> +
diff --git a/app/views/admin/notes/_show.html.erb b/app/views/admin/notes/_show.html.erb new file mode 100644 index 0000000000..5eccb94753 --- /dev/null +++ b/app/views/admin/notes/_show.html.erb @@ -0,0 +1,31 @@ +
+ <% if notes.size > 0 %> + + + + + + + + + + <% notes.each do |note| %> + + + + + + + + <% end %> +
IDNotable IDNotable typeNotable tagActions
<%= h note.id %><%= h note.notable_id %><%= h note.notable_type %><%= h note.notable_tag %><%= link_to "Edit", edit_admin_note_path(note) %>
+ <% else %> +

None yet.

+ <% end %> +
+ +
+

+ <%= link_to "New note", new_admin_note_path(notable_type: notable.class, notable_id: notable), class: "btn btn-info" %> +

+
diff --git a/app/views/admin/notes/edit.erb b/app/views/admin/notes/edit.erb new file mode 100644 index 0000000000..0ad11316c6 --- /dev/null +++ b/app/views/admin/notes/edit.erb @@ -0,0 +1,21 @@ +
+
+ +
+
+ +<%= form_for [:admin, @note], class: 'form form-horizontal' do |f| %> + <%= render partial: 'form', locals: { f: f } %> +
+ <%= submit_tag 'Save', class: 'btn btn-success' %> +
+<% 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 %> diff --git a/app/views/admin/notes/new.html.erb b/app/views/admin/notes/new.html.erb new file mode 100644 index 0000000000..3dce3850c2 --- /dev/null +++ b/app/views/admin/notes/new.html.erb @@ -0,0 +1,14 @@ +
+
+ +
+
+ +<%= form_for [:admin, @note], class: 'form form-horizontal' do |f| %> + <%= render partial: 'form', locals: { f: f } %> +
+ <%= submit_tag 'Create', class: 'btn btn-success' %> +
+<% end %> diff --git a/app/views/admin/tags/_notes.html.erb b/app/views/admin/tags/_notes.html.erb new file mode 100644 index 0000000000..27c63546b6 --- /dev/null +++ b/app/views/admin/tags/_notes.html.erb @@ -0,0 +1,13 @@ +

Notes

+ +
+
+ <%= link_to 'New note', new_admin_note_path(notable_tag: @tag), class: 'btn' %> +
+
+ +<% if @notes.empty? %> +

No notes.

+<% else %> + <%= render partial: 'tagging', collection: @notes %> +<% end %> diff --git a/app/views/admin/tags/_tagging.html.erb b/app/views/admin/tags/_tagging.html.erb index 9a9d2f2a37..3079577a5d 100644 --- a/app/views/admin/tags/_tagging.html.erb +++ b/app/views/admin/tags/_tagging.html.erb @@ -14,6 +14,11 @@ snippet: tagging } %> +<% when Note %> + <%= render partial: 'admin/notes/note', locals: { + note: tagging + } %> + <% else %>
<%= tagging.to_json %>
<% end %> diff --git a/app/views/admin/tags/show.html.erb b/app/views/admin/tags/show.html.erb index ffc7813e69..4dc9bbd620 100644 --- a/app/views/admin/tags/show.html.erb +++ b/app/views/admin/tags/show.html.erb @@ -1,5 +1,7 @@

<%= @title = "Tag – #{@tag}" %>

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

Taggings

diff --git a/app/views/admin_public_body/show.html.erb b/app/views/admin_public_body/show.html.erb index a21205d719..48a4a8df79 100644 --- a/app/views/admin_public_body/show.html.erb +++ b/app/views/admin_public_body/show.html.erb @@ -131,3 +131,11 @@

Censor rules

<%= render :partial => 'admin_censor_rule/show', :locals => { :censor_rules => @public_body.censor_rules, :public_body => @public_body } %> + +
+ +

Notes

+ +<%= render partial: 'admin/notes/show', + locals: { notes: @public_body.all_notes, + notable: @public_body } %> diff --git a/config/routes.rb b/config/routes.rb index ed94afd313..e070c8726f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -485,6 +485,22 @@ end #### + #### AdminNote controller + namespace :admin do + resources :notes, except: [:index, :show] + end + + direct :admin_note_parent do |note| + if note.notable_tag + admin_tag_path(tag: note.notable_tag) + elsif note.notable + url_for([:admin, note.notable]) + else + admin_general_index_path + end + end + #### + #### AdminPublicBody controller scope '/admin', :as => 'admin' do resources :bodies, @@ -498,6 +514,9 @@ :only => [:new, :create] end end + direct :admin_public_body do |pb| + admin_body_path(pb) + end #### #### AdminPublicBodyCategory controller @@ -569,6 +588,9 @@ :only => [:new, :create] end end + direct :admin_info_request do |ir| + admin_request_path(ir) + end #### #### AdminComment controller diff --git a/db/migrate/20220720085105_create_notes.rb b/db/migrate/20220720085105_create_notes.rb new file mode 100644 index 0000000000..5e50edb3b8 --- /dev/null +++ b/db/migrate/20220720085105_create_notes.rb @@ -0,0 +1,19 @@ +class CreateNotes < ActiveRecord::Migration[6.1] + def change + create_table :notes do |t| + t.references :notable, polymorphic: true + t.string :notable_tag + t.timestamps + end + + reversible do |dir| + dir.up do + Note.create_translation_table!(body: :text) + end + + dir.down do + Note.drop_translation_table! + end + end + end +end diff --git a/spec/controllers/admin/notes_controller_spec.rb b/spec/controllers/admin/notes_controller_spec.rb new file mode 100644 index 0000000000..85f6c0bc7c --- /dev/null +++ b/spec/controllers/admin/notes_controller_spec.rb @@ -0,0 +1,232 @@ +require 'spec_helper' + +RSpec.describe Admin::NotesController do + before(:each) { basic_auth_login(@request) } + + describe 'GET new' do + before { get :new } + + it 'returns a successful response' do + expect(response).to be_successful + end + + it 'assigns the note' do + expect(assigns[:note]).to be_a(Note) + end + + it 'renders the correct template' do + expect(response).to render_template(:new) + end + end + + describe 'POST #create' do + before do + post :create, params: params + end + + shared_context 'successful create' do + it 'assigns the note' do + expect(assigns[:note]).to be_a(Note) + end + + it 'creates the note' do + expect(assigns[:note].body).to eq('New body') + end + + 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 } + + 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 + + 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: '' } } + end + + it 'assigns the note' do + expect(assigns[:note]).to be_a(Note) + end + + it 'does not create the note' do + expect(assigns[:note]).to be_new_record + end + + it 'renders the form again' do + expect(response).to render_template(:new) + end + end + end + + describe 'GET edit' do + let!(:note) { FactoryBot.create(:note) } + + before { get :edit, params: { id: note.id } } + + it 'returns a successful response' do + expect(response).to be_successful + end + + it 'assigns the note' do + expect(assigns[:note]).to eq(note) + end + + it 'renders the correct template' do + expect(response).to render_template(:edit) + end + end + + describe 'PATCH #update' do + let!(:note) { FactoryBot.create(:note) } + + before do + patch :update, params: params + end + + shared_context 'successful update' do + it 'assigns the note' do + expect(assigns[:note]).to eq(note) + end + + it 'updates the note' do + expect(note.reload.body).to eq('New body') + end + + 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 public body admin' do + expect(response).to redirect_to(admin_public_body_path(public_body)) + 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: '' } } + end + + it 'assigns the note' do + expect(assigns[:note]).to eq(note) + end + + it 'does not update the note' do + expect(note.reload.body).not_to be_blank + end + + it 'renders the form again' do + expect(response).to render_template(:edit) + end + end + end + + describe 'DELETE #destroy' do + let!(:note) { FactoryBot.create(:note) } + + it 'destroys the note' do + allow(Note).to receive(:find).and_return(note) + expect(note).to receive(:destroy) + delete :destroy, params: { id: note.id } + end + + it 'sets a notice' do + delete :destroy, params: { id: note.id } + expect(flash[:notice]).to eq('Note successfully destroyed.') + end + + 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 + + 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 diff --git a/spec/controllers/admin/tags_controller_spec.rb b/spec/controllers/admin/tags_controller_spec.rb index 833480fec8..756e04c0e4 100644 --- a/spec/controllers/admin/tags_controller_spec.rb +++ b/spec/controllers/admin/tags_controller_spec.rb @@ -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 diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb new file mode 100644 index 0000000000..b4f042430f --- /dev/null +++ b/spec/factories/notes.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 +# + +FactoryBot.define do + 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 diff --git a/spec/helpers/admin/tag_helper_spec.rb b/spec/helpers/admin/tag_helper_spec.rb index 7a9153919d..a26de9b07c 100644 --- a/spec/helpers/admin/tag_helper_spec.rb +++ b/spec/helpers/admin/tag_helper_spec.rb @@ -42,5 +42,30 @@ to eq(expected) end end + + context 'tag with no value, as a string' do + let(:record_tag) { 'foo' } + + it 'renders the tag with a link' do + expected = '' \ + 'foo' \ + '' + expect(helper.render_tag(record_tag)). + to eq(expected) + end + end + + context 'tag with a value, as a string' do + let(:record_tag) { 'foo:bar' } + + it 'renders the tag with a link' do + expected = '' \ + 'foo:' \ + 'bar' \ + '' + expect(helper.render_tag(record_tag)). + to eq(expected) + end + end end end diff --git a/spec/models/concerns/notable.rb b/spec/models/concerns/notable.rb new file mode 100644 index 0000000000..c941749878 --- /dev/null +++ b/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 diff --git a/spec/models/concerns/notable_and_taggable.rb b/spec/models/concerns/notable_and_taggable.rb new file mode 100644 index 0000000000..edf9e13a7d --- /dev/null +++ b/spec/models/concerns/notable_and_taggable.rb @@ -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 diff --git a/spec/models/note_spec.rb b/spec/models/note_spec.rb new file mode 100644 index 0000000000..0b82d4d852 --- /dev/null +++ b/spec/models/note_spec.rb @@ -0,0 +1,62 @@ +# == 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 +# + +require 'spec_helper' + +RSpec.describe Note, type: :model do + let(:note) { FactoryBot.build(:note) } + + describe 'validations' do + specify { expect(note).to be_valid } + + it 'requires body' do + note.body = nil + expect(note).not_to be_valid + end + + 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 + + describe 'translations' do + before { note.save! } + + it 'adds translated body' do + expect(note.body_translations).to_not include(es: 'body') + AlaveteliLocalization.with_locale(:es) { note.body = 'body' } + expect(note.body_translations).to include(es: 'body') + end + end + + describe 'associations' do + context 'when info request cited' do + let(:note) { FactoryBot.build(:note, :for_public_body) } + + it 'belongs to a public body via polymorphic notable' do + expect(note.notable).to be_a PublicBody + end + end + end +end diff --git a/spec/models/public_body_spec.rb b/spec/models/public_body_spec.rb index 26ec36b015..c1bfe0a7a3 100644 --- a/spec/models/public_body_spec.rb +++ b/spec/models/public_body_spec.rb @@ -29,8 +29,13 @@ # 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