diff --git a/app/controllers/admin/member_notes_controller.rb b/app/controllers/admin/member_notes_controller.rb index 2f2e4f0b7..45aa8662d 100644 --- a/app/controllers/admin/member_notes_controller.rb +++ b/app/controllers/admin/member_notes_controller.rb @@ -1,4 +1,6 @@ class Admin::MemberNotesController < Admin::ApplicationController + before_action :authorize_note, only: %i[update destroy] + def create @note = MemberNote.new(member_note_params) authorize @note @@ -8,6 +10,32 @@ def create redirect_back fallback_location: root_path end + def edit; end + + def update + if @note.update(member_note_params) + flash[:notice] = 'Note successfully updated.' + redirect_to admin_member_path(@note.member) + else + flash[:error] = @note.errors.full_messages unless @note.save + redirect_back fallback_location: root_path + end + end + + def destroy + if @note.destroy + flash[:notice] = 'Note successfully deleted.' + else + flash[:error] = 'Failed to delete note.' + end + redirect_back fallback_location: root_path + end + + def authorize_note + @note = MemberNote.find(params[:id]) + authorize @note + end + def member_note_params params.require(:member_note).permit(:note, :member_id) end diff --git a/app/policies/member_note_policy.rb b/app/policies/member_note_policy.rb index 7b8baa327..bdf428602 100644 --- a/app/policies/member_note_policy.rb +++ b/app/policies/member_note_policy.rb @@ -2,4 +2,12 @@ class MemberNotePolicy < ApplicationPolicy def create? user && (user.has_role?(:admin) || user.roles.where(resource_type: 'Chapter').any?) end + + def destroy? + user && (user.has_role?(:admin) || user == record.author) + end + + def update? + user && (user.has_role?(:admin) || user == record.author) + end end diff --git a/app/views/admin/member_notes/_member_note.html.haml b/app/views/admin/member_notes/_member_note.html.haml index 741db5844..6546e0cf7 100644 --- a/app/views/admin/member_notes/_member_note.html.haml +++ b/app/views/admin/member_notes/_member_note.html.haml @@ -1,10 +1,14 @@ .row.d-flex.align-items-start.note .col-2.col-md-1 - %span.fa-stack.text-primary - %i.fas.fa-circle.fa-stack-2x - %i.fas.fa-pencil-alt.fa-stack-1x.fa-inverse + = link_to '#', class: "btn btn-sm p-0 me-2", turbo: false, 'data-bs-toggle': 'modal', 'data-bs-target': "#edit-note-modal-#{action.id}" do + %span.fa-stack.text-primary + %i.fas.fa-circle.fa-stack-2x + %i.fas.fa-pencil-alt.fa-stack-1x.fa-inverse .col-9.col-md-11 %strong Note added by #{link_to(action.author.full_name, admin_member_path(action.author))} %blockquote.blockquote.mb-0=action.note - .date + .d-flex.align-items-center.justify-content-between = l(action.created_at, format: :website_format) + = link_to admin_member_note_path(action), method: :delete, data: { confirm: "Are you sure you want to delete this note?" }, class: "btn btn-sm btn-outline-danger" do + %i.fas.fa-trash + = render partial: 'note', locals: { note: action, member: @member } \ No newline at end of file diff --git a/app/views/admin/members/_actions.html.haml b/app/views/admin/members/_actions.html.haml index 924a58094..94e3e7af6 100644 --- a/app/views/admin/members/_actions.html.haml +++ b/app/views/admin/members/_actions.html.haml @@ -17,4 +17,4 @@ = link_to new_admin_member_ban_path(@member), class: 'btn btn-primary d-block py-4 rounded-0' do %i.fas.fa-ban.warning.me-2 Suspend - = render 'note' + = render partial: 'note', locals: { note: MemberNote.new, member: @member } diff --git a/app/views/admin/members/_note.html.haml b/app/views/admin/members/_note.html.haml index 57e16ec9a..0e10f87ce 100644 --- a/app/views/admin/members/_note.html.haml +++ b/app/views/admin/members/_note.html.haml @@ -1,11 +1,12 @@ -#note-modal.modal.fade{ 'aria-labelledby': 'modal-title', 'aria-hidden': 'true', role: 'dialog' } +.modal.fade{ id: "#{note.persisted? ? "edit-note-modal-#{note.id}" : 'note-modal'}", 'aria-labelledby': 'modal-title', 'aria-hidden': 'true', role: 'dialog' } .modal-dialog .modal-content .modal-header - %h5.modal-title#modal-title Add a note for #{@member.full_name} + %h5.modal-title#modal-title + = note.persisted? ? "Update note for #{member.full_name}" : "Add a note for #{member.full_name}" %button.btn-close{ type: 'button', 'data-bs-dismiss': 'modal', 'aria-label': 'Close' } .modal-body - = simple_form_for [:admin, MemberNote.new], html: { class: 'form-inline' } do |f| + = simple_form_for [:admin, note], html: { class: 'form-inline' } do |f| = f.input :note, label: false, input_html: { rows: 3 }, placeholder: 'e.g. very enthusiastic student.' = f.hidden_field :member_id, value: @member.id .text-right diff --git a/config/routes.rb b/config/routes.rb index f5b29ccb0..05193f76f 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -91,7 +91,7 @@ resources :bans, only: %i[index new create] end - resources :member_notes, only: [:create] + resources :member_notes, only: [:create, :destroy, :edit, :update] resources :chapters, only: %i[index new create show edit update] do get :members diff --git a/spec/controllers/admin/member_notes_controller_spec.rb b/spec/controllers/admin/member_notes_controller_spec.rb index 35f9b0a0d..764073b1f 100644 --- a/spec/controllers/admin/member_notes_controller_spec.rb +++ b/spec/controllers/admin/member_notes_controller_spec.rb @@ -35,4 +35,60 @@ end.not_to change { MemberNote.all.count } end end + + describe 'PATCH #update' do + let!(:member_note) { Fabricate(:member_note, member: member, author: admin, note: 'Original note') } + + it "Doesn't allow anonymous users to edit notes" do + patch :update, params: { id: member_note.id, member_note: { note: 'Updated anonymously' } } + expect(member_note.reload.note).to eq('Original note') + end + + it "Doesn't allow regular users to edit notes" do + login member + + patch :update, params: { id: member_note.id, member_note: { note: 'Updated by member' } } + expect(member_note.reload.note).to eq('Original note') + end + + it 'Allows admin to edit notes' do + login admin + + patch :update, params: { id: member_note.id, member_note: { note: 'Updated by admin' } } + expect(member_note.reload.note).to eq('Updated by admin') + end + + it "Doesn't allow notes to be updated to be blank" do + login admin + + patch :update, params: { id: member_note.id, member_note: { note: '' } } + expect(member_note.reload.note).to eq('Original note') + end + end + + describe 'DELETE #destroy' do + let!(:member_note) { Fabricate(:member_note, member: member, author: admin, note: 'Note') } + + it "Doesn't allow anonymous users to delete notes" do + expect do + delete :destroy, params: { id: member_note.id } + end.not_to change { MemberNote.all.count } + end + + it "Doesn't allow regular users to delete notes" do + login member + + expect do + delete :destroy, params: { id: member_note.id } + end.not_to change { MemberNote.all.count } + end + + it 'Allows note owner to delete notes' do + login admin + + expect do + delete :destroy, params: { id: member_note.id } + end.to change { MemberNote.count }.by(-1) + end + end end