ValueError with inline and save as new

[Luca Lenardi]

Given two models that are related to each other with a foreign key, as follows:

class Post(models.Model):
    title = models.CharField()
    slug = models.CharField(max_length=200, unique = True)

class Block(models.Model):
    post = models.ForeignKey(Post)
    slug = models.CharField(max_length=200, unique = True)

Suppose you have set save_as = true in the admin.py for the class PostAdmin. When you try to create a new Post, generated from an existing one with the "save as new" button after correcting some validation errors, the following exception is being generated:
ValueError: invalid literal for int() with base 10: ''

If you take a look at the request POST parameters, you could find some empty values for the block_set-N-post block_set-N-id (FK and id).

To replicate the error, follow this sequence:

1. Create a new object with at least one inline
2. Save it as new with some validation errors
3. Try to save it again, after correcting the errors of the form

[Karen Tracey]

#10448 was an earlier ticket in the same area, but it does not seem to have addressed the inline issue.

[Luca Lenardi]

The error appears to be related with the value of save_as_new in options.py around line ​730.

When the form is submitted the first time, it is set to True, while the second time, after the validation errors, it is set to False since it actually creates a new object. Forcing the value to True, the object is being saved without exceptions (as expected).

[paulos]

If inline formset contains an image or file field, form will always fail to validate on save_as (#14760).

[paulos]

Replying to gptvnt:

I think the most reliable way to correct this is creating a new admin URL like:

{{app_name}}/{{model_name}}/{{id}}/save_as/

or

{{app_name}}/{{model_name}}/clone/{{id}}/

[Shu Zong Chen]

Unless a new admin URL is created, the only way to get the current add_view form to behave is to replace the values for INITIAL_FORMS and TOTAL_FORMS, as well as the fk_field value. The INITIAL_FORMS is the cause of the ValueError of the ticket (setting it to 0 makes things happy), and the TOTAL_FORMS/fk_field mess with the form if you try to add any new inlines while you are fixing the validation errors. The TOTAL_FORMS needs to be set to the old INITIAL_FORMS value, and the fk_field needs to be blank.

[kevin1024]

OK, I spent some time hacking on this today. Here is a patch. Be gentle; it's my first contribution to Django. :)

[Preston Timmons]

Hi Kevin,

Thanks for putting a patch together. I tested it out. It applies cleanly to revision 16181 and seems to fix the issue here, but the changes to django/forms/formsets.py introduce problems into the model formsets:

ERROR: test_add_form_deletion_when_invalid (modeltests.model_formsets.tests.DeletionTests)
    ...
AttributeError: 'PoetFormFormSet' object has no attribute 'save_as_new'

I'm marking this one as needing improvement.

[anonymous]

OK, let's try this again.

[Preston Timmons]

Thanks, Kevin. I verified your new patch fixes the problem.

I'm hesitant to mark it ready for check-in. The "save_as_new" property is really a part of BaseInlineFormSet. Adding a check for it in BaseFormSet seems wrong. It might be more appropriate to put this fix in a custom "_management_form" method on the BaseInlineFormSet instead.

I'm also attaching a separate patch. Instead of modifying the formset classes it modifies the add_view method to post the value "_saveasnew" on subsequent requests, even when form errors appear. When the form is redisplayed the admin switches from the change_view to the add_view and loses this piece of information in the process.

I'm not sure what the best solution is.

[Preston Timmons]

Patch that modifies add_view to maintain the _saveasnew parameter

[Preston Timmons]

After some more testing of 13223.v2.diff, I found one problem with it. Since it doesn't bind posted data to the management form, when the form is redisplayed it loses inline data if you add more blocks than the initial form count at the same time you use "save as new".

The newer patch that maintains "_saveasnew" in the template doesn't suffer from that problem.

[candlerb]

I have a related problem:

• model admin has save_as = True
• browse to existing model, click "Save as new"
• the model fails to validate (e.g. because of a failed uniqueness constraint)
• when the form is redisplayed with the error, I see "Save and add another" instead of "Save as new"

This behaviour still remains after applying 13223.templatefix.diff (onto released django-1.3), although I've not tried any of the other patches in this thread.

(Platform: python 2.6.5, Ubuntu 10.04.2 LTS)

[Ramiro Morales]

Replying to candlerb:

I have a related problem:

• model admin has save_as = True
• browse to existing model, click "Save as new"
• the model fails to validate (e.g. because of a failed uniqueness constraint)
• when the form is redisplayed with the error, I see "Save and add another" instead of "Save as new"

This behaviour still remains after applying 13223.templatefix.diff (onto released django-1.3), although I've not tried any of the other patches in this thread.

This would be #5681 -- In particular, look at this comment for some discussion about why that behavior isn't wrong IMHO.

[Ramiro Morales]

I've updated the patch contributed by prestontimmons with a more complete test case that simulates a complete workflow as originally reported.

[Ramiro Morales]

Patch using the fix strategy proposed by contributor prestontimmons with enhanced tests

[Ramiro Morales]

Patch updated to current trunk status, still needs work

[Ramiro Morales]

I've attached an updates version of the patch that simply allows it to apply cleanly in trunk.

But th OP in ticket #17417 (slightly related to this one) makes a good point regarding the current buggy behavior and that isn't fixed by this patch:

If you press "Save as New" without changing the slug field a ValidationError? is raised, and the for is displayed to the user with a warning (as it should). However, the url is that of the old entity ...

That means that at that point the user is presented with the admin add view/form for a new instance of the model, pre-filled with values of the original instance (the one whose edit view/form she was looking at when she decided to press the Save as new button) but the URL in the browser address bar hasn't changed.

IMHO this is semantically incorrect and puts comment:5 in a new perspective. Perhaps we need to implement an intermediate view as suggested there?

[Igor Sobreira]

I was wondering how this intermediate view could be implemented.

My idea is to replace the "Save as New" button for a link to a clone/ view. This view would present a pre-filled form with the data from the original object you'd like to clone. It's (very) similar to add_view , but using initial to fill the form based on the original object.
The formsets are also pre-filled using initial .

All of this happens on GET. POSTing is the same as add_view's POST.

This is the same to call add_view passing initial values as GET parameters. But I think a clone/ url is much cleaner.

Let me know if I'm on the right track so I can continue with the implementation. It's ugly with lot's of duplication and incomplete at this moment, the patch is just a prototype. But I need some feedback before moving on...

[Igor Sobreira]

Prototype for an intermediate clone/ view

[Igor Sobreira]

I'm pushing small commits to my git fork: ​https://github.com/igorsobreira/django/commits/master (if anyone is interesting in follow or review step-by-step).

When I'm done I'll attach the full patch here.

[anonymous]

Did anyone else pick this up? The github link above has disappeared to try and pick up his line of work.

[dakinsloss]

Did anyone else pick this up? The github link above has disappeared to try and pick up his line of work.

[Igor Sobreira]

Sorry for the broken link @dakinsloss. Since the review/feedback was running to slow I ended up hacking it as an external app.

​https://github.com/RealGeeks/django-modelclone

It's basically the intermediate 'clone' view, same form as 'add_view' but already pre-populated.

[Tim Graham]

Duplicate of #23387 which has been fixed in master (1.9) by only showing the "Save as new" button if it's selected and there are validation errors.