Skip to content

Bogus type checking condition in LocalizedValueDescriptor.__get__, LocalizedValue._interpret_value may need recosideration #75

@atleta

Description

@atleta

The type check on line 56 in descriptor.py is bogus and it causes a few related issues. The main one being is that you can't set a value for a language not specified in settings.LANGUAGES, because it will be overwritten on the next attribute access. So e.g. if you have a model like this:

class MyModel(models.Model):
    content = LocalizedTextField()

and you don't have e.g. the 'nl' (Dutch) language enabled, then doing this:

instance = MyModel()
instance.content.set('nl', 'Yolo')

won't set (won't retain) the setting for nl. The reason is the following line in descriptor.py:

        if isinstance(value, dict):

Because LocalizedValue inherits from dict, this will keep reinitializing the field on every attribute access:

            attr = self.field.attr_class(value)
            instance.__dict__[self.field.name] = attr

This is problematic because it can e.g. break data migrations if you change the language settings (this is how I found out about this). Also, it's an unnecessary copy operation with an impact on performance.

the solution is to change the line with the condition to:

if not isinstance(value, LocalizedValue) and isinstance(value, dict)

(most of the time it will be a LocalizedValue, as on the first access we change it to be that.)

On a side not, LocalizedValue._interpret_value may not be doing the right thing either (that's the other side of the problem): changing the available languages in the settings is basically equivalent to changing the db schema. (Again, it could break migrations too.)

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions