Skip to content
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

Event: Allow host to add cohosts #1925

Closed
wants to merge 25 commits into from

Conversation

superryeti
Copy link
Contributor

@superryeti superryeti commented Mar 8, 2023

Related to #1901

Context

On #1901, we reverted the add cohost feature because of security issue.
On this PR we reimplement the feature by using forms

Cohosts are supposed to be helper for hosts and can manage the participants(approving, rejecting participants)
To be cohost, one has to be first approved to join the event.

Workflow

The event host clicks the Make Cohost or the Remove Cohost button, and the following will happen

  1. we will immediately disable the button(so that host cant spam the button), it also acts like a feedback to host that the task is processing.
  2. Then we send Ajax request to our view
  3. Depending on the response from view,
    a. If successful, we will either change the button from Make Cohost to Remove Cohost or vice versa(depending on whether the host was trying to make cohost or remove cohost)
    b. If unsuccessful, we will enable the button again to allow host to try again.

Screen Record for quick review

Screen.Recording.3-8-2023.at.05.01.PM.webm

@superryeti superryeti force-pushed the au/event/feature/add_cohosts branch from a0591d8 to 22e430d Compare March 8, 2023 22:04
@superryeti superryeti marked this pull request as draft March 8, 2023 22:04
@superryeti superryeti force-pushed the au/event/feature/add_cohosts branch from 22e430d to 98ece89 Compare March 8, 2023 22:10
@bemoody
Copy link
Collaborator

bemoody commented Mar 10, 2023

What is a "cohost"? I feel like that question has not been answered.

Is "cohost" status a meaningless gold star? Or is it intended to confer some permissions on the person?

Why would we want to have "cohosts" rather than having multiple "hosts"?

@superryeti
Copy link
Contributor Author

What is a "cohost"? I feel like that question has not been answered.

Is "cohost" status a meaningless gold star? Or is it intended to confer some permissions on the person?

Why would we want to have "cohosts" rather than having multiple "hosts"?

My bad, let me explain
Cohost is a participant who will have some extra permission like approving/disapproving other participants, viewing the list of applications.

Why would we want to have "cohosts" rather than having multiple "hosts"?

The idea for multiple hosts never came up before during the discussion. Tagging @tompollard and @alistairewj for feedback on multiple hosts.

@alistairewj
Copy link
Member

I think we'd want to avoid multiple hosts to avoid having multiple users with permission to update event content (similar to the role of the corresponding author in projects).

@bemoody
Copy link
Collaborator

bemoody commented Mar 10, 2023

Okay, yeah, I see this was mentioned briefly in issue #1839.

This needs further discussion, I'm not really comfortable with being able to grant permissions over data access control so casually.

@bemoody
Copy link
Collaborator

bemoody commented Mar 10, 2023

@alistairewj: this is true, though only allowing a single person to edit is often annoying and it'd be better if we could avoid that.

@superryeti
Copy link
Contributor Author

superryeti commented Mar 10, 2023

@alistairewj @bemoody @tompollard i remember we wanted to meetup do discuss about Events anyway, I was trying to setup the meeting, but i was unable to because i couldn't get everyone's availability. So brining this up to setup meeting as looks like we need to discuss/plan bunch of things :)

Also sorry i couldn't find all 3 of your on Slack so brought this up here

@superryeti superryeti force-pushed the au/event/feature/add_cohosts branch from 98ece89 to bfe40e4 Compare March 10, 2023 19:32
@superryeti
Copy link
Contributor Author

Email templates for reference

For Make Cohost

physionet-build-dev-1   | [10/Mar/2023 14:28:38] "GET /events/ HTTP/1.1" 200 31822
physionet-build-dev-1   | Content-Type: text/plain; charset="utf-8"
physionet-build-dev-1   | MIME-Version: 1.0
physionet-build-dev-1   | Content-Transfer-Encoding: 7bit
physionet-build-dev-1   | Subject: DataShare Event Cohost Status Change
physionet-build-dev-1   | From: localhost@localhost
physionet-build-dev-1   | To: amitupreti@fake_gmail.com
physionet-build-dev-1   | Date: Fri, 10 Mar 2023 19:28:41 -0000
physionet-build-dev-1   | Message-ID: <167847652126.73.15411785239375743028@f0b72aed4da1>
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | Dear Amit Upreti,
physionet-build-dev-1   | 
physionet-build-dev-1   | You have been added as Cohost to the Event : Test Event physionet.
physionet-build-dev-1   | 
physionet-build-dev-1   | You can now manage the event participants from your events dashboard.
physionet-build-dev-1   | 
physionet-build-dev-1   | You can view further information about the event using the following
physionet-build-dev-1   | link:
physionet-build-dev-1   | http://localhost:8000/events/PsenZXNhPluI/
physionet-build-dev-1   | 
physionet-build-dev-1   | Regards
physionet-build-dev-1   | The DataShare Team
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | -------------------------------------------------------------------------------
physionet-build-dev-1   | Content-Type: text/plain; charset="utf-8"
physionet-build-dev-1   | MIME-Version: 1.0
physionet-build-dev-1   | Content-Transfer-Encoding: 7bit
physionet-build-dev-1   | Subject: DataShare Event Cohost Status Change
physionet-build-dev-1   | From: localhost@localhost
physionet-build-dev-1   | To: amitupreti@fake_gmail.com
physionet-build-dev-1   | Date: Fri, 10 Mar 2023 19:28:41 -0000
physionet-build-dev-1   | Message-ID: <167847652126.73.5870397640578469008@f0b72aed4da1>
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | Dear Roger Greenwood Mark,
physionet-build-dev-1   | 
physionet-build-dev-1   | You have provided Amit Upreti Cohost access to the Event : Test Event
physionet-build-dev-1   | physionet.
physionet-build-dev-1   | 
physionet-build-dev-1   | If this was done by mistake, you can change the cohost status by
physionet-build-dev-1   | visiting the events dashboard.
physionet-build-dev-1   | 
physionet-build-dev-1   | http://localhost:8000/events/
physionet-build-dev-1   | 
physionet-build-dev-1   | Regards
physionet-build-dev-1   | The DataShare Team
physionet-build-dev-1   | 
physionet-build-dev-1   | 

For Remove Cohost

physionet-build-dev-1   | -------------------------------------------------------------------------------
physionet-build-dev-1   | [10/Mar/2023 14:28:41] "POST /events/manage_co_hosts/ HTTP/1.1" 200 40
physionet-build-dev-1   | Content-Type: text/plain; charset="utf-8"
physionet-build-dev-1   | MIME-Version: 1.0
physionet-build-dev-1   | Content-Transfer-Encoding: 7bit
physionet-build-dev-1   | Subject: DataShare Event Cohost Status Change
physionet-build-dev-1   | From: localhost@localhost
physionet-build-dev-1   | To: amitupreti@fake_gmail.com
physionet-build-dev-1   | Date: Fri, 10 Mar 2023 19:28:45 -0000
physionet-build-dev-1   | Message-ID: <167847652579.73.11830662540034523928@f0b72aed4da1>
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | Dear Amit Upreti,
physionet-build-dev-1   | 
physionet-build-dev-1   | Your Cohost access has been removed from the Event : Test Event
physionet-build-dev-1   | physionet.
physionet-build-dev-1   | 
physionet-build-dev-1   | You can view further information about the event using the following
physionet-build-dev-1   | link:
physionet-build-dev-1   | http://localhost:8000/events/PsenZXNhPluI/
physionet-build-dev-1   | 
physionet-build-dev-1   | Regards
physionet-build-dev-1   | The DataShare Team
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | -------------------------------------------------------------------------------
physionet-build-dev-1   | Content-Type: text/plain; charset="utf-8"
physionet-build-dev-1   | MIME-Version: 1.0
physionet-build-dev-1   | Content-Transfer-Encoding: 7bit
physionet-build-dev-1   | Subject: DataShare Event Cohost Status Change
physionet-build-dev-1   | From: localhost@localhost
physionet-build-dev-1   | To: amitupreti@fake_gmail.com
physionet-build-dev-1   | Date: Fri, 10 Mar 2023 19:28:45 -0000
physionet-build-dev-1   | Message-ID: <167847652580.73.11619867650555598756@f0b72aed4da1>
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | Dear Roger Greenwood Mark,
physionet-build-dev-1   | 
physionet-build-dev-1   | You have removed Amit Upreti's Cohost access from the Event : Test
physionet-build-dev-1   | Event physionet.
physionet-build-dev-1   | 
physionet-build-dev-1   | If this was done by mistake, you can change the cohost status by
physionet-build-dev-1   | visiting the events dashboard.
physionet-build-dev-1   | 
physionet-build-dev-1   | http://localhost:8000/events/
physionet-build-dev-1   | 
physionet-build-dev-1   | Regards
physionet-build-dev-1   | The DataShare Team
physionet-build-dev-1   | 
physionet-build-dev-1   | 
physionet-build-dev-1   | -------------------------------------------------------------------------------

