search

thewca / worldcubeassociation.org

All of the code that runs on worldcubeassociation.org
https://www.worldcubeassociation.org/
GNU General Public License v3.0
320 stars 175 forks source link

notify_delegate_of_wca_id_claim assumes that delegate_to_handle_wca_id_claim exists #943

Open jfly opened 7 years ago

jfly commented 7 years ago

This assumption was true in a world where we synchronously sent emails, but now that we do them in a background job, there's a window of time where the delegate could approve a WCA ID request (thereby clearing delegate_to_handle_wca_id_claim) before we actually run the job (this has happened to us a few times because our background jobs have stuff running, and then when we get them running again, we find that some delegates processed the WCA ID claims in the interim.

I think the only thing we can do here is to have notify_delegate_of_wca_id_claim bail out if delegate_to_handle_wca_id_claim is not defined. This should be an easy change.

We should definitely test this by hand, as I have a vague suspicion that Rails might do something weird when you decide not to send an email in a ApplicationMailer method (see this code).

larspetrus commented 7 years ago

The "computer science" solution would be waiting to clear the delegate_to_handle_wca_id_claim until both the email has been sent and the delegate has approved it.

But, yeah, there is no need to tell the delegate about this claim if he has already approved it.