Skip to content

Allow admins to edit and delete member notes #2276

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

Closed
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
24 changes: 24 additions & 0 deletions app/controllers/admin/member_notes_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,30 @@ def create
redirect_back fallback_location: root_path
end

def update
@note = MemberNote.find(params[:id])
authorize @note

if @note.update(member_note_params)
flash[:notice] = 'Note updated successfully.'
else
flash[:error] = @note.errors.full_messages.to_sentence
end

redirect_back fallback_location: root_path
end

def destroy
@note = MemberNote.find(params[:id])
authorize @note
if @note.destroy
flash[:notice] = 'Note deleted successfully.'
else
flash[:alert] = 'Failed to delete the note.'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems a bit more natural to me without the.

Suggested change
flash[:alert] = 'Failed to delete the note.'
flash[:alert] = 'Failed to delete note.'

end
redirect_back fallback_location: root_path
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 update?
user&.has_role?(:admin)
end

def destroy?
user&.has_role?(:admin)
end
end
17 changes: 16 additions & 1 deletion app/views/admin/member_notes/_member_note.html.haml
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,23 @@
%span.fa-stack.text-primary
%i.fas.fa-circle.fa-stack-2x
%i.fas.fa-pencil-alt.fa-stack-1x.fa-inverse
.col-2.col-md-1
= link_to 'Edit', "#edit-note-modal-#{action.id}", data: { bs_toggle: 'modal', bs_target: "#edit-note-modal-#{action.id}" }
.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
%blockquote.blockquote.mb-0= action.note
.date
= l(action.created_at, format: :website_format)

.modal.fade{ id: "edit-note-modal-#{action.id}", tabindex: "-1", role: "dialog" }
.modal-dialog
.modal-content
.modal-header
%h5.modal-title Edit Note for ID #{action.id}
%button.btn-close{ type: 'button', 'data-bs-dismiss': 'modal', 'aria-label': 'Close' }
.modal-body
= simple_form_for [:admin, member_note], html: { class: 'form-inline' } do |f|
= f.input :note, label: false, input_html: { rows: 3 }
= f.hidden_field :member_id, value: member_note.member_id
.text-right
= f.button :submit, 'Save note', class: 'btn btn-primary mb-0'
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: %i[create update destroy]

resources :chapters, only: %i[index new create show edit update] do
get :members
Expand Down
47 changes: 47 additions & 0 deletions spec/controllers/admin/member_notes_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,4 +33,51 @@
end.not_to change { MemberNote.all.count }
end
end

describe 'PATCH #update' do
it "Doesn't allow anonymous users to update notes" do
patch :update, params: { id: member_note.id, member_note: { note: 'Updated note' } }
expect(response).to redirect_to(root_path)
end

it "Doesn't allow regular users to update notes" do
login member
patch :update, params: { id: member_note.id, member_note: { note: 'Updated note' } }
expect(response).to redirect_to(root_path)
end

it 'Allows chapter organisers to update notes' do
login admin
patch :update, params: { id: member_note.id, member_note: { note: 'Updated note' } }
expect(response).to redirect_to(root_path)
expect(flash[:notice]).to eq('Note updated successfully.')
end

it "Doesn't allow blank notes to be updated" do
login admin
patch :update, params: { id: member_note.id, member_note: { note: ' ' } }
expect(response).to redirect_to(root_path)
expect(flash[:error]).to eq("Note can't be blank")
end
end

describe 'DELETE #destroy' do
it "Doesn't allow anonymous users to delete notes" do
delete :destroy, params: { id: member_note.id }
expect(response).to redirect_to(root_path)
end

it "Doesn't allow regular users to delete notes" do
login member
delete :destroy, params: { id: member_note.id }
expect(response).to redirect_to(root_path)
end

it 'Allows chapter organisers to delete notes' do
login admin
delete :destroy, params: { id: member_note.id }
expect(response).to redirect_to(root_path)
expect(flash[:notice]).to eq('Note deleted successfully.')
end
end
end
Loading