search

feincms / form-designer

A simple form designer for Django
https://form-designer.readthedocs.io/
BSD 3-Clause "New" or "Revised" License
75 stars 32 forks source link

changing field types triggers creation of new migration #30

Closed benzkji closed 4 years ago

benzkji commented 4 years ago

./manage.py makemigrations with changed field types will trigger a new migration, that is created in site-packages/form_designer/migrations - not what one wants. I propose to add a form to the inline, that then would define the choices, so they can be changed at any time. The db field would become a simple CharField.

matthiask commented 4 years ago

The models code requires the field types too; but yeah, the field types initialization could be even lazier.

OTOH adding choices to the model field makes sense too since it also ensures that the model has get_*_display methods etc.

This approach works well in feincms3, and could serve as an inspiration for a solution to this problem: https://github.com/matthiask/feincms3/blob/cbcb9a94cfb701e69fce62a78bcf911fa0da7214/feincms3/mixins.py#L54

benzkji commented 4 years ago

aaahh. get_*_display. the template mixin looks goods, as far as I can understand. I was a bit short thinking...just got in a migration mess on a sunday morning ,-) - documenting it (need of having form_designer in MIGRATION_MODULES) would also be a viable, less expensive option.

jin-park-dev commented 4 years ago

I was also about to make post about this. I tripped over same issue this week and took some time to figure out why migration was failing on CI system.

benzkji commented 4 years ago

so, basically initializing with ("", ""), and adding real values only after model initialization (class_prepared signal), that would be it, right? I hope to stikc together a PR soonclass_prepared!

matthiask commented 4 years ago

I don't think that's late enough; the migrations framework will probably still see the values when they are assigned in class_prepared.

You probably have to move the assignment of field types into a modelform used in the admin panel.

benzkji commented 4 years ago

ahhh. my initial thought, but not having a CharField only...see. will test. in feincms3 you do it then. curious.

matthiask commented 4 years ago

Ah yes; changing the template configuration or apps etc. in feincms3 generates migrations actually, so the interesting part for this issue is only the ("", "") thing.

matthiask commented 4 years ago

Thank you; I found a less intrusive way of fixing this, which only affects the migration machinery.

matthiask commented 4 years ago

Heads up: 0.17 is now available hopefully fixing this problem for good!

benzkji commented 4 years ago

nice thx!