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 10 commits into
base: master
Choose a base branch
from

Conversation

jazzygasper
Copy link
Contributor

@jazzygasper jazzygasper commented Apr 24, 2025

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
Screenshot 2025-04-24 at 13 39 25 Screenshot 2025-04-24 at 13 38 40 Screenshot 2025-04-24 at 13 37 31 Screenshot 2025-04-24 at 13 37 56 Screenshot 2025-04-24 at 13 38 12

user && (user.has_role?(:admin) || user == record.author)
end

def update?
Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@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?

Copy link
Contributor

Choose a reason for hiding this comment

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

@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.

Copy link
Contributor

Choose a reason for hiding this comment

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

@jazzygasper are you okay to make the suggested changes?

Copy link
Contributor

@jonodrew jonodrew left a comment

Choose a reason for hiding this comment

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

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?

@@ -2,4 +2,13 @@ class MemberNotePolicy < ApplicationPolicy
def create?
user && (user.has_role?(:admin) || user.roles.where(resource_type: 'Chapter').any?)
end

def destroy?
puts "Chapters: #{record.member.chapters.inspect}"
Copy link
Contributor

Choose a reason for hiding this comment

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

@jazzygasper can we remove the puts statement please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Unable to Edit or Delete Notes on Student/Coach Profiles
3 participants