Admin forgets URL used for prefilling forms when hitting Save and add another

[Odin Hørthe Omdal]

When I go to /admin/framlegg/patch/add/?document=5 everything works fine, the 5 is prefilled. However, when I hit «save and add another» the GET request variable isn't in the URL any more.

[Batiste Bieler]

Keep the request's query string when the button "Save and continue" or "Add and continue" are used.

[Ramiro Morales]

See also #6903 that also proposes preserving the query string in another admin use case.

[Batiste Bieler]

Preserve query string, with tests

[Batiste Bieler]

The bug #6903 is quite a different beast. This one just keep the query string when you push the "Save and continue" button in the admin. As simple as that. No session needed​.

[Karen Tracey]

Sorry, but 1.2 milestone is only for release-critical bugs at this point and this does not qualify.

[anonymous]

Reading my last patch, I spotted what could be a small error:

?_popup=1" + request.META.get('QUERY_STRING')

Should be

?_popup=1&" + request.META.get('QUERY_STRING')

[Chris Beaven]

This isn't the neatest code:
?_popup=1" + request.META.get('QUERY_STRING')

I'd use request.GET.copy(), add _popup if necessary, and then only append the querystring if necessary.

And you should write a test for the case where there's a querystring already so that we're not talking about whether it "could be" an error ;)

[Batiste Bieler]

Better tests that goes trough all the use case

[Batiste Bieler]

There you go. The tests are more complete. I used the urllib and urlencode modules. Is that ok to use them within Django?

Cheers,
Batiste

[Batiste Bieler]

Simplification of the tests

[Batiste Bieler]

Simplification of the tests

[Luke Plant]

Changed back to 'Accepted' to reflect SmileyChris's criticisms. There doesn't seem to be any need to parse META['QUERY_STRING'] when the request object has done that already.

[Batiste Bieler]

Replying to lukeplant:

Changed back to 'Accepted' to reflect SmileyChris's criticisms. There doesn't seem to be any need to parse META['QUERY_STRING'] when the request object has done that already.

I am not sure if you are satisfied by the state of the patch? Do I need to make some more change and not parse METAQUERY_STRING? I would really like to see this issue fixed and I am ready to improve the patch if needed.

[Batiste Bieler]

Remove the urlparse call and use request.GET.copy instead

[Batiste Bieler]

The latest patch (preserve_query_string_with_tests_5.diff) removed the urlparse and use request.GET.copy() instead. The tests still pass. To test the 2 tests:

python runtests.py admin_views.AdminResponseChangeTest --settings test_sqlite

I am awaiting review of the new patch.

[Chris Beaven]

batiste, I've got a local branch with some more changes than your patch which I started yesterday. I'll get it pushed somewhere for you to look at, or at least attach it as a patch here.

There's an issue actually with the current logic of _ispopup which I want to address.

[Batiste Bieler]

Thanks Chris. Keep me updated with the progress.

[Chris Beaven]

Sorry for the lack of progress. Here's the branch, anyway: ​http://github.com/SmileyChris/django/compare/master...12241-preserve-querystring

[Batiste Bieler]

The commit looks good to me. Did you commit in the trunk already?

[Luke Plant]

Looks good to me. There seems to be a stray comment in one of the tests — "Technically, only the response_change" — that I can't make sense of.

[anonymous]

This looks like an unfinished comment. The test being exactly the same as the one above, I think it should be removed.

[Batiste Bieler]

Last comment was from me. Any news on this issue? I would really like to see it in 1.3.

[Chris Beaven]

It was me committing in the middle of a thought, then getting distracted and moving on to other stuff :P

The test looks the same, but the behaviour of the _saveasnew forces a response_add instead of a response_change from memory. I wanted to look at it a bit more, but still haven't got around to it yet.

[Batiste Bieler]

Isn't it there that you wanted to add the query string?:

​http://github.com/SmileyChris/django/blob/master/django/contrib/admin/options.py#L873

What can I do to help to push this on 1.3?

[Julien Phalip]

Note that urlparse.parse_qs() was introduced in Python 2.6 [1], so the patch needs to be modified to work with Python >= 2.4.

[1] ​http://docs.python.org/library/urlparse.html#urlparse.parse_qs

[Julien Phalip]

#10824 was closed as a dupe. Note that Malcolm and Chris had triaged the ticket differently than this one (i.e. considered as a new feature with DDN).

[Batiste Bieler]

Merged latest Django with chris changes, fix tests, added documentation on the test that was missing, removed parse_qs

​https://github.com/batiste/django/commit/af3285ca12604ddde1b7f2af65fed1f8634e340c
​https://github.com/batiste/django/commit/90708a9b162d7c6c2f48afe8a32a7e5e42b91e54

What else can I do?

[Batiste Bieler]

Last patch, tests, doc, no parse_qs

[Batiste Bieler]

Same as last one, but removing adding the proxy thing in add_view

[matthewn]

Greetings from DjangoCon US! First time contributor here with a pull request for this: ​https://github.com/django/django/pull/16206

Note: Earlier discussion in this ticket considered altering the behavior for 'save and continue' as well, but I am not sure that actually makes sense from a use case perspective. My proposed fix is much more targeted, addressing only the 'save and add another' issue mentioned in the ticket title.

[matthewn]

As requested in the PR, I've expanded my fix to cover save-and-add and save-and-continue on both add and change forms. Four tests cover the four cases. ​https://github.com/django/django/pull/16206

[Mariana]

​PR

[Lily Foote]

The patch looks good to me, but would probably benefit from a review from someone more familiar with the admin. There's also some code we think is dead that we're not sure whether to delete as part of this ticket or as a separate PR.

[Mariusz Felisiak <felisiak.mariusz@…>]

In fc62e177:

Fixed #12241 -- Preserved query strings when using "Save and continue/add another" in admin.

Co-authored-by: Grady Yu <gradyy@…>
Co-authored-by: David Sanders <shang.xiao.sanders@…>
Co-authored-by: Matthew Newton <matthewn@…>

[Mariusz Felisiak <felisiak.mariusz@…>]

In 0bbe6ca2:

[5.0.x] Fixed #12241 -- Preserved query strings when using "Save and continue/add another" in admin.

Co-authored-by: Grady Yu <gradyy@…>
Co-authored-by: David Sanders <shang.xiao.sanders@…>
Co-authored-by: Matthew Newton <matthewn@…>

Backport of fc62e17778dad9eab9e507d90d85a33d415f64a7 from main

[Odin Hørthe-Omdal Urdland]

This app I was working on when filing this bug 14 years ago is not used anymore. But I have a more recent one made 12 years ago which is still in production and could benefit from this in the next release. ;)

Thank you all! :)))