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

Fixed #35449 -- Fixed validation issue with SplitArrayField when remove_trailing_nulls=True #18178

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

saJaeHyukc
Copy link
Contributor

Trac ticket number

ticket-35449

Branch description

The proposed changes ensure that trailing null values are correctly removed before validation occurs, and that errors are appropriately handled.

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 checked the "Has patch" ticket flag in the Trac system.
  • I have added or updated relevant tests.
  • I have added or updated relevant docs, including release notes if applicable.
  • I have attached screenshots in both light and dark modes for any UI changes.

@saJaeHyukc saJaeHyukc changed the title Fixed #35449 -- Fixed validation issue with SplitArrayField when remove_trailing_nulls=True Fixed #35449 -- Fixed validation issue with SplitArrayField when remove_trailing_nulls=True May 18, 2024
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 for this @saJaeHyukc!
I have a couple of comments 👍

Comment on lines +1349 to +1356
field.clean(["abc", "", ""])
self.assertEqual(
cm.exception.messages,
[
"Item 1 in the array did not validate: Ensure this value has at most 2 "
"characters (it has 3).",
],
)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
field.clean(["abc", "", ""])
self.assertEqual(
cm.exception.messages,
[
"Item 1 in the array did not validate: Ensure this value has at most 2 "
"characters (it has 3).",
],
)
field.clean(["abc", "", "", ""])
self.assertEqual(
cm.exception.messages,
[
"Item 1 in the array did not validate: Ensure this value has at most 2 "
"characters (it has 3).",
"Item 2 in the array did not validate: This field is required.",
"Item 3 in the array did not validate: This field is required.",
],
)

Sorry, I think the suggested test case wasn't exactly right.
I think because required=True we should have errors for missing Item 2 and 3 but because remove_trailing_nulls=True we can ignore that the list is too long 🤔

docs/releases/5.0.7.txt Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants