diff --git a/news/21.bugfix b/news/21.bugfix new file mode 100644 index 0000000..a5e9820 --- /dev/null +++ b/news/21.bugfix @@ -0,0 +1,2 @@ +Fix ``TypeError`` in the sharing view when two role entries tie on +``(sticky, type, name)``; [jmevissen] diff --git a/src/plone/app/workflow/browser/sharing.py b/src/plone/app/workflow/browser/sharing.py index ceacc2b..600b5ee 100644 --- a/src/plone/app/workflow/browser/sharing.py +++ b/src/plone/app/workflow/browser/sharing.py @@ -285,7 +285,10 @@ def existing_role_settings(self): (a["id"] not in self.STICKY, a["type"], a["name"], a) for a in items.values() ] - dec_users.sort() + # Sort only on the first three entries, + # the fourth is a dict and would raise a TypeError + # see plone/plone.app.workflow#21 + dec_users.sort(key=lambda d: (d[0], d[1], d[2])) # Add the items to the info dict, assigning full name if possible. # Also, recut roles in the format specified in the docstring diff --git a/src/plone/app/workflow/tests/test_sharing_view.py b/src/plone/app/workflow/tests/test_sharing_view.py index df11b1b..23416e9 100644 --- a/src/plone/app/workflow/tests/test_sharing_view.py +++ b/src/plone/app/workflow/tests/test_sharing_view.py @@ -234,3 +234,26 @@ def lrme_handler(context, event): # check subscriber called self.assertEqual(context.context, context) self.assertEqual(context.event, event) + + def test_existing_role_settings_sort_with_duplicate_names(self): + """Regression test for plone/plone.app.workflow#21. + + When two entries tie on (sticky, type, name), the old + ``dec_users.sort()`` fell through to comparing the item dicts at + index 3, raising ``TypeError: '<' not supported between instances + of 'dict' and 'dict'`` on Python 3. + """ + login(self.portal, "manager") + sharing = getMultiAdapter((self.portal, self.request), name="sharing") + + def fake_inherited_roles(): + return ( + ("shared_name", ("Reader",), "user", "user_a"), + ("shared_name", ("Reader",), "user", "user_b"), + ) + + sharing._inherited_roles = fake_inherited_roles + info = sharing.existing_role_settings() + ids = {entry["id"] for entry in info} + self.assertIn("user_a", ids) + self.assertIn("user_b", ids)