justinmayer / django-autoslug

AutoSlugField for Django. Supports (but not does not require) unidecode/pytils for transliteration. Old issue tracker is at Bitbucket: https://bitbucket.org/neithere/django-autoslug/issues
https://readthedocs.org/projects/django-autoslug/
GNU Lesser General Public License v3.0
295 stars 105 forks source link

allow_unicode should not be popped off the kwargs #64

Open CaptainCuddleCube opened 4 years ago

CaptainCuddleCube commented 4 years ago

Currently, the django-autoslug sets self.allow_unicode to the kwargs value and it pops allow_unicode out the kwargs.

https://github.com/justinmayer/django-autoslug/blob/3a87391ae7a5141d06396b93ee5b4030d9790a14/autoslug/fields.py#L206

Since SlugField has allow_unicode=False in its init kwargs, and then sets self.allow_unicode=allow_unicode, you get a situation where self.allow_unicode is always false.

The solution is to just remove the code where self.allow_unicode is set and the kwargs are popped off.

devo-wm commented 3 years ago

Currently, the django-autoslug sets self.allow_unicode to the kwargs value and it pops allow_unicode out the kwargs.

https://github.com/justinmayer/django-autoslug/blob/3a87391ae7a5141d06396b93ee5b4030d9790a14/autoslug/fields.py#L206

Since SlugField has allow_unicode=False in its init kwargs, and then sets self.allow_unicode=allow_unicode, you get a situation where self.allow_unicode is always false.

The solution is to just remove the code where self.allow_unicode is set and the kwargs are popped off.

Tried this locally and I actually ran into another error:

Traceback (most recent call last):
  File "~/Projects/project/./appname/manage.py", line 15, in <module>
    execute_from_command_line(sys.argv)
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 401, in execute_from_command_line
    utility.execute()
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/core/management/__init__.py", line 377, in execute
    django.setup()
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/__init__.py", line 24, in setup
    apps.populate(settings.INSTALLED_APPS)
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/apps/registry.py", line 114, in populate
    app_config.import_models()
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/apps/config.py", line 211, in import_models
    self.models_module = import_module(models_module_name)
  File "/usr/local/Cellar/python@3.9/3.9.1/Frameworks/Python.framework/Versions/3.9/lib/python3.9/importlib/__init__.py", line 127, in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
  File "<frozen importlib._bootstrap>", line 1030, in _gcd_import
  File "<frozen importlib._bootstrap>", line 1007, in _find_and_load
  File "<frozen importlib._bootstrap>", line 986, in _find_and_load_unlocked
  File "<frozen importlib._bootstrap>", line 680, in _load_unlocked
  File "<frozen importlib._bootstrap_external>", line 790, in exec_module
  File "<frozen importlib._bootstrap>", line 228, in _call_with_frames_removed
  File "~/Projects/project/appname/inventory/models/__init__.py", line 44, in <module>
    from .path import Path
  File "~/Projects/project/appname/inventory/models/path.py", line 41, in <module>
    from .step import Step
  File "~/Projects/project/appname/inventory/models/step.py", line 61, in <module>
    class Step(ModelMeta, LocalizedJSONMixin, SiteSoftDeleteModelBase, TimestampModelBase):
  File "~/Projects/project/appname/inventory/models/step.py", line 71, in Step
    slug = AutoSlugField(populate_from='name', unique=True, always_update=True, allow_unicode=True)
  File "~/Projects/project/venv/lib/python3.9/site-packages/autoslug/fields.py", line 214, in __init__
    super(SlugField, self).__init__(*args, **kwargs)
  File "~/Projects/project/venv/lib/python3.9/site-packages/django/db/models/fields/__init__.py", line 986, in __init__
    super().__init__(*args, **kwargs)
TypeError: __init__() got an unexpected keyword argument 'allow_unicode'
jared-hardy commented 3 years ago

I would also argue for changing the default to allow_unicode=True in autoslug (despite any Django slug defaults past 1.9, conflicting or otherwise), as disallowing unicode has almost no practical benefit here, and creates needless problems with localization.