Switch away from django-multiselectfield.

Review Request #13561 — Created Feb. 20, 2024 and submitted — Latest diff uploaded

Information

Review Board
release-7.x

Reviewers

The django-multiselectfield package hasn't been maintained in a few
years, and it doesn't support Django 4.x+. There's a third-party fork
which has had some updates, but no releases that we can consume.

django-multiselectfield itself isn't very big, and of what it offers, we
don't actually use very much. A new field has been added to Djblets to
handle the list serialization/deserialization for comma-separated
values. This change switches us over to that.

The major difference here is that we have to instantiate the form
field ourselves. While the old MultiSelectField had its own validation
for the database side, we can rely on the form's MultipleChoiceField
validation.

This also manually sets a max_length on the field. The MultiSelectField
would compute the max_length based on the provided choices, but that
meant that any change to the choices would need an evolution.

  • Ran unit tests.
  • Ran the new evolution.
  • Tested the WebHookTarget admin form and verified that everything
    worked as expected.

Diff Revision 3

This is not the most recent revision of the diff. The latest diff is revision 4. See what's changed.

orig
1
2
3
4

Commits

First Last Summary ID Author
Switch away from django-multiselectfield.
The django-multiselectfield package hasn't been maintained in a few years, and it doesn't support Django 4.x+. There's a third-party fork which has had some updates, but no releases that we can consume. django-multiselectfield itself isn't very big, and of what it offers, we don't actually use very much. A new field has been added to Djblets to handle the list serialization/deserialization for comma-separated values. This change switches us over to that. The major differences here are that we have to instantiate the form field ourselves, and the new Djblets field doesn't actually have validation that the entries in the list are part of the choices. The validation isn't important to us because the checkbox widget that Django provides does the right thing for the typical case, and our notifications backend doesn't actually care if there's unknown items in the events list. This also manually sets a max_length on the field. The MultiSelectField would compute the max_length based on the provided choices, but that meant that any change to the choices would need an evolution. Testing Done: - Ran unit tests. - Ran the new evolution. - Tested the WebHookTarget admin form and verified that everything worked as expected.
0a92d9d7bf0b59e438f845e80d8535de84ac77c4 David Trowbridge
reviewboard/dependencies.py
reviewboard/notifications/forms.py
reviewboard/notifications/managers.py
reviewboard/notifications/models.py
reviewboard/notifications/evolutions/__init__.py
reviewboard/notifications/evolutions/webhooktarget_events_charfield.py
Loading...