This change has been marked as completed.

Describe the completed change (optional):

Pushed to release-5.x (0165c8b)

Summary

Override validation for CommaSeparatedValuesField.

Review Request #14350 — Created Feb. 17, 2025 and submitted 1 month ago

Information

Owner

david*

Repository

Djblets

Branch

release-5.x

Bugs

Depends On

Reviewers

Groups

djblets

People

Description*

Testing Done

Verified that saving changes to a comma-separated values field using a
multi checkbox widget worked correctly.

Commits

Summary	ID
Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	49ea0fc4c63a7ee9ce186c0bb5b1ec9d046fe818

Summary

Override validation for CommaSeparatedValuesField.

The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.

49ea0fc4c63a7ee9ce186c0bb5b1ec9d046fe818

Issues

Description	From	Last Updated
Could we add unit tests for this?	maubin	1 month, 1 week ago
Let's make this a set. That'll be faster than scanning the whole list for every item in value.	chipx86	1 month, 1 week ago
There are no open issues

flake8 passed.

JSHint passed.

The issue has been resolved. Show all issues
```
Could we add unit tests for this?
```

Change Summary:

Add a unit test.

Commits:

	Summary	ID
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	5f39ca8382da6d7a4e3507622f30aa6c84e6bd01
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	0d09dfde6d2d1eadd76ae7a6d6c1564ea02404c5

Diff:

Revision 2 (+102 -4)

Show changes

	djblets/db/fields/comma_separated_values_field.py
	djblets/db/tests/test_comma_separated_values_field.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Ship it!

```
Ship It!
```

djblets/db/fields/comma_separated_values_field.py (Diff revision 2)
The issue has been resolved. Show all issues
```
Let's make this a set. That'll be faster than scanning the whole list for every item in value.
```

Commits:

	Summary	ID
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	0d09dfde6d2d1eadd76ae7a6d6c1564ea02404c5
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	9456ff03f727837a508fab58c6fe12114a6c6cd5

Diff:

Revision 3 (+272 -4)

Show changes

	pyproject.toml
	djblets/db/fields/comma_separated_values_field.py
	djblets/db/tests/test_comma_separated_values_field.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Commits:

	Summary	ID
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	9456ff03f727837a508fab58c6fe12114a6c6cd5
	Override validation for CommaSeparatedValuesField. The new comma separated values field that replaced the old django-multiselectfield dependency was falling back on the base field validation, which wasn't handling choices correctly. This change adds an override so we actually iterate through the value instead of just checking it directly. Testing Done: Verified that saving changes to a comma-separated values field using a multi checkbox widget worked correctly.	49ea0fc4c63a7ee9ce186c0bb5b1ec9d046fe818

Diff:

Revision 4 (+278 -4)

Show changes

	pyproject.toml
	djblets/db/fields/comma_separated_values_field.py
	djblets/db/tests/test_comma_separated_values_field.py

Checks run (2 succeeded)

flake8 passed.

JSHint passed.

Main change looks fine, but the lint rules should be their own change so they can be discussed separately from this fix.

david 1 month ago

Oops, that was definitely not meant to sneak into this. I've just been playing with various rules. Will make a proposal once I've worked out the kinks.

Status:: Completed
Change Summary:: Pushed to release-5.x (0165c8b)