Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Model form validation in collections: instance attribute not set #55

Closed
Actionb opened this issue Apr 14, 2023 · 3 comments
Closed

Model form validation in collections: instance attribute not set #55

Actionb opened this issue Apr 14, 2023 · 3 comments

Comments

@Actionb
Copy link

Actionb commented Apr 14, 2023

Collections create a copy of the underlying form for validation via HolderMixin.replicate. But replicate does not assign a model instance to model forms, leading to issues with validation.
When creating a new object, this isn't an issue. But when editing an existing object, the form's instance attribute must be set to that object for uniqueness validation to work properly.

To clarify, this only affects validation, more specifically uniqueness validation, because the unique checks take the instance's state and primary key into account: model._perform_unique_checks. Note that including the primary key field in the form is not enough: the form's model instance was instantiated as new object and it will remain so - instance._state.adding remains True until the object was saved.

To set the correct instance, I suggest trying to fetch the existing object from the database when the form is being replicated. I can whip up a PR if you want.

@jrief
Copy link
Owner

jrief commented Apr 14, 2023

Please retry with this branch: https://github.com/jrief/django-formset/tree/releases/1.0.x
or https://pypi.org/project/django-formset/1.0.dev0/

There the problem with validating unique indices is solved.
Unfortunately it will take a few additional weeks until I can release an official version, because I have to rewrite parts of the documentation.

@Actionb
Copy link
Author

Actionb commented Apr 17, 2023

Yes, that works for fields with unique=True.

I think I noticed another related issue: collections for related models do not know their parent instance during validation.

Inline formsets are instantiated with the parent instance of the relation, and that instance is passed on to the model forms as data for the InlineForeignKeyField. This way, the parent id is available for validation - most notably unique_together validation.

This does not happen for collections for related models.
You can work around this when editing existing relations of an existing parent: since model_to_dict includes the parent in the initial data of the relation's model form, you just need to include the formfield for the parent in the model form and it validates properly.
However, this doesn't work when you are adding a 'new parent', or a new relation to an existing parent. You need to validate the model form of the parent first, and then you can grab the form's instance and pass it on to the related collections. For example like it is done in django admin.

Currently when adding a new related object that would violate unique constraints, the validation for unique_together is skipped when the form of the new object is first validated, because the parent field is missing data. Later, when trying to save the duplicate object, the parent instance is available, and a second validation of the form in construct_instance fails as expected due to the unique constraint. The duplicate object is never saved, but the user doesn't know about this, because they are never presented an error.

@jrief
Copy link
Owner

jrief commented Apr 17, 2023

Please handle separate issues as such, and do not use an existing one for adding new features, otherwise we get confused.
I'm going to close this. Please open a new issue with the new request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants