search

coderholic / django-cities

Countries and cities of the world for Django projects
MIT License
920 stars 374 forks source link

Bugfix: SlugModel.save(force_insert=True) tries to double-insert, and crashes #178

Closed PiDelport closed 6 years ago

PiDelport commented 6 years ago

Currently, saving any SlugModel subclass with save(force_insert=True) will crash with a database-level IntegrityError.

This happens when using many Django APIs that internally force insertion, such as City.objects.create(…):

django.db.utils.IntegrityError: duplicate key value violates unique constraint "cities_city_pkey"
DETAIL:  Key (id)=(1) already exists.

Cause: In the SlugModel.save() code that saves the object twice, if the force_insert flag was passed, it will also be passed on to the second save() call, even though by this point the object is assumed to be created. This inadvertently forces the existing object to be saved with an INSERT instead of an an UPDATE, which causes the integrity error above.

This PR clears the force_insert flag for the second save, which should avoid the problem. It also adds some basic regression tests for the affected models: let me know if those look okay.

PiDelport commented 6 years ago

Cool, thanks!

blag commented 6 years ago

I forgot to mention that this is in v0.5.0.5 on PyPI. I'm releasing v0.5.0.6 shortly that will include this and support for Django 2.0+.

blag commented 6 years ago

Thanks for your contribution!