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

Warning running makemigrations after updating to Django 3.1 and django-currentuser 0.5.1 #43

Open
fgs-dbudwin opened this issue Aug 11, 2020 · 13 comments

Comments

@fgs-dbudwin
Copy link
Contributor

Environment:

  • Windows 10 (WSL 2 Debian)
  • Python 3.8
  • pipenv
  • Django 3.1
  • django-currentuser 0.5.1

Problem:

When I run python manage.py makemigrations I get the following warning. This warning is new after updating to the latest version of Django and django-currentuser. If I revert Django to 3.0.X the warning will go away.

/home/admin/.local/share/virtualenvs/Project-HFzz1tdt/lib/python3.8/site-packages/django_currentuser/db/models/fields.py:57: UserWarning: You passed an argument to CurrentUserField that will be ignored. Avoid args and following kwargs: default, null, to.
  warnings.warn(self.warning)

I have one reference to CurrentUserField in my project:

from django.db import models
from django_currentuser.db.models import CurrentUserField


class UserTrackingModel(models.Model):
    created_by = CurrentUserField()

    class Meta:
        abstract = True

I use UserTrackingModel as a base class for some of my models, like so:

from camp.db_models import UserTrackingModel

class Foo(TimeStampedModel, UserTrackingModel):
    name = models.CharField(default="", max_length=200, unique=True)
    thing = models.ForeignKey(Thing, on_delete=models.PROTECT, null=False)

Desired Outcome

I would like to be able to generate migrations without getting a warning that could potentially turn into a bigger problem down the road.

@belugame
Copy link
Collaborator

Thank you for the detailed bug report. I assume Django 3.1 passes a new built-in kwarg which CurrentUser should expect but does not. Though we will need to look into the docs/source for that since we don't use 3.1 yet.

@N1K1TAS95
Copy link

I have the same warning (I guess):

/usr/local/lib/python3.9/site-packages/django_currentuser/db/models/fields.py:57: UserWarning: You passed an argument to CurrentUserField that will be ignored. Avoid args and following kwargs: default, null, to.
warnings.warn(self.warning)

I'm using this setup:

  • Ubuntu 18.04.5 LTS
  • Python 3.9.1
  • Django 3.1.3
  • django-current-user 0.5.2

@avinashjoshi
Copy link
Contributor

Same issue here! It seems like this issues is specific to when using a custom user model. Did some digging into this by adding print statements here and found that the to parameter passed in kwargs different from that in self.defaults:

kwargs: {..., "to": "accounts.user"}
self.defaults: {..., "to": "accounts.User"}

Updating AUTH_USER_MODEL in settings.py to the following fixed the issue (note the lowercase u in user):

AUTH_USER_MODEL = "accounts.user"

Now the bigger question is which of the following is correct:

  • AUTH_USER_MODEL = "accounts.User" (this is what is recommended in django docs)
    OR
  • AUTH_USER_MODEL = "accounts.user"

Any thoughts on what is the right approach to fix this issue?

@N1K1TAS95
Copy link

Same issue here! It seems like this issues is specific to when using a custom user model. Did some digging into this by adding print statements here and found that the to parameter passed in kwargs different from that in self.defaults:

kwargs: {..., "to": "accounts.user"}
self.defaults: {..., "to": "accounts.User"}

Updating AUTH_USER_MODEL in settings.py to the following fixed the issue (note the lowercase u in user):

AUTH_USER_MODEL = "accounts.user"

Now the bigger question is which of the following is correct:

  • AUTH_USER_MODEL = "accounts.User" (this is what is recommended in django docs)
    OR
  • AUTH_USER_MODEL = "accounts.user"

Any thoughts on what is the right approach to fix this issue?

I cannot say anything about this solution because I'm using a custom model for user
AUTH_USER_MODEL = "core.User"

@fgs-dbudwin
Copy link
Contributor Author

@avinashjoshi and @N1K1TAS95 are you using version 0.5.2 of this library? I submitted a fix and had it merged, I believe this issue should be closed unless it still appears to happen on the latest version.

See: https://github.com/PaesslerAG/django-currentuser/releases/tag/0.5.2

@N1K1TAS95
Copy link

@avinashjoshi and @N1K1TAS95 are you using version 0.5.2 of this library? I submitted a fix and had it merged, I believe this issue should be closed unless it still appears to happen on the latest version.

See: https://github.com/PaesslerAG/django-currentuser/releases/tag/0.5.2

I tried to reinstall the package (last version 0.5.2), but I still have the same warning.

Maybe it's my fault?

Currently I'm using the CurrentUserFiled as this:

user = CurrentUserField(
        verbose_name='Creato da',
        on_delete=models.PROTECT
)

Maybe I'm missing some args or kwargs?

@cccaballero
Copy link

I can confirm that this is happening using 0.5.2 with Django 3.1.5. I am also using django-allauth (I'm not sure if it has something to do)

@EthanZeigler
Copy link

@fgs-dbudwin any updates on this?

@aidanlister
Copy link

Changing AUTH_USER_MODEL = "accounts.User" to lowercase fixed my warnings too.

@EthanZeigler
Copy link

Changing AUTH_USER_MODEL = "accounts.User" to lowercase fixed my warnings too.

I mean, it's a solution, but I don't like this given that django recommends the accounts.User pattern.

@clovisp
Copy link

clovisp commented Sep 13, 2021

Same problem here, any idea when it could be solved?

@jongracecox
Copy link
Contributor

jongracecox commented Jan 10, 2022

I am also hitting this issue, but I am not using any custom user model. I'm using django==3.1.5 and django-currentuser==0.5.3. I do have a custom authentication middleware, but I don't think that would factor in.

@jongracecox
Copy link
Contributor

As a test I updated the _warn_for_shadowing_args() function in django_currentuser.db.models.fields.py to include extra information in the warning message:

    def _warn_for_shadowing_args(self, *args, **kwargs):
        if args:
            warnings.warn(self.warning)
        else:
            for key in set(kwargs).intersection(set(self.defaults.keys())):
                if not kwargs[key] == self.defaults[key]:
                    warnings.warn(
                        self.warning + f" Unexpected arg was: {key}={repr(kwargs[key])}."
                                       f" Default is: {repr(self.defaults[key])}"
                    )
                    break

This results in:

Operations to perform:
  Apply all migrations: admin, auth, client_dis, contenttypes, drs, portal_home, sessions
.../venv/lib/python3.9/site-packages/django_currentuser/db/models/fields.py:65: UserWarning: You passed an argument to CurrentUserField that will be ignored. Avoid args and following kwargs: default, null, to. Unexpected arg was: to='auth.user'. Default is: 'auth.User'

or

.../venv/lib/python3.9/site-packages/django_currentuser/db/models/fields.py:65: UserWarning: You passed an argument to CurrentUserField that will be ignored. Avoid args and following kwargs: default, null, to. Unexpected arg was: null=False. Default is: True

I wonder whether it would be nice to include this as an update so it's more obvious which unexpected args were passed.

Running a Django shell I can see that my user model is set to auth.User, and updating the settings to use auth.user does make the warning go away, but this doesn't seem like a good fix.

Here's an option for handling the simple difference in case, in the CurrentUserField class in django_currentuser.db.models.fields.py:

    def __init__(self, *args, **kwargs):
        self.on_update = kwargs.pop("on_update", False)

        # If `to` is present in kwargs, and the same when ignoring case then
        # update `to` to use the defaults.
        # Fix for https://github.com/PaesslerAG/django-currentuser/issues/43
        if "to" in kwargs and kwargs["to"].lower() == self.defaults['to'].lower():
            kwargs["to"] = self.defaults['to']

        self._warn_for_shadowing_args(*args, **kwargs)
        ...

It does feel a little hacky, but it should be safe, and prevent the warning for users that are using the default user model.

jongracecox added a commit to jongracecox/django-currentuser that referenced this issue Jan 11, 2022
Fix issue where a warning is raised when running makemigrations
when user model is passed with different case to the default.
This commonly happens when the default is `auth.User` and
`auth.user` is passed in the `to` field.

The fix is to detect the case difference, and update the incoming
args to match the default.

Relates to zsoldosp#43.
jongracecox added a commit to jongracecox/django-currentuser that referenced this issue Jan 11, 2022
Fix issue where a warning is raised when running makemigrations
when user model is passed with different case to the default.
This commonly happens when the default is `auth.User` and
`auth.user` is passed in the `to` field.

The fix is to detect the case difference, and update the incoming
args to match the default.

Relates to zsoldosp#43.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

9 participants