Add uuid activation code and logic to make it backwards compatible #275
Conversation
Added a new activation_uuid field. Changed default for old activation_code to blank, and default to new code to uuid.uuid4, so new records will have the new code, not old code. This will maintain legacy records and still be valid. I added some methods to get the activation_code, which checks for a legacy code, returns that if present, otherwise returns the new code. I replaced all direct references to activation_code with this new method.
The old email based format should work, but new urls are generated with the activation code as UUID, and no email. There is one particular case where an update could be sent without an activation code or an email, which I’m really not sure how that would have worked? This is on test_web.py test_update_request_activate_form. I’ve kind of disabled this, or changed what it was originally testing.
|
This same test fails for me locally (on python 3.7), and it even fails on 'master' branch. I'm not sure what is going on there, or why it passed before, but it is related to a date field that is created and then later tested. A Y-M-D is setup, and in my case, this FAILS because of a time zone issue. The original date is for "today", but the new date is for "tomorrow", but UTC (or, something like that). I suspect this will pass at some time of the day, and, probably at whatever time zone you are in!! |
|
See, this passed once my timezone was close enough to UTC. |
|
@kutenai This is super interesting! I'm sorry I have been very busy, I haven't had time to reply or look into this before. I will likely be quite busy for some time to come. Perhaps someone else could give this a preliminary review? @claudep @pcraston @dsanders11 |
|
@kutenai This PR looks quite clean to me and also your description in the initial issue explain what you have changed well. But I'm kind of missing the why. What is the driver/motivation behind this change? |
|
Code looks good to me, great work! There's some minor issues though, that I'd love to have fixed before fitting this in. It would be great if we could ship this in a 1.0 release, some time soon! I am really, really sorry I didn't get to review this before. I've been badly burned out in the last year, sadly. But, recuperating. :)
Specifically:
Note the TODO comments on missing coverage. If you don't do it, it's likely nobody else will either!
|
Add the new activation_uuid as a UUIDField but leave the previous code field alone since an installation may have outstanding subscriptions with that old code, and those are not compatible with a UUID.
Make the old field allow blanks, and default to blank, but the new field defaults to a valid uuid.uuid4(). Now all new records will get a new uuid4() value, but not an old value.
Modify logic to check the UUID using the get_activation_code() method, and the form validates with a new 'valid_activation(data)' method that checks against the old code and the new code, converting the data to a UUID if needed.
Tests pass, at least under Python 3.7