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

[marianaasilva]

Fix for ticket 12241 to ensure the query string is preserved when clicking "Save and add another" or "Save and continue"

Added tests for query string with changelist filters

Based on @matthewn's patch pr/16206

[LilyFoote]

Nice to see progress on this!

I see that we also have _saveasnew in response_change - is this deliberately handled differently? I see it's bundled with the _continue handling in response_add, so we're changing the behaviour there.

Either way, I think we should add tests for this case too.

[marianaasilva]

Nice to see progress on this!

I see that we also have _saveasnew in response_change - is this deliberately handled differently? I see it's bundled with the _continue handling in response_add, so we're changing the behaviour there.

Either way, I think we should add tests for this case too.

@LilyFoote Thank you for the feedback 😄

After looking more closely into this, I believe the _saveasnew condition in response_change is never reached? From what I understood, the response_add and response_change methods get called in _changeform_view but if the request method is a POST and _saveasnew is present in the request data, it will always default to response_add

[LilyFoote]

I think you're right! In that case I think it's reasonable to delete the unreachable code. Though perhaps in a separate PR. @felixxm @nessita Thoughts?

For this PR, I still think we should add a test for preserving the query strings (or explicitly not preserving them) for the _saveasnew option in response_add, since that is reachable if I understand correctly.

[marianaasilva]

I think you're right! In that case I think it's reasonable to delete the unreachable code. Though perhaps in a separate PR. @felixxm @nessita Thoughts?

For this PR, I still think we should add a test for preserving the query strings (or explicitly not preserving them) for the _saveasnew option in response_add, since that is reachable if I understand correctly.

Makes sense! Already updated the tests to include the _saveasnew option. Thank you!

[shangxiao]

(Adding my 2¢ since I looked at the previous PR)

Just tested it out, looks nice 🏆

I didn't really read the code in anger although the only thing that catches my eye is "qsl" which made me wonder what it meant … eventually saw the docs for parse_qsl() as returning list.

[felixxm]

@marianaasilva Thanks 👍 Have you checked how it works with options that are currently preserved (_popup, and _to_field)? We should add tests for them, check out #16206 (comment).

[felixxm]

I think you're right! In that case I think it's reasonable to delete the unreachable code. Though perhaps in a separate PR. @felixxm @nessita Thoughts?

For this PR, I still think we should add a test for preserving the query strings (or explicitly not preserving them) for the _saveasnew option in response_add, since that is reachable if I understand correctly.

Good catch 🎯 Yes we should remove it in a separate PR. @marianaasilva Would you like to prepare it?

[felixxm]

@marianaasilva Thanks 👍

I squashed commits, and simplified tests.

[marianaasilva]

I think you're right! In that case I think it's reasonable to delete the unreachable code. Though perhaps in a separate PR. @felixxm @nessita Thoughts?
For this PR, I still think we should add a test for preserving the query strings (or explicitly not preserving them) for the _saveasnew option in response_add, since that is reachable if I understand correctly.

Good catch 🎯 Yes we should remove it in a separate PR. @marianaasilva Would you like to prepare it?

Sure! Working on it 😄