Reservation Assigned to "Deleted User"

[orenyk]

From Art (see screenshot below):

Yes, that camera was checked out to Michelle Mboya; I asked an MT to switch her reservation to my account and somehow this resulted. I think we'll need Casey's help to remedy the situation; I've copied him here.

Not sure what's going on, will try to check it out later.

[orenyk]

I visited the reservation page, which is returning a 404 error. I tried to find the deleted user by going to the Users page and hitting "Show Deactivated Users in List" but that's returning a 500 error. Unfortunately, the dropdown on the reservations list page (shown above) doesn't appear when you click on "Deleted User", so I can't tell what ID that user has.

I have also been unable to reproduce this issue. I created a reservation for a user, checked it out, and then edited the reservation to make me the reserver. Upon returning to the list of reservations everything looked fine, I can visit the reservation page and it has been transferred to me. I'll keep digging and see where this came from.

[orenyk]

Ok, I have some more information. In the Reservation model we define the method reserver to either return the user whose ID is stored in the reserver_id attribute, and we rescue any exceptions (e.g. no user found) by returning a new, unsaved User whose name is "Deleted User" and has a bunch of other dummy information.

Now, I've checked, and the original reserver's account still exists. as does Johannes' (to whom the reservation was supposedly transferred). It could be that somehow when transferring the reservation the reserver_id was somehow corrupted, resulting in "Deleted User" owning the reservation, but that doesn't explain the 404 error. I'm going to keep playing with my local instance and see what I can find.

[orenyk]

Ah ha! I went into the console and manually changed the reserver_id to some nonsensical number and that reproduced the "Deleted User" appearing above. When I try to click on the reservation link I get the same 404 error and it gives me some more info, namely, that we're dealing with a routing error:

Still digging... I'm pretty sure this has to do with one of the links in the view, I'll update after I figure it out.

[orenyk]

Ok, issue resolved. On the bottom of the reservation page, we have the button / link to the manage reservation page (via the manage_reservation_btn method in reservations_helper.rb, see below for screenshot). This links to (for a checked out reservation):

link_to 'Check-In', manage_reservations_for_user_path(@reservation.reserver.id,
          anchor: 'check_in_row')

manage_reservations_for_user_path routes to {root}/reservations/manage/{user_id}#check_in_now but since we're pulling the user_id from @reservation.reserver.id and the reserver is just dummy content from our reservations controller, we're passing a user_id of nil to the routing method and it raises the error.

My guess would be that the easiest way to resolve this issue is to simply go into the console and delete the reservation. If I'm not mistaken (verified on my local instance), when we run the following in the console:

Reservation.find(1913).delete

it will delete the problematic reservation, and this will automatically set the relevant equipment item as available since it's no longer tied to a reservation. Hope that helps!

[orenyk]

All that said, this is an edge case we should be able to handle, so we need to figure out how to deal with reservations where the user account no longer exists / reserver raises an exception. Maybe we update the manage_reservation_btn method to return a link that deletes the reservation to resolve similar issues in the future, should they arise. Will discuss with @caseywatts and make the necessary changes, if desired.

[orenyk]

CHECK TO SEE IF SOFT DELETE CAUSED THIS

Do we want soft deleting of users?

[orenyk]

I'll get in touch with ITS to run the following commands in the console:

Reservation.find(1913)
Reservation.find(1913).delete

Hopefully this will resolve their issues and give us more information about what caused this (specifically, what is in Reservation.find(1913).reserver_id).

Make sure that the update / create reservation methods check for bad input (e.g. if you change the reserver, can you send an invalid user ID) --> this might not have been caused by soft delete, but by a bad request.

Also get the original reserver's user information (and the current reserver if there is an ID).

User.find_by_last_name("Mboya")

Also get all deactivated users (if there are any):

User.all.keep_if { |user| user.deleted_at }

Also get a list of all user IDs in the database (use a collect):

User.all.collect(&:login)

[orenyk]

Just had a Google Hangout with Michael Dunlap at ITS and we ran the commands above. Apparently during the switch over to Reservations some time between my work above and today the reservation was deleted from the database; Michael sent me the MySQL dump and it wasn't there. There were two deactivated users in the database; Andrew Clifford (it looks like he created an account with the wrong NetID, but it has a higher user ID than the correct account so I'm not sure what the deal is) and Zak Arctander (again, this is an account with an incorrect NetID, the corrected account is active but has a lower user ID). It doesn't seem like they're using soft deletion for users for any particular reason so we can probably phase it out. I'll discuss w/ @caseywatts and we'll close this out.

[shippy]

@orenyk Does this require further action? (I have to say that the custom-rescue method of "Deleted User" looks like a bad smell to me, although I'm not sure what the systemic solution would be -- disabling users instead of deleting them?)

[orenyk]

I think we can look into a few things before closing this out:

• Refactoring the view reservation page (specifically the manage_reservation_btn function) to rescue routing issue where the user no longer exists so that access to the view reservation page is not predicated on the existence of the reserver's user account and we don't encounter this again.
• Alternatively, add a delete reservation link to the reservation list page (it would go in the table).
• Figure out if clients are using soft-deletion of users / if this functionality is necessary. This probably should then become its own issue, but it came up while we were debugging this so we should put that discussion to rest.
• Think about alternative ways to rescue the reserver method.

[squidgetx]

Users should never be deleted (#529); the only time something like this would come up now is if the reserver id was somehow set to a non-existent ID. In that (very rare, should-never-happen-unless-hardware-fails-or-data-is-corrupted scenario) I think it's fine to return the dummy user, and we can change the helper to not-render in the case that the reserver doesn't exist (ActiveAdmin or cancel reservation button can be used to delete the reservation)

[orenyk]

Agreed, I'll review #714 and close once it's merged.