Skip to content

Edit and Delete Member notes #2208

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions app/controllers/admin/member_notes_controller.rb
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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
Expand Down
8 changes: 8 additions & 0 deletions app/policies/member_note_policy.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
12 changes: 8 additions & 4 deletions app/views/admin/member_notes/_member_note.html.haml
Original file line number Diff line number Diff line change
@@ -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 }
2 changes: 1 addition & 1 deletion app/views/admin/members/_actions.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
7 changes: 4 additions & 3 deletions app/views/admin/members/_note.html.haml
Original file line number Diff line number Diff line change
@@ -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
Expand Down
2 changes: 1 addition & 1 deletion config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
56 changes: 56 additions & 0 deletions spec/controllers/admin/member_notes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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