physionet-build
physionet-build copied to clipboard
Add invite event host button on user management page
This PR partially addresses #2104 by adding a button to the user management page that is able to add a user to the event host permission group. I also added a pop-up after clicking the button to ensure the admin is committed to adding the user as an event host.
Two additional changes I plan to make to follow-up on this:
- Add a 'remove' event host button for circumstances where we need to remove someone's permissions
- Add an email option on click which will send instructions on the events functionality to the user
Thanks for working on this! There are a couple of problems I see with the implementation here:
- When someone submits a form, you always need to check that the request method was POST. (The only exception is a "search" form, which can use the GET method because it doesn't have any lasting effect on the server.)
Think of it this way: if it's a POST request, we know that the form was actually submitted by the given user (request.user
). Other types of HTTP requests do not have that guarantee, and can easily be spoofed.
So any view that does a save
or create
or delete
, or anything like that, always needs to check if request.method == 'POST'
before doing so. You can find lots of examples of this throughout the code.
- We only want to allow certain authorized people to add event hosts, which is why we want the
@permission_required
decorator here. But I think the permission should be something more specific thanuser.view_all_events
. I'm not sure if any of the existing permissions are suitable or if we should add a new permission to cover this case.
Thanks for your feedback, @bemoody.
1. When someone submits a form, you **always need** to check that the request method was POST. (The only exception is a "search" form, which can use the GET method because it doesn't have any lasting effect on the server.)
I have since added this to the view.
user.view_all_events
. I'm not sure if any of the existing permissions are suitable or if we should add a new permission to cover this case.
I'm not sure either, it may be worth adding a new permission? I mainly copied the permission from the event_management function as it seemed the most relatable permission group to assign to this function.
@mscanlan-git I think we probably want to create a permission specifically for the ability to invite hosts. You could do this by adding the permission to the Event model, e.g.:
class Event(models.Model):
"""
Captures information on events such as datathons, workshops and classes.
Used to allow event hosts to assist with credentialing.
"""
title = models.CharField(max_length=128)
description = SafeHTMLField(max_length=10000, blank=True, null=True)
category = models.CharField(choices=EventCategory.choices, max_length=32)
host = models.ForeignKey("user.User", on_delete=models.CASCADE)
added_datetime = models.DateTimeField(auto_now_add=True)
start_date = models.DateField(default=timezone.now)
end_date = models.DateField(default=timezone.now)
slug = models.SlugField(unique=True)
allowed_domains = models.CharField(blank=True, null=True, validators=[
validators.validate_domain_list], max_length=100)
class Meta:
unique_together = ('title', 'host')
permissions = [('view_all_events', 'Can view all events in the console'),
('invite_event_host', 'Can grant event host status to a user'),
('view_event_menu', 'Can view event menu in the navbar')]
Some other notes:
- After modifying the permission, you would need to generate a migration with
python manage.py makemigrations
and then apply the migration withpython manage.py migrate
. - For testing, you can assign the
invite_event_host
permission to a user from the console at https://physionet.org/admin/user/user/ (log in as admin). - We would need to modify the "Invite event host" button to make it clear who can and cannot use it (e.g. deactivate it or hide it for people without the
invite_event_host
permission? - Currently the
invite_event_host
is displayed and active for users who are already event hosts. What is the desired behaviour in this situation?
Thanks @tompollard, many of these changes have been added.
@mscanlan-git I think we probably want to create a permission specifically for the ability to invite hosts. You could do this by adding the permission to the Event model, e.g.:
class Event(models.Model): """ Captures information on events such as datathons, workshops and classes. Used to allow event hosts to assist with credentialing. """ title = models.CharField(max_length=128) description = SafeHTMLField(max_length=10000, blank=True, null=True) category = models.CharField(choices=EventCategory.choices, max_length=32) host = models.ForeignKey("user.User", on_delete=models.CASCADE) added_datetime = models.DateTimeField(auto_now_add=True) start_date = models.DateField(default=timezone.now) end_date = models.DateField(default=timezone.now) slug = models.SlugField(unique=True) allowed_domains = models.CharField(blank=True, null=True, validators=[ validators.validate_domain_list], max_length=100) class Meta: unique_together = ('title', 'host') permissions = [('view_all_events', 'Can view all events in the console'), ('invite_event_host', 'Can grant event host status to a user'), ('view_event_menu', 'Can view event menu in the navbar')]
Some other notes:
- After modifying the permission, you would need to generate a migration with
python manage.py makemigrations
and then apply the migration withpython manage.py migrate
.
I have added the migration file for this change, as well as add the decorator @permission_required('user.invite_event_host')
- We would need to modify the "Invite event host" button to make it clear who can and cannot use it (e.g. deactivate it or hide it for people without the
invite_event_host
permission?
For this, I added {% if perms.self.invite_event_host %}
in the template to make sure only users with the permission can view the button.
- Currently the
invite_event_host
is displayed and active for users who are already event hosts. What is the desired behaviour in this situation?
This is the last bit I'm missing. I have tried numerous ways to enforce this, but the result seems to be binary each time (either the button is hidden for both event host and non-event host users, or vice versa). I committed the changes I made to add template tags, but I would like help as to how I can get this implemented conditionally.