Opened 13 years ago
Closed 9 years ago
#16327 closed Bug (fixed)
"save as new" redirects to list view instead of newly-created item
Reported by: | candlerb | Owned by: | Markus Amalthea Magnuson |
---|---|---|---|
Component: | contrib.admin | Version: | 1.3 |
Severity: | Normal | Keywords: | saveasnew |
Cc: | Triage Stage: | Accepted | |
Has patch: | yes | Needs documentation: | no |
Needs tests: | no | Patch needs improvement: | no |
Easy pickings: | no | UI/UX: | no |
Description
I have a model with save_as = True
After browsing to the model, changing some parameters and clicking "Save As", the browser redirects to the list of all objects instead of the newly-saved model instance.
There is code in response_change
which looks like it is supposed to handle this case. However, after putting in some sys.stderr.write's, I see that the request is actually ending up in response_add
instead :-(
The attached one-line patch to django/contrib/admin/options.py fixes it for me. However I'm not sure if that's right for all circumstances, nor whether the code in response_change which tests for "_saveasnew"
is always unused and should also be removed.
Attachments (1)
Change History (15)
by , 13 years ago
Attachment: | django-16327.diff added |
---|
comment:1 by , 13 years ago
Triage Stage: | Unreviewed → Design decision needed |
---|---|
Type: | Bug → Uncategorized |
comment:2 by , 13 years ago
The use case is to be able to make multiple clones of an existing model, each slightly different. It's painful if you have to keep re-locating the just-saved model in the changelist.
"continue editing" is arguably orthogonal to "save / save as new" - but I don't want four buttons :-)
Suggestion: permit _continue
as a separate form field. That is, the posted data could contain both _saveasnew
and _continue
. You could then add _continue
as a hidden field (for fixed behaviour) or a checkbox.
For this to work, we just need to ensure that both flags are honoured if they are both present in the view code. It should be backwards-compatible, since _continue
can also be a submit button.
I would like to be able to control this from the ModelAdmin
without having to write my own submit_line
though. e.g.
save_as = True continue_editing = False / True / 'ask' # ???
comment:3 by , 12 years ago
Type: | Uncategorized → Bug |
---|
comment:4 by , 12 years ago
Triage Stage: | Design decision needed → Accepted |
---|
comment:5 by , 11 years ago
This is still a problem with 1.6.1. I agree with candlerb - the 'save as' feature is useful for quickly hammering out lists of objects where only 1 or 2 (of many) fields change each time.
I think ModelAdmin.continue_after_save_as = False
would probably be the best way to achieve this. It wouldn't break backwards compatibility. If anyone else thinks this is a good idea, I can hack up a patch.
comment:6 by , 10 years ago
Patch needs improvement: | set |
---|
I think changing "Save as new" to redirect to the newly created object is probably okay, but someone might complain. If we have to add ModelAdmin.save_as_continue
(I'd prefer that name so it's in the docs next to save_as
) then that seems reasonable. Simply adding a fourth "Save as new and continue editing" button (if save_as = True
) might be fine as well. Whatever happens, I hope we can keep the solution as simple as possible.
comment:7 by , 9 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
Can we get some sort of final decision on this one? I would love this (and personally I'd like the "Save as new" button to always redirect to the created object.) In any case, when it's decided on that, or a separate attribute(s), I can continue work on this. Don't know if the five year old patch is in shape to continue, I might just start over.
comment:8 by , 9 years ago
Feel free to make a proposal on the DevelopersMailingList. That's the best venue for getting feedback.
comment:9 by , 9 years ago
During work with tests for this, I've discovered that a side-effect from this change is that a user needs both the add and change permissions to be able to use the "Save as new" button, as opposed to only needing the add permission before. (It causes tests to fail currently.)
What do we think about that?
One way would be to redirect to the change page if the user has change permissions, otherwise redirect to the list view, i.e. the previous behavior.
comment:11 by , 9 years ago
Patch needs improvement: | unset |
---|
I've fixed everything based on Tim's comments in the pull request, except this:
I've written a new test that is currently failing because I can't figure out a detail about it: https://github.com/django/django/pull/6403/files#diff-c4a8cc513ba1b80d8c9887121b881437R1128
To test the option, I create a custom model admin and register it instead of the previous PersonAdmin. However, even though when inspecting the registry of the site object I see my new custom model admin, the request on the row after my register seems to use the previous PersonAdmin anyway.
What might be wrong here? Is there a better way to write this test?
tl;dr: We need to decide if after clicking the 'Save as new' in a model change view the user should be taken to the changelist like we are doing or to the change view of the model instance just cloned as the OP proposes.
The behavior of redirecting to the model changelist (or the admin app index page) has been so since newforms admin was merged back in r8273 (with later fixups like the ones in #10448/r10713). And yes, AFAICT the code block in
ModelAdmin.response_change()
that supposedly handles this has been dead code since its introduction in r8273.See also #8001.