[Django] #16327: "save as new" redirects to list view instead of newly-created item
Django
-----------------------+-------------------------------
Reporter: candlerb | Owner: nobody
Type: Bug | Status: new
Milestone: | Component: contrib.admin
Version: 1.3 | Severity: Normal
Keywords: saveasnew | Triage Stage: Unreviewed
Has patch: 1 | Easy pickings: 0
UI/UX: 0 |
-----------------------+-------------------------------
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: candlerb | Owner: nobody
Type: | Status: new
Uncategorized | Component: contrib.admin
Milestone: | Severity: Normal
Version: 1.3 | Keywords: saveasnew
Resolution: | Has patch: 1
Triage Stage: Design | Needs tests: 0
decision needed | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Changes (by ramiro):
* needs_better_patch: => 0
* stage: Unreviewed => Design decision needed
* type: Bug => Uncategorized
* needs_tests: => 0
* needs_docs: => 0
Comment:
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
[https://code.djangoproject.com/browser/django/trunk/django/contrib/admin/options.py?rev=16422#L786
code block] in `ModelAdmin.response_change()` that supposedly handles this
has been dead code since its introduction in r8273.
See also #8001.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:1>
Django
Reporter: candlerb | Owner: nobody
Uncategorized | Component: contrib.admin
Milestone: | Severity: Normal
Version: 1.3 | Keywords: saveasnew
Resolution: | Has patch: 1
Triage Stage: Design | Needs tests: 0
decision needed | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 0 |
UI/UX: 0 |
-------------------------------------+-------------------------------------
Comment (by candlerb):
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' # ???
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:2>
Django
Reporter: candlerb | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Design
Has patch: 1 | decision needed
Needs tests: 0 | Needs documentation: 0
Easy pickings: 0 | Patch needs improvement: 0
| UI/UX: 0
-------------------------------------+-------------------------------------
Changes (by aaugustin):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:3>
Django
Reporter: candlerb | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by jacob):
* stage: Design decision needed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:4>
Django
Reporter: candlerb | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by ste@…):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:5>
Django
Reporter: candlerb | Owner: nobody
Type: Bug | Status: new
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:6>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by giuliettamasina):
* owner: nobody => giuliettamasina
* status: new => assigned
Comment:
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:7>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
Feel free to make a proposal on the DevelopersMailingList. That's the best
venue for getting feedback.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:8>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by giuliettamasina):
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.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:9>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by giuliettamasina):
Pull request is here: https://github.com/django/django/pull/6403
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:10>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by giuliettamasina):
* needs_better_patch: 1 => 0
Comment:
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?
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:11>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by giuliettamasina):
Test fixed and passing now.
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:12>
Django
Reporter: candlerb | Owner: giuliettamasina
Type: Bug | Status: assigned
Severity: Normal | Resolution:
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Comment (by giuliettamasina):
Hey Tim, have you had the chance to look at this one again?
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:13>
Django
Reporter: candlerb | Owner: giuliettamasina
Component: contrib.admin | Version: 1.3
Severity: Normal | Resolution: fixed
Keywords: saveasnew | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"08cd6a0e56bac04e322388e7059072ad98db303c" 08cd6a0]:
{{{
#!CommitTicketReference repository=""
revision="08cd6a0e56bac04e322388e7059072ad98db303c"
Fixed #16327 -- Redirected "Save as new" to change view instead of the
changelist.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/16327#comment:14>