Skip to content

Conversation

nessita
Copy link
Contributor

@nessita nessita commented Aug 16, 2024

Trac ticket number

ticket-10941

Branch description

This branch adds extra tests for the querystring template tag, in preparation for merging PR #18368.
This branch also implements a small test refactoring to avoid duplication, inspired by utils' tests for safestring assertRenderEqual.

Checklist

  • This PR targets the main branch.
  • The commit message is written in past tense, mentions the ticket number, and ends with a period.
  • I have added or updated relevant tests.

@nessita nessita requested a review from knyghty August 16, 2024 17:49
@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch from 0db2b42 to 946cefc Compare August 16, 2024 18:11
@nessita nessita changed the title Refs #10941 -- Added tests for querystring template tag to handle empty params. Refs #10941 -- Added tests for querystring template tag. Aug 16, 2024
@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch 2 times, most recently from 4d207d2 to b518f43 Compare August 19, 2024 12:23
@nessita
Copy link
Contributor Author

nessita commented Aug 19, 2024

I split the diff in two commits, the first one introduces the helper and does the refactor, the second adds two new tests.

Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I have small suggestions

@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch from b518f43 to 69b1623 Compare August 20, 2024 21:35
@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch from 69b1623 to d6ab249 Compare October 15, 2024 18:24
@g-nie
Copy link
Contributor

g-nie commented Nov 20, 2024

Hey, how is this looking? I'd like to finish off #18368 once this is merged ✨

@nessita
Copy link
Contributor Author

nessita commented Nov 20, 2024

Hey, how is this looking? I'd like to finish off #18368 once this is merged ✨

@g-nie Thank you for the ping! Let me resurface and update this, with the goal of landing this ASAP. Sorry for the delay and thank you for your patience.

@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch from d6ab249 to ecd8df1 Compare November 20, 2024 18:18
…g template tag.

Thank you Sarah Boyce for the review and suggestions.
These extra tests assert over the handling of empty params (None, empty
dict, empty QueryDict), and also for dicts having non-string keys.
@nessita nessita force-pushed the refs-10941-querystring-extra-tests branch from ecd8df1 to ccf0b93 Compare November 20, 2024 19:40
@nessita nessita requested a review from sarahboyce November 29, 2024 01:07
Copy link
Contributor

@sarahboyce sarahboyce left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@sarahboyce sarahboyce merged commit 15ca754 into django:main Nov 29, 2024
42 checks passed
@nessita nessita deleted the refs-10941-querystring-extra-tests branch November 29, 2024 12:04
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.

4 participants