Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with merge form action when querystring is present. #927

Merged
merged 2 commits into from
Nov 5, 2024

Conversation

garrettc
Copy link
Contributor

@garrettc garrettc commented Nov 2, 2024

Querystrings are preventing the tags from merging, because get_full_path() in the redirect preserves querystrings.

Steps to reproduce

  1. Load the admin area > Taggit > Tags
  2. Sort by slug so that the path is /admin/taggit/tag/?o=2.1 (with querystring)
  3. Select two tags, eg: "Adventure" and "Anthology, then action "Merge selected tags"

Result

Because the resulting path is /admin/taggit/tag/?o=2.1merge-tags/ you stay on the listing page.

This also affects paging querystrings.

Fix

Use request.path instead of request.get_full_path().

@rtpg
Copy link
Contributor

rtpg commented Nov 4, 2024

Thanks for diagnosing this! Just changed this up a bit to call reverse directly instead of manually building up the URL, @garrettc would you be able to sanity check the changes I pushed up?

@garrettc
Copy link
Contributor Author

garrettc commented Nov 4, 2024

@rtpg Looks good to me.

@rtpg
Copy link
Contributor

rtpg commented Nov 5, 2024

alrighty, merging this! Thanks again

@rtpg rtpg merged commit ee3d2d1 into jazzband:master Nov 5, 2024
6 checks passed
@garrettc garrettc deleted the fix-querystring-bug branch November 5, 2024 01:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants