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

What happens to old slots after a user has been removed? #173

Open
itsrachelfish opened this issue Jul 30, 2019 · 8 comments · May be fixed by #182
Open

What happens to old slots after a user has been removed? #173

itsrachelfish opened this issue Jul 30, 2019 · 8 comments · May be fixed by #182

Comments

@itsrachelfish
Copy link
Member

itsrachelfish commented Jul 30, 2019

Related to #123

Originally when a shift was edited, all of the slots would be deleted and recreated. This was an issue because slots that users had signed up for would be deleted and users would not be notified that their volunteer shift had been removed.

As of PR #125, slots with users are no longer deleted when a shift is edited. As of PR #155, users are notified when an admin removes them from a slot. These two PRs solve the underlying user experience issue, however, there isn't currently any code to handle removing these old slots after the user is removed.

This means after a shift is edited and a user is removed from the old slot, the old slots are still available for people to sign up for.

After a user is removed from an old slot, the slot itself should be deleted.

@Meleeman01
Copy link
Contributor

so i've replicated this, but maybe it would be a good idea to give the admin the option to remove a slot and not just a user from a slot. so maybe a delete slot button could accomplish this. maybe placing it on the schedule view for convenience for the admin.

@se7enmilgram
Copy link
Contributor

Depending on the browser rendering, these shifts can be hard to remove (you have scale the window to be narrow enough to list the slots instead of placing them on a timeline, otherwise the slots will lay on top of each other, often with an empty shift on top of the occupied shift). I think there are two kinds of edits that require different different outcomes.

If the schedule changes such that the existing slots aren't compatible (slot start / duration changes) then all the slots should just be removed and new ones created, with the users being notified. Could be as easy as adding a delete method that notifies the affected user.

If the changes only add additional slots (either by extending the start time the same number of hours as duration or by increasing the number of volunters / number of times the schedule repeats) we just need to add some checks while creating the slots to see if there's an existing slot that already matches the creation criteria. For example, if we're about to create a slot at 2PM that's three hours long, and there's already a slot for this shift id with that date, start and end, we can skip creating a new one. We'd probably need to grab the existing slot ID and count them up to make sure we have the right number of slots for the number of volunteers. Reducing the number of volunteers may be trickier in that we'd probably need to match the slot display logic (it'd be weird if you went from two volunteers to one and deleted the top slot). I think these changes are within my ability, although I'll need to spend some time with Laravel's ORM to make sure I'm not adding a mess of repetitive database queries that can be avoided.

The only other thing I could think of to improve the situation would be to have a schedule edit confirmation page if there were affected users, a la "This will affect the following users, are you sure?" or modify the schedule preview on the edit page to display the effects, both of which are outside of my wheelhouse (front end development is my idea of hell).

@demogorgonzola
Copy link
Contributor

I say we just fire this off as an hourly, daily, whatever-amount-of-time job that can be done as a quick query for all old slots and then run a mass delete on them.

I feel like I might not be getting the full picture though or is that the scope we're looking for?

@Meleeman01
Copy link
Contributor

Meleeman01 commented Aug 4, 2019

i like the idea of checking for slots that are the same as well @se7enmilgram , i think that would eliminate some of the overlapping shifts that occur when the schedule is edited, and for slots that weren't exactly that time, they would be visible to the admin and they could promptly delete those slots with a delete slot method/button, which would hopefully notify users. i think the reason we kept those users was so the admin could reassign them.

@demogorgonzola there is a method to delete old slots but they all get deleted, all the ones with null user_id's anyway. so are you proposing a scheduled mass delete?

how would your solution work? so like i generate a new schedule with slots, and i have the slots that are still there, because they have user_ids, they are scheduled to be deleted at a certain time, and if scheduled perhaps they could be greyed out or set to a different color to let users/admin know that they will be removed at a certain time. i think this would be a nice functionality, if thats what I'm thinking you're thinking. and i think they should be soft deleted, so they can be retrieved if necessary or if the admin forgets to update the times of users who were assigned to the slots, but maybe you had something different in mind?

perhaps we could soft delete the slots with the associated users, created a list of soft deleted slots with users and remind the admin that "hey these users need to be reassigned" send notifications to affected users maybe explaining they've been removed from a slot, and that they will probably be reassigned and should check the schedule periodically for updates. We could put these on the schedule with the old slots in a different color/opacity and it would only be visible to the Admin, that way, we can keep the schedule clear for the volunteers, and the admin can see the changes that need to be changed. that way we don't need to schedule a time to delete them permanently, but that could be an added feature anyway to delete them permanently, or once users have been reassigned this is going off of @demogorgonzola 's idea.

@demogorgonzola
Copy link
Contributor

After talking a bit with Rachel and getting some clarification and direction, I think I get what's being done now.

So I'm toying with some of the UI right now to how adding a "0" row would affect the js for displaying slots. That way any slots that need to be reassigned, they show up at the top of each schedule in a cool color. If there are none that show up, then it appears as normal. There would also be a warning which says "hey there's some stuff you gotta deal with, start scrolling".

A good way to flag these would totally be soft deletes, it automatically disables every query looking fo them, unless we specify were querying soft-deleted slots themselves. That way regular queries act as normal and we can just alter a few queries for the admin view.

@demogorgonzola
Copy link
Contributor

There's also another idea we could go with to put more on the user.

It's a bit like @se7enmilgram suggested, but instead of just doing a check which automatically transfers the same-time shifts, we create an algo which just reassigns them based off of closeness of their start_time and end_time. If they hit a weighted value beyond what we want (passed a day or something) we can then remvoe them and tell by sending them a removal email. This way we do minimal work I think. It just depends on how much work the algo needs.

@se7enmilgram
Copy link
Contributor

Just spent some time reacquainting myself with the code.

The check I'm proposing would live inside of generate() in the Slot model, within the for loop that iterates over the number of times the schedule repeats. Basically, before we create the new slots, we check to see if there's existing slots, belonging to this schedule, that match the start, end, and duration, and in the correct quantity to support the number of volunteers. This would require a change in how we handle the number of volunteers, which looks like it repeats the entire for loop once for each volunteer requested. If we track slot id's that are created or located within this loop, at the end of it we should have a list of ID's that are all good. Then we can do a simple -> delete on any of this schedule's slots with an ID not in that list. If we add a simple hook to the slot deletion that notifies an assigned user when a slot they've taken is deleted, I think we're good to go.

A nice side benefit of this method is that it would also handle the case where repeat or volunteers is reduced. If we have a schedule with 3 volunteers change to 2, and we have slots with ids 10, 11, 12, we add 10 and 11 to the list of good ids within the for loop, and during clean up we delete any slots whose id isn't in the good list (in this case, 12). This produces reliable behavior in terms of which shifts are deleted in the UI, as long as the UI renders slots with higher id on the second row.

I can't think of a good reason to keep slots around that don't match their schedule. They're essentially artifacts and require manual admin actions to be removed. I don't think we should try to accommodate them on the front end, whether we soft delete them or not.

As for trying to match users to new shifts that they didn't sign up for, I'd be wary of that as an organizer. The schedule change needs to be communicated to the volunteer, and if they can't work the new times they have to come in and drop the shift, etc etc. Better to just say "your shift was changed / deleted, come grab a new shift".

@demogorgonzola
Copy link
Contributor

@se7enmilgram I still think we need an admin interface to allow an event to recover from rescheduling.

Looking over the problem again, I think we should keep in mind we don't have a common company policy to go off since we, meaning we don't have a clear solution. So I propose we make the most general solution we can.

We let all affected by the rescheduling, event admins and volunteers, know that it's happened. For volunteers, we send out a notification in the daily digest, at the very top in muted red or something, that they've been booted and need to go back to reschedule. For admins, we offer a couple of tools. We take a partial view similar to the event grid view and apply it to the view of the old slot. We then rewrite the button functionality to send the user in the old slots to be assigned to the new slot. We kill two birds this way since we reassign to new slots and delete old slots in one click. To display the old slots, we display them above the shifts that they were booted from in the event page so admins have a better idea of where to reassign them. We highlight the shift in the old slot view so they know where the original stood.

This offers us the most flexibility that way - regardless of company policy - they have the option to reschedule the users or let the users reschedule themselves.

As a side note, we would need to replace the "you need to reschedule" with a "you have been rescheduled" notification that is still topped in the users daily digest.

Also on the note of auto-reassigning, this is a gamble because it takes awareness away from the admin from what is happening, which I'm now not a fan of. Although the auto-reassign for shifts with an exact time, date, etc, I could see being useful. It probably goes beyond the scope of this problem currently and would be a cool extra feature to add later.

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 a pull request may close this issue.

4 participants