Skip to content

Bugfix change CustomUser's manager to enable migration making #451

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

Merged
dybi merged 1 commit into from
Jul 23, 2019
Merged

Bugfix change CustomUser's manager to enable migration making #451

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
18 changes: 18 additions & 0 deletions users/migrations/0006_auto_20190722_1131.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
# Generated by Django 2.2.3 on 2019-07-22 11:31

from django.db import migrations


class Migration(migrations.Migration):

dependencies = [
('users', '0005_auto_20190703_1737'),
]

operations = [
migrations.AlterModelManagers(
name='customuser',
managers=[
Copy link
Collaborator Author

@maciejSamerdak maciejSamerdak Jul 22, 2019

Choose a reason for hiding this comment

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

I guess it's okay, since the manager is now created dynamically?

Copy link
Contributor

@rwrzesien rwrzesien Jul 22, 2019

Choose a reason for hiding this comment

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

Looks like thanks to this CustomUser managers will no longer be tracked in migrations, so it should be ok.

],
),
]
2 changes: 0 additions & 2 deletions users/models.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,6 @@ def get_with_prefetched_reports(self, reports: QuerySet) -> QuerySet:


class CustomUserManager(BaseUserManager):
use_in_migrations = True
Copy link
Collaborator Author

@maciejSamerdak maciejSamerdak Jul 22, 2019

Choose a reason for hiding this comment

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

The usage of this property is obscure. According to Django documentation it has to do with serialisation. @Szymiks reported seeing this on forums regarding REST framework. This line was created by @MartynaAnnaGottschling back in October 2018. full-check did not raise any errors after it got removed.

Is it therefore safe to assume that this is a leftover from the REST implementation and can be safely deleted?

Copy link
Contributor

@rwrzesien rwrzesien Jul 22, 2019

Choose a reason for hiding this comment

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

Not related with REST exactly, but with possibility to use custom manager methods in migrations. As we don't use them, lets remove it.


def _create_user(
self, email: str, password: str, is_staff: bool, is_superuser: bool, user_type: str
) -> "CustomUser":
Expand Down