search

danirus / django-comments-xtd

A pluggable Django comments application with thread support, follow-up notifications, mail confirmation, like/dislike flags, moderation, a ReactJS plugin and Bootstrap 5.3.
https://django-comments-xtd.readthedocs.io
BSD 2-Clause "Simplified" License
594 stars 157 forks source link

initialize_nested_count when run in a migration can result in schema errors #271

Closed jxcl closed 3 years ago

jxcl commented 3 years ago

Since django-comments-xtd runs a management command that looks at the schema outside of the current migration it's running, it's possible for it to see a schema state that is not the same as the state the schema editor is in.

In my case, when migrating from an empty database, the django_comments_xtd.0008_auto_20200920_2037 migration was run before all of my user model migrations, causing initialize_nested_count to inspect my user model and try to access columns on the database that were not there.

This can be solved by specifying run_before in the user model migrations, but it would be nice if that were not necessary. It would be better to find a way to populate the nested count outside of a migration, or else have a separate command that utilizes the model state in the migration rather than the model state derived from the codebase.

Thanks!

danirus commented 3 years ago

Thank you for your report. I can't figure how can initialize_nested_count inspect your user model. Could you please, make a repository with the code? I would like to see it in detail.

danirus commented 3 years ago

I finally reproduced the error. The management command initialize_nested_count does inspect the user model, but only implicitly. user is an attribute of the model tree of which XtdComment is part of. The problem is indeed as you mention, that the user model, whatever it is, is not yet reflected in the DB as defined in code, at the moment the migration 0008 tries to run. Thank you again for the report.

danirus commented 3 years ago

The fix has been merged in master. The solution consist of not running the management command in association with the migration 0008. Django developers who want to use the nested_count attribute can run the command manually. I have added a new section in the documentation to explain the initialize_nested_count.