Edit and Delete Member notes

[jazzygasper]

Fixes #2176

This PR adds functionality for admin to edit and delete notes associated with members.

Features:

• Note pencil icon reused for edit button and delete button added
• Modal interface for updating notes inline
• Validation to prevent saving empty notes
• Authorisation checks to ensure only note authors or admins can edit/delete notes
• Flash messages for success and failure feedback

[matyikriszta]

@jazzygasper just one quick question. Is the intention here to allow admins and organisers to update the note of another admin/organiser? In that case this won't work for organisers, only admins. See line 3 above in the create? method. We need to check for the admin and organiser roles to allow for that.

If the intention was to allow admins to update anyone's notes but organisers to only update their own notes, this is fine.

Just wanted to clarify before I approved.

[jazzygasper]

@matyikriszta Yeah my original idea was for admins to update anyone’s notes, and organisers only their own. But I'm happy to change it so any organiser can update any note, or restrict it to organisers updating notes from their own chapter. What do you think is best?

[matyikriszta]

@jazzygasper I think we should go with the second option, e.g. organisers updating notes from their own chapter. I think this might be useful in the case if an organiser leaves but a note they left needs to be updated. Other organisers from the same chapter should be able to do this without having to ping an admin.

[jonodrew]

@jazzygasper are you okay to make the suggested changes?

[jonodrew]

Thank you for adding this. I think it's well on its way, but there are a couple of tests failing. I don't think you can access Chapter from MemberNote, so you might need to take a look at that. Maybe MemberNote.author.chapters will give you something?

[mroderick]

Thanks for this contribution — the core logic is solid and CI is passing, which is a great sign.

This PR needs a few things before it can merge. Here's what I've found:

Blocker:

• The branch is 11 months behind master. Rebasing will surface a conflict in member_note_params — master converted to params.expect since the PR was last synced.

Should fix:

• update action's else branch runs @note.save after @note.update already failed (dead code), and passes an array to flash[:error] instead of a string. Should be flash[:error] = @note.errors.full_messages.to_sentence.
• No policy specs for the new update? and destroy? methods — the PR adds the logic but no tests for it.
• edit route and def edit; end are dead code — nothing links to the edit page (edits use a modal). Remove :edit from routes and the empty action.

Nice-to-have:

• A Capybara feature spec exercising the modal edit/delete flow would be ideal for a UI feature like this.

Are you able to finish this off yourself, or would you like a hand getting it over the line?