@superryeti superryeti force-pushed the au/event/feature/add_cohosts branch 2 times, most recently from e9bed5c to 53ab4bb Compare March 10, 2023 19:38
@superryeti superryeti marked this pull request as ready for review March 10, 2023 20:19
@superryeti superryeti force-pushed the au/event/feature/add_cohosts branch from 53ab4bb to 1135c7d Compare March 15, 2023 15:40
@kshalot kshalot self-requested a review October 5, 2023 14:53
Copy link
Member

@kshalot kshalot left a comment

Choose a reason for hiding this comment

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

I did an initial review. There is a lot more coming up, but I wanted to give you some immediate pointers @Rutvikrj26

Copy link
Member

@kshalot kshalot left a comment

Choose a reason for hiding this comment

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

Sorry for a late review - I'm leaving some comments. If you need more context then let me know Rutvik, we can even jump on a call.

physionet-django/notification/utility.py Outdated Show resolved Hide resolved
physionet-django/events/urls.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
@kshalot
Copy link
Member

kshalot commented Nov 28, 2023

@Rutvikrj26 This requires a backmerge/rebase.

superryeti and others added 9 commits November 28, 2023 08:56
The view will be used by ajax request which will be sent from event_home.html.
Here, when event host clicks the `Make Cohost` or the `Remove Cohost` button,
1. we will immediately disable the button(so that host cant spam the button), it also acts like a feedback to host that the task is processing.
2. Then we send ajax request to our view
3. Depending on the response from view,
 a. If successfull, we will either change the button from `Make Cohost` to `Remove Cohost` or vice versa(depending on whether the host was trying to make cohost or remove cohost)
 b. If unsuccessfull, we will enable the button again to allow host to try again.
Whenever a host adds or removes a cohost to the event, email notification are sent to a) host and b) the added/removed cohost
@Rutvikrj26 Rutvikrj26 force-pushed the au/event/feature/add_cohosts branch from 693624a to fc4599c Compare November 28, 2023 14:44
physionet-django/events/models.py Outdated Show resolved Hide resolved
physionet-django/events/templates/events/event_home.html Outdated Show resolved Hide resolved
physionet-django/events/templates/events/event_home.html Outdated Show resolved Hide resolved
physionet-django/events/templates/events/event_home.html Outdated Show resolved Hide resolved
physionet-django/events/templates/events/event_home.html Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Outdated Show resolved Hide resolved
physionet-django/notification/utility.py Outdated Show resolved Hide resolved
@kshalot kshalot self-requested a review January 9, 2024 14:15
physionet-django/events/templates/events/event_home.html Outdated Show resolved Hide resolved
physionet-django/events/urls.py Outdated Show resolved Hide resolved
physionet-django/events/views.py Show resolved Hide resolved
physionet-django/events/views.py Show resolved Hide resolved
@Rutvikrj26
Copy link
Contributor

@kshalot @tompollard
I've implemented single endpoints for Add & Remove co-hosts. Please review and let me know your thoughts.

@tompollard
Copy link
Member

Sorry to say this so late in the day, but I feel like an "invite co-host" button somewhere might be cleaner (both for user and implementation-wise).

@Rutvikrj26
Copy link
Contributor

As discussed during the regular Standup, this feature implementation needs to be redone for security issues.
I'll be opening a new PR and implement an invite cohost button on the main event page.

@Rutvikrj26 Rutvikrj26 closed this Feb 6, 2024
bemoody pushed a commit that referenced this pull request May 13, 2024
This Pull Request implements the feature to add cohosts for an event.

This PR is a continuation of the Cohost PR : #1925

The feature has been implemented in a similar fashion to how the
author Invitation is handled currently.

Cohosts are supposed to be helper for hosts and can manage the
participants (approving, rejecting participants)
To be cohost, one has to be first approved to join the event.

Worflow:

When a host opens the event page, the host will be able to see the
option to invite a user as a cohost.
![event-cohost-form](https://github.com/MIT-LCP/physionet-build/assets/46196557/80246cc7-03a9-4c70-a32e-55a8f89cd01e)

The person invited gets an email for cohost Invitation:
![cohost-invitation-email](https://github.com/MIT-LCP/physionet-build/assets/46196557/6f4db845-7c27-46f1-a19d-df131e24c511)

The person sees the invitation on the specific event's page.
![cohost-request](https://github.com/MIT-LCP/physionet-build/assets/46196557/56faa1b0-6c4f-4b60-bec3-fb52c7de7b8f)
![cohost-request-accept](https://github.com/MIT-LCP/physionet-build/assets/46196557/6e976a09-b16a-4a40-9896-89dc20ec0cb1)

When the person accepts, the host gets an email that the person has
accepted the cohost invitation.

The form runs the check if the user is a participant in the event and
prohibits inviting a random user as the last one implemented.
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.

6 participants