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

notify_delegate_of_wca_id_claim assumes that delegate_to_handle_wca_id_claim exists #943

Open
jfly opened this issue Oct 15, 2016 · 1 comment
Labels
META: good first issue Small/easy change which is a good introduction to working in the WCA repo TYPE: bug Bug reported by a stakeholder

Comments

@jfly
Copy link
Contributor

jfly commented Oct 15, 2016

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).

@jfly jfly added TYPE: bug Bug reported by a stakeholder META: good first issue Small/easy change which is a good introduction to working in the WCA repo labels Oct 15, 2016
@larspetrus
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
META: good first issue Small/easy change which is a good introduction to working in the WCA repo TYPE: bug Bug reported by a stakeholder
Projects
Status: No status
Development

No branches or pull requests

2 participants