This PR moves Django models and related code from emails to privaterelay.
It does not update the database. The moved models have the Meta.db_table property set to ensure the table names do not change (still emails_profile and emails_registeredsubdomain. The migrations use the SeparateDatabaseAndState operation to avoid SQL. Deployment will work, and that the terraform SQL will continue to work. Thanks to https://davit.hashnode.dev/django-move-model for explaining this technique.
It may be possible to smoothly rename the tables as well. PostgreSQL views are updatable, so we could create a emails_profile view that allows existing code to continue to query, insert, and update to emails_profile while the table is named privaterelay_profile. There are two issues. One is the SQLite does not have updatable views, so I'd need to craft triggers to make tests pass. The second is that if a test needs to run in a transaction, TRUNCATE is used and further complicates things. Both could be handled. SQLite triggers are possible to write, just hard. I eliminated all the transaction tests, and an upstream contribution to pytest_django is possible. We could also just say "dev and migration tests are broken for a bit". In any case, I'd like to avoid any table renames in this PR.
Models:
Moved Profile, since it contains a mix of email, phone, and account information for a Relay user
Moved RegisteredSubdomain, since it is concerned with Profile.subdomain, with a validator that uses it. I could also argue that this should remain in emails, since subdomains are an email feature.
Did not move AbuseMetrics, since it only has email usage and doesn't have a relationship to the Profile. I could also argue that this should move to privaterelay, since it could gain phone abuse metrics in the future.
Validators:
Moved valid_available_subdomain to privaterelay, to follow RegisteredSubdomain
Signals:
Moved all signals to privaterelay, to follow Profile
Exceptions:
Moved CannotMakeSubdomainException to privaterelay, to follow RegisteredSubdomain
Tests, Admin - also moved to stay with related code
How to test
Get a hot beverage.
Review the code.
Think about these questions:
Should we instead break up Profile into RelayProfile, EmailsProfile, and PhoneProfile? If so, are those separate tables or combined using abstract base classes?
Which app should have RegisteredSubdomain?
Which app should have AbuseMetrics?
CircleCI does all of the validation that this works. The migrations tests are especially important. If you want to verify they run no SQL:
This PR moves Django models and related code from
emailstoprivaterelay.It does not update the database. The moved models have the
Meta.db_tableproperty set to ensure the table names do not change (stillemails_profileandemails_registeredsubdomain. The migrations use the SeparateDatabaseAndState operation to avoid SQL. Deployment will work, and that the terraform SQL will continue to work. Thanks to https://davit.hashnode.dev/django-move-model for explaining this technique.It may be possible to smoothly rename the tables as well. PostgreSQL views are updatable, so we could create a
emails_profileview that allows existing code to continue to query, insert, and update toemails_profilewhile the table is namedprivaterelay_profile. There are two issues. One is the SQLite does not have updatable views, so I'd need to craft triggers to make tests pass. The second is that if a test needs to run in a transaction,TRUNCATEis used and further complicates things. Both could be handled. SQLite triggers are possible to write, just hard. I eliminated all the transaction tests, and an upstream contribution topytest_djangois possible. We could also just say "dev and migration tests are broken for a bit". In any case, I'd like to avoid any table renames in this PR.Profile, since it contains a mix of email, phone, and account information for a Relay userRegisteredSubdomain, since it is concerned withProfile.subdomain, with a validator that uses it. I could also argue that this should remain inemails, since subdomains are an email feature.AbuseMetrics, since it only has email usage and doesn't have a relationship to theProfile. I could also argue that this should move toprivaterelay, since it could gain phone abuse metrics in the future.valid_available_subdomaintoprivaterelay, to followRegisteredSubdomainprivaterelay, to followProfileCannotMakeSubdomainExceptiontoprivaterelay, to followRegisteredSubdomainHow to test
ProfileintoRelayProfile,EmailsProfile, andPhoneProfile? If so, are those separate tables or combined using abstract base classes?RegisteredSubdomain?AbuseMetrics?CircleCI does all of the validation that this works. The migrations tests are especially important. If you want to verify they run no SQL:
both return:
Previous PRs
The initial spike was broken up into smaller, focused PRs. Thanks to @groovecoder and @say-yawn for the reviews and making this a better change.
user.profileto other methods of getting the profile