Skip to content

URB-3649 Fix TypeError in selecte2widget - #578

Open
WBoudabous wants to merge 1 commit into
2.9.xfrom
URB-3649_fix_error_opening_dossiers
Open

URB-3649 Fix TypeError in selecte2widget#578
WBoudabous wants to merge 1 commit into
2.9.xfrom
URB-3649_fix_error_opening_dossiers

Conversation

@WBoudabous

@WBoudabous WBoudabous commented Jun 5, 2026

Copy link
Copy Markdown
Contributor

This PR fixes a TypeError in Select2Widget.view() when opening dossiers and adds a regression test to ensure the fix continues to work correctly.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

This PR prevents a TypeError in Select2Widget.view when processing list-stored field values by normalizing them into a consistent list format before vocabulary resolution, includes a regression test validating the fix, and documents the change in the news file.

Changes

Select2Widget List Value TypeError Fix

Layer / File(s) Summary
Select2Widget.view value normalization
src/Products/urban/widget/select2widget.py
Select2Widget.view now normalizes the value returned from the parent widget into a list: it uses list(value) for list/tuple inputs and wraps scalars in a one-element list before passing to resolve_vocabulary, fixing the TypeError that occurred with list values.
Regression test for list value handling
src/Products/urban/tests/test_select2widget.py
New test module TestSelect2Widget with regression test test_view_list_value_does_not_raise that verifies widget.view handles list-stored field values without raising TypeError, mocking field accessor and vocabulary resolution.
Bugfix news entry
news/URB-3649.bugfix
News entry documents the fix preventing TypeError when opening dossiers with list-stored values in Select2Widget fields.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~8 minutes

Possibly related PRs

  • IMIO/Products.urban#562: Both PRs modify src/Products/urban/widget/select2widget.py's Select2Widget.view to change how values are normalized and passed into resolve_vocabulary.

Suggested reviewers

  • mpeeters

Poem

🐰 A widget that stumbled when lists came to play,
Now gently unwraps them in just the right way,
Lists, tuples, and singles all sorted with care,
No more TypeError floating through the air! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title refers to fixing a TypeError in select2widget, which is the main objective of the PR, but contains a typo ('selecte2widget' instead of 'select2widget').
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch URB-3649_fix_error_opening_dossiers

Comment @coderabbitai help to get the list of available commands and usage tips.

@WBoudabous
WBoudabous requested a review from mpeeters June 5, 2026 13:37
Comment on lines +12 to +22
widget = Select2Widget()
field = Mock()
field.__name__ = "test_field"
field.getAccessor.return_value.return_value = ["value1", "value2"]

with patch(
"Products.urban.widget.select2widget.resolve_vocabulary",
return_value="value1, value2",
):
result = widget.view(Mock(), field, Mock())
self.assertEqual(result, "value1, value2")

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is too much complexity in this test, here is an example of how it should be done:

        context = Mock()
        context.test_field = ["value1", "value2"]
        field = Mock()
        field.__name__ = "test_field"

        with patch(
            "Products.urban.widget.select2widget.resolve_vocabulary",
            return_value="value1, value2",
        ):
            result = Select2Widget().view(context, field, Mock())
            self.assertEqual(result, "value1, value2")

Also:

  • you should test from without mocking Products.urban.widget.select2widget.resolve_vocabulary
  • the test file should be under Products/urban/widget/tests/

Comment on lines +73 to +76
if isinstance(value, (list, tuple)):
values = list(value)
else:
values = [value]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is correct but should be in the other way to ensure that even if we get a iterable value, we get a string (first value).
resolve_vocabulary should be adapted to accept either a string or a iterable.

Comment thread news/URB-3649.bugfix
@@ -0,0 +1,2 @@
Fix TypeError when opening dossiers

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please improve the changelog to express exactly what was the issue

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