search

carpentries / amy

A web-based workshop administration application built using Django.
https://amy.carpentries.org
MIT License
114 stars 72 forks source link

Error text when creating Instructor task from recruitment #2449

Open maneesha opened 1 year ago

maneesha commented 1 year ago

As identified in this private slack conversation:

When an admin tries to accept an instructor signup to create an instructor task, if that instructor already has an Instructor task for that event, we get this not very useful error: image

We should consider how we handle a signup if the Person already has an Instructor task for that event. Suggestion: Clicking "accept" gives a notification that the Person already has an instructor task, and maps that acceptance to that task. Clicking "decline" gives an error, requiring the admin to remove the associated Instructor task before continuing.

Tagging @danielle06 and @elichad for feedback.

elichad commented 1 year ago

Technical notes:

This error arises when an IntegrityError occurs while handling a signup: https://github.com/carpentries/amy/blob/39f9544c8edfc50a3abc9a3fc5d07f77d7d71cfd/amy/recruitment/views.py#L375-L396

I think we can expect most (if not all) of these IntegrityErrors to come from a violation of our constraint that any Task should have a unique combination of event, person, and role (i.e. instructor already has a task for this event). The other way would be for the Task to be provided with a non-existent event/person/role key.

In any case, we should be able to perform this check for an existing task as part of this try/catch loop for the "accept" part, or just as part of the add_instructor_task method. For "decline," we already quietly remove the associated Instructor task if present.

maneesha commented 1 year ago

On decline, we should not quietly remove the associated Instructor task if present -- there should be an explicit warning that this is happening and the recruitment state should change to decline.

On accept, we should have a notification that the task already exists and the recruitment state should change to accepted. (the second part looks like it is already happening)

danielle06 commented 1 year ago

Hi @maneesha and @elichad , thank you so much for prioritizing this. The changes to the process, outlined in Maneesha's last comment, make the workflow very clear and uncomplicated.