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

enable_hashid_object=False breaks Django admin? #69

Closed
ckcollab opened this issue Feb 11, 2022 · 15 comments
Closed

enable_hashid_object=False breaks Django admin? #69

ckcollab opened this issue Feb 11, 2022 · 15 comments

Comments

@ckcollab
Copy link

order_id = HashidField(enable_hashid_object=False, null=True, blank=True)

Unless I set enable_hashid_object = True then I get the following error in Django admin attempting to save a model:

TypeError
'<' not supported between instances of 'str' and 'int'

image

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

It looks like you have a MinValueValidator on that field, which doesn't work with a string, so either enable the hashid_object or remove the validator.

@nshafer nshafer closed this as completed Feb 11, 2022
@ckcollab
Copy link
Author

ckcollab commented Feb 11, 2022

That's the confusing thing and why I shared the field definition: I didn't set this validator or anything?! Changing enable_hashid_object removes the error

Maybe something special django admin does?

@nshafer nshafer reopened this Feb 11, 2022
@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

What version of Django? I just tested with 3.2 and had no problems

@ckcollab
Copy link
Author

ckcollab commented Feb 11, 2022

Django==3.2.10 and I just tried on the latest Django==3.2.12 as well, same problemo. Using django-hashid-field==3.3.3

I went to a completely different model, added this, and tried to save it in Django admin:

special_id = HashidField(enable_hashid_object=False, null=True, blank=True, default=1)

..same error! It could definitely be something silly i am doing..

I'm down to hop in a quick Google Meet or something, I can share my environment and maybe be more helpful?

EDIT: To remove Django Admin from the equation, here's a quick repro:

>>> u = User.objects.first()
>>> u.full_clean()
Traceback (most recent call last):
  File "<input>", line 1, in <module>
    u.full_clean()
  File "/usr/local/lib/python3.10/site-packages/django/db/models/base.py", line 1229, in full_clean
    self.clean_fields(exclude=exclude)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/base.py", line 1271, in clean_fields
    setattr(self, f.attname, f.clean(raw_value, self))
  File "/usr/local/lib/python3.10/site-packages/django/db/models/fields/__init__.py", line 671, in clean
    self.run_validators(value)
  File "/usr/local/lib/python3.10/site-packages/django/db/models/fields/__init__.py", line 623, in run_validators
    v(value)
  File "/usr/local/lib/python3.10/site-packages/django/core/validators.py", line 358, in __call__
    if self.compare(cleaned, limit_value):
  File "/usr/local/lib/python3.10/site-packages/django/core/validators.py", line 392, in compare
    return a < b
TypeError: '<' not supported between instances of 'str' and 'int'

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

My guess is you have somewhere in your code-base, or some library that is helpfully adding it. If you do something like this, what do you get?

In [1]: key = Book._meta.get_field("key")

In [2]: key
Out[2]: <hashid_field.field.HashidField: key>

In [3]: key.enable_hashid_object
Out[3]: False

In [4]: key.validators
Out[4]: []

@ckcollab
Copy link
Author

ckcollab commented Feb 11, 2022

>>> key = User._meta.get_field("special_id")
>>> key
<hashid_field.field.HashidField: special_id>
>>> key.enable_hashid_object
False
>>> key.validators
[<django.core.validators.MinValueValidator object at 0x7f7e2b8fa050>, <django.core.validators.MaxValueValidator object at 0x7f7e2b8fa0b0>]

you're right...

Something must be setting "default validators" somehow... ok..! probably not a problem with your project.

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

What does your model inherit from? models.Model that's imported from django.db import models?

@ckcollab
Copy link
Author

ckcollab commented Feb 11, 2022

In one case:

class User(AbstractBaseUser, PermissionsMixin):
    ...
    special_id = HashidField(enable_hashid_object=False, null=True, blank=True, default=1)

In another case:

class Booking(SoftDeletableModel):
    ...
    order_id = HashidField(enable_hashid_object=False, null=True, blank=True)

# And here's soft deletable model:
class SoftDeletableModel(models.Model):
    deleted = models.BooleanField(default=False)

    objects = SoftDeleteModelManager()
    all_objects = models.Manager()

    class Meta:
        abstract = True

EDIT:

Looking at HashidField itself, the inherited IntegerField may be producing these validators for me!? not sure why it wouldn't for you..

class IntegerField(Field):
    
    ...

    @cached_property
    def validators(self):
        # These validators can't be added at field initialization time since
        # they're based on values retrieved from `connection`.
        validators_ = super().validators
        internal_type = self.get_internal_type()
        min_value, max_value = connection.ops.integer_field_range(internal_type)
        if min_value is not None and not any(
            (
                isinstance(validator, validators.MinValueValidator) and (
                    validator.limit_value()
                    if callable(validator.limit_value)
                    else validator.limit_value
                ) >= min_value
            ) for validator in validators_
        ):
            validators_.append(validators.MinValueValidator(min_value))
        if max_value is not None and not any(
            (
                isinstance(validator, validators.MaxValueValidator) and (
                    validator.limit_value()
                    if callable(validator.limit_value)
                    else validator.limit_value
                ) <= max_value
            ) for validator in validators_
        ):
            validators_.append(validators.MaxValueValidator(max_value))
        return validators_

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

Yeah none of that is a clue of where it's coming from. I'm going to hazard a guess that something is doing a isinstance(field, models.IntegerField) check, which is true, but obviously doesn't work. A case where duck-typing is better in python/django. Why do you want to disable the Hashid object? Is leaving it on an option?

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

Oh... what database?

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

min_value, max_value = connection.ops.integer_field_range(internal_type) is getting something from your adapter that I've never seen, but I'm 100% postgres in my work.

@ckcollab
Copy link
Author

Well this is a fun one! Yeah, Postgres here, too..

'ENGINE': 'django.db.backends.postgresql_psycopg2',
psycopg2-binary==2.9.3

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

Ha! I lied! Both my tests and sandbox are using sqlite. Fix incoming.

@nshafer
Copy link
Owner

nshafer commented Feb 11, 2022

Version 3.3.4 is on pypi with a fix for this. Sorry about that! But thank you for working with me to a solution.

Nate

nshafer added a commit that referenced this issue Feb 11, 2022
@ckcollab
Copy link
Author

No problemo, was fun! Confirmed fixed.

Have a good one!

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

2 participants