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.

Sign up for GitHub

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

Fix #5 Prevent MigrationAutoDetector removes the user unique email constraint #6

Merged
merged 1 commit into from
Nov 20, 2023
Merged

Fix #5 Prevent MigrationAutoDetector removes the user unique email constraint #6

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/unique_user_email/apps.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,4 +23,4 @@ def ready(self):
]
User._meta.constraints = User.Meta.constraints
# ... as long as original_attrs is not updated.
# User._meta.original_attrs["constraints"] = User.Meta.constraints
User._meta.original_attrs["constraints"] = User.Meta.constraints
Copy link
Owner

@carltongibson carltongibson Nov 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that this will cause the auto-detector to create an auth migration when unique_user_email is installed but the migration is not run. ( maybe it won't... — I put this together interactively, so maybe the end result isn't the same as the half-way point…)

The comment you removed here was part of the longer comment beginning on ln12.

I think we should have tests for both that case and the issue here (which is that an auth migration is created after the unique_user_email migration is run).

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah ha! It' looks like it doesn't 🕺

% ./manage.py showmigrations
admin
 [ ] 0001_initial
 [ ] 0002_logentry_remove_auto_add
 [ ] 0003_logentry_add_action_flag_choices
auth
 [ ] 0001_initial
 [ ] 0002_alter_permission_name_max_length
 [ ] 0003_alter_user_email_max_length
 [ ] 0004_alter_user_username_opts
 [ ] 0005_alter_user_last_login_null
 [ ] 0006_require_contenttypes_0002
 [ ] 0007_alter_validators_add_error_messages
 [ ] 0008_alter_user_username_max_length
 [ ] 0009_alter_user_last_name_max_length
 [ ] 0010_alter_group_name_max_length
 [ ] 0011_update_proxy_permissions
 [ ] 0012_alter_user_first_name_max_length
contenttypes
 [ ] 0001_initial
 [ ] 0002_remove_content_type_name
sessions
 [ ] 0001_initial
unique_user_email
 [ ] 0001_initial
% ./manage.py makemigrations auth
No changes detected in app 'auth'

OK, let me have a play over the weekend, and I'll get this out.

Thanks @trottomv 🎁

Copy link
Contributor Author

@trottomv trottomv Nov 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for completenes:

(.venv) $ python manage.py showmigrations
admin
 [X] 0001_initial
 [X] 0002_logentry_remove_auto_add
 [X] 0003_logentry_add_action_flag_choices
auth
 [X] 0001_initial
 [X] 0002_alter_permission_name_max_length
 [X] 0003_alter_user_email_max_length
 [X] 0004_alter_user_username_opts
 [X] 0005_alter_user_last_login_null
 [X] 0006_require_contenttypes_0002
 [X] 0007_alter_validators_add_error_messages
 [X] 0008_alter_user_username_max_length
 [X] 0009_alter_user_last_name_max_length
 [X] 0010_alter_group_name_max_length
 [X] 0011_update_proxy_permissions
 [X] 0012_alter_user_first_name_max_length
contenttypes
 [X] 0001_initial
 [X] 0002_remove_content_type_name
sessions
 [X] 0001_initial
unique_user_email
 [ ] 0001_initial
(.venv) $ python manage.py makemigrations
No changes detected

8 changes: 8 additions & 0 deletions tests/tests.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from django.test import TestCase
from unique_user_email.backend import EmailBackend
from unique_user_email.forms import AuthenticationForm
from django.db.models.constraints import UniqueConstraint


class UniqueEmailTestCase(TestCase):
Expand Down Expand Up @@ -55,6 +56,13 @@ def test_model_save_disallows_duplicate_emails(self):
):
user2.save()

def test_user_constraints(self):
self.assertIsInstance(User._meta.constraints[0], UniqueConstraint)
self.assertEqual("unique_user_email", User._meta.constraints[0].name)
self.assertEqual(
"unique_user_email",
User._meta.original_attrs.get("constraints")[0].name
)

class EmailBackendTests(TestCase):
def test_none_for_username_logins(self):
Expand Down