Opened 7 years ago

Closed 7 years ago

Last modified 4 years ago

#6406 closed (duplicate)

form_save_add and form_save_change method for ModelAdmin

Reported by: michelts Owned by: nobody
Component: contrib.admin Version: newforms-admin
Severity: Keywords: form object save commit
Cc: michelts@…, bpederse@…, cmawebsite@… Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: UI/UX:

Description

ModelAdmin calls form.save() inside the save_add method:

def save_add(self, request, model, form, formsets, post_url_continue):
    ...
    new_object = form.save(commit=True)
    if formsets:
        for formset in formsets:
            formset.instance = new_object
            formset.save()
    pk_value = new_object._get_pk_val()
    ...

I want to be able to hack the form.save method and calculate aditional data to the object based on the request. Basically, I want to attach the request.user to an attribute of my just added object. I have a solution, I can call a save_form_add method of ModelAdmin that saves the form:

def save_add(self, request, model, form, formsets, post_url_continue):
    ...
    new_object = self.save_form_add(request, model, form, formsets, post_url_continue)
    pk_value = new_object._get_pk_val()
    ...

def save_form_add(self, request, model, form, formsets, post_url_continue):
    new_object = form.save(commit=True)
    if formsets:
        for formset in formsets:
            formset.instance = new_object
            formset.save()
    return new_object

This way I can do something like this:

class ObjectOptions(admin.ModelAdmin):
    def form_save_add(self, request, model, form, formsets, post_url_continue):
        new_object = form.save(commit=False)
        new_object.user = request.user
        new_object.save()
        if formsets:
            for formset in formsets:
                formset.instance = new_object
                formset.save()
        return new_object  

I didn´t find any other solution to intercept the object save and have access to the request.
What do you think about the addition of save_form_add and save_form_change? I will write the proper patch.

Best regards!

Attachments (2)

form_save_hooks.diff (2.5 KB) - added by Alex 7 years ago.
Ok, this is a fairly simple implementation, if this is not what was intended let me know so I can fix it, fair simple/
admin_options.diff (2.1 KB) - added by tanzer@… 7 years ago.
Factor _save_form and _save_formset from save_add and save_change.

Download all attachments as: .zip

Change History (13)

comment:1 Changed 7 years ago by brosner

  • Keywords nfa-blocker added
  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Design decision needed

I see what you are trying to accomplish and it can be accomplished by overriding save_add and save_change based on what you are trying to do. However, both of those methods are doing alot. I feel this stuff needs to be encapsulated in its own that is then worked with in the view. I will begin to think about this a bit more. Basically it needs to be handled one level higher than this.

Changed 7 years ago by Alex

Ok, this is a fairly simple implementation, if this is not what was intended let me know so I can fix it, fair simple/

comment:2 Changed 7 years ago by mathwizard

What I understand. This is probably a duplicate of #6002 as 6002 is more generic and provides more hooks. BTW...is this issue heading anywhere? I see this as an important feature. Copying the whole save_add/change method to my Admin options class is a terrible waste.

comment:3 Changed 7 years ago by garcia_marc

  • milestone set to 1.0 alpha

comment:4 in reply to: ↑ description Changed 7 years ago by tanzer@…

Replying to michelts:

I think it is better to factor two smaller methods:

    def _save_form(self, request, model, form, commit=True) :
        return form.save(commit=commit)

    def _save_formset(self, request, model, formset) :
        return formset.save()

This way one doesn't need to repeat the formsets loop when one wants to add an attribute to form. I'll attach a patch for this.

Changed 7 years ago by tanzer@…

Factor _save_form and _save_formset from save_add and save_change.

comment:5 Changed 7 years ago by anonymous

  • Cc bpederse@… added

comment:6 Changed 7 years ago by CollinAnderson

  • Cc cmawebsite@… added

comment:7 Changed 7 years ago by brosner

  • Keywords nfa-blocker removed
  • milestone changed from 1.0 alpha to 1.0 beta

This actually really isn't critical for a merge. I am going to drop the nfa-blocker and bump to 1.0 beta. I will look at it later.

comment:8 Changed 7 years ago by julien

Some other approaches have been proposed in #8005. Not sure which ticket should be considered most in this regard.

comment:9 Changed 7 years ago by jacob

  • Triage Stage changed from Design decision needed to Accepted

comment:10 Changed 7 years ago by jacob

  • Resolution set to duplicate
  • Status changed from new to closed

This is dancing around the same problems as #6002 and #8005. For sanity's sake, let's make #6002 the master here.

comment:11 Changed 4 years ago by jacob

  • milestone 1.0 beta deleted

Milestone 1.0 beta deleted

Note: See TracTickets for help on using tickets.
Back to Top