general: lookup roles by slug (#60518)

tests/test_formdef_import.py

@@ -595,6 +595,13 @@ def test_workflow_roles(pub):
 
     fd2 = FormDef.import_from_xml_tree(xml_export, include_id=True)
     assert fd2.workflow_roles.get('_receiver') == role.id
+
+    # found by slug
+    fd2 = FormDef.import_from_xml_tree(xml_export, include_id=False)
+    assert fd2.workflow_roles.get('_receiver') == role.id
+
+    role.slug = 'something else'
+    role.store()
     fd2 = FormDef.import_from_xml_tree(xml_export, include_id=False)
     assert fd2.workflow_roles.get('_receiver') is None
 

tests/test_workflow_import.py

@@ -1,4 +1,5 @@
 import io
+import re
 import xml.etree.ElementTree as ET
 
 import pytest
@@ -191,17 +192,22 @@ def test_status_actions_named_existing_role(pub):
     commentable.parent = st1
 
     wf2 = assert_import_export_works(wf)
-    assert b'<item role_id="2">Test Role named existing role</item>' in ET.tostring(
-        indent(wf.export_to_xml())
+    assert re.findall(
+        '<item.*role_id="2".*>Test Role named existing role</item>',
+        ET.tostring(indent(wf.export_to_xml())).decode(),
     )
     assert wf2.possible_status[0].items[0].by == ['2']
 
     # check that it works even if the role_id is not set
     xml_export_orig = ET.tostring(export_to_indented_xml(wf))
-    xml_export = xml_export_orig.replace(
-        b'<item role_id="2">Test Role named existing role</item>',
-        b'<item>Test Role named existing role</item>',
-    )
+    xml_export = xml_export_orig.replace(b'role_id="2"', b'')
+    wf3 = Workflow.import_from_xml_tree(ET.parse(io.BytesIO(xml_export)))
+    assert wf3.possible_status[0].items[0].by == ['2']
+
+    # check that it works even if role_id and slug are not set
+    xml_export_orig = ET.tostring(export_to_indented_xml(wf))
+    xml_export = xml_export_orig.replace(b'role_id="2"', b'')
+    xml_export = xml_export_orig.replace(b'slug="test-role-named-existing-role"', b'')
     wf3 = Workflow.import_from_xml_tree(ET.parse(io.BytesIO(xml_export)))
     assert wf3.possible_status[0].items[0].by == ['2']
 
@@ -231,16 +237,16 @@ def test_status_actions_named_missing_role(pub):
 
     # check that role name has precedence over id
     xml_export_orig = ET.tostring(export_to_indented_xml(wf))
-    assert b'<item role_id="3">Test Role A</item>' in xml_export_orig
-    xml_export = xml_export_orig.replace(
-        b'<item role_id="3">Test Role A</item>', b'<item role_id="4">Test Role A</item>'
-    )
+    assert b'role_id="3"' in xml_export_orig
+    xml_export = xml_export_orig.replace(b'role_id="3"', b'role_id="4"').replace(b'slug="test-role-a"', b'')
     wf3 = Workflow.import_from_xml_tree(ET.parse(io.BytesIO(xml_export)))
     assert wf3.possible_status[0].items[0].by == ['3']
 
     # check that it creates a new role if there's no match on id and name
-    xml_export = xml_export_orig.replace(
-        b'<item role_id="3">Test Role A</item>', b'<item role_id="999">foobar</item>'
+    xml_export = (
+        xml_export_orig.replace(b'role_id="3"', b'role_id="999"')
+        .replace(b'slug="test-role-a"', b'')
+        .replace(b'Test Role A', b'foobar')
     )
     nb_roles = pub.role_class.count()
     wf3 = Workflow.import_from_xml_tree(ET.parse(io.BytesIO(xml_export)))
@@ -249,9 +255,7 @@ def test_status_actions_named_missing_role(pub):
     # check that it doesn't fallback on the id if there's no match on the
     # name
     nb_roles = pub.role_class.count()
-    xml_export = xml_export_orig.replace(
-        b'<item role_id="3">Test Role A</item>', b'<item role_id="3">Test Role C</item>'
-    )
+    xml_export = xml_export_orig.replace(b'Test Role A', b'Test Role C').replace(b'slug="test-role-a"', b'')
     wf3 = Workflow.import_from_xml_tree(ET.parse(io.BytesIO(xml_export)))
     assert wf3.possible_status[0].items[0].by != ['3']
     assert pub.role_class.count() == nb_roles + 1

wcs/formdef.py

@@ -1185,6 +1185,20 @@ class FormDef(StorableObject):
         for field in self.fields or []:
             fields.append(field.export_to_xml(charset=charset, include_id=include_id))
 
+        from wcs.workflows import get_role_name_and_slug
+
+        def add_role_element(roles_root, role_id):
+            if not role_id:
+                return
+            role_name, role_slug = get_role_name_and_slug(role_id)
+            sub = ET.SubElement(roles_root, 'role')
+            if role_slug:
+                sub.attrib['slug'] = role_slug
+            if include_id:
+                sub.attrib['role_id'] = str(role_id)
+            sub.text = role_name
+            return sub
+
         roles_elements = [
             ('roles', 'user-roles'),
             ('backoffice_submission_roles', 'backoffice-submission-roles'),
@@ -1194,39 +1208,14 @@ class FormDef(StorableObject):
                 continue
             roles = ET.SubElement(root, node_name)
             for role_id in getattr(self, attr_name):
-                if role_id is None:
-                    continue
-                role_id = str(role_id)
-                if role_id.startswith('_') or role_id == 'logged-users':
-                    role = force_text(role_id, charset)
-                else:
-                    try:
-                        role = force_text(get_publisher().role_class.get(role_id).name, charset)
-                    except KeyError:
-                        role = force_text(role_id, charset)
-                sub = ET.SubElement(roles, 'role')
-                if include_id:
-                    sub.attrib['role_id'] = role_id
-                sub.text = role
+                add_role_element(roles, role_id)
 
         if self.workflow_roles:
             roles = ET.SubElement(root, 'roles')
             for role_key, role_id in self.workflow_roles.items():
-                if role_id is None:
-                    continue
-                role_id = str(role_id)
-                if role_id.startswith('_') or role_id == 'logged-users':
-                    role = force_text(role_id, charset)
-                else:
-                    try:
-                        role = force_text(get_publisher().role_class.get(role_id).name, charset)
-                    except KeyError:
-                        role = force_text(role_id, charset)
-                sub = ET.SubElement(roles, 'role')
-                sub.attrib['role_key'] = role_key
-                if include_id:
-                    sub.attrib['role_id'] = role_id
-                sub.text = role
+                sub = add_role_element(roles, role_id)
+                if sub is not None:
+                    sub.attrib['role_key'] = role_key
 
         options = ET.SubElement(root, 'options')
         for option in sorted(self.workflow_options or []):
@@ -1425,19 +1414,19 @@ class FormDef(StorableObject):
             role_id = None
             value = xml_node_text(role_node)
             if value.startswith('_') or value == 'logged-users':
-                role_id = value
-            elif include_id:
+                return value
+
+            if include_id:
                 role_id = role_node.attrib.get('role_id')
-                if role_id and not get_publisher().role_class.get(role_id, ignore_errors=True):
-                    role_id = None
+                if role_id and get_publisher().role_class.get(role_id, ignore_errors=True):
+                    return role_id
 
-            if not role_id:
-                for role in get_publisher().role_class.select(ignore_errors=True):
-                    if role.name == value:
-                        role_id = role.id
-                        break
+            role_slug = role_node.attrib.get('slug')
+            role = get_publisher().role_class.resolve(uuid=None, slug=role_slug, name=value)
+            if role:
+                return role.id
 
-            return role_id
+            return None
 
         roles_elements = [
             ('roles', 'user-roles'),

wcs/roles.py

@@ -146,19 +146,20 @@ class Role(StorableObject):
         return role
 
     @classmethod
-    def resolve(cls, uuid, slug=None, name=None):
-        try:
-            return cls.get_on_index(uuid, 'uuid')
-        except KeyError:
-            pass
-        try:
-            return cls.get(uuid)
-        except KeyError:
-            pass
-        try:
-            return cls.get_on_index(uuid, 'slug')
-        except KeyError:
-            pass
+    def resolve(cls, uuid=None, slug=None, name=None):
+        if uuid:
+            try:
+                return cls.get_on_index(uuid, 'uuid')
+            except KeyError:
+                pass
+            try:
+                return cls.get(uuid)
+            except KeyError:
+                pass
+            try:
+                return cls.get_on_index(uuid, 'slug')
+            except KeyError:
+                pass
         if slug:
             try:
                 return cls.get_on_index(slug, 'slug')

wcs/wf/dispatch.py

@@ -21,7 +21,7 @@ from quixote import get_publisher
 from quixote.html import htmltext
 
 from wcs.roles import get_user_roles
-from wcs.workflows import WorkflowStatusItem, XmlSerialisable, get_role_name, register_item_class
+from wcs.workflows import WorkflowStatusItem, XmlSerialisable, get_role_name_and_slug, register_item_class
 
 from ..qommon import _
 from ..qommon.form import (
@@ -207,13 +207,14 @@ class DispatchWorkflowStatusItem(WorkflowStatusItem):
             )
 
     def get_role_id_parameter_view_value(self):
-        return get_role_name(self.role_id)
+        return get_role_name_and_slug(self.role_id)[0]
 
     def get_rules_parameter_view_value(self):
         result = []
         for rule in self.rules or []:
             result.append(
-                htmltext('<li>%s → %s</li>') % (rule.get('value'), get_role_name(rule.get('role_id')))
+                htmltext('<li>%s → %s</li>')
+                % (rule.get('value'), get_role_name_and_slug(rule.get('role_id'))[0])
             )
         return htmltext('<ul class="rules">%s</ul>') % htmltext('').join(result)
 

wcs/workflows.py

@@ -1213,10 +1213,12 @@ class XmlSerialisable:
             if role_id is None:
                 continue
             role_id = str(role_id)
-            role = get_role_name(role_id, charset)
+            role_name, role_slug = get_role_name_and_slug(role_id)
             sub = ET.SubElement(el, 'item')
+            if role_slug:
+                sub.attrib['slug'] = role_slug
             sub.attrib['role_id'] = role_id
-            sub.text = role
+            sub.text = role_name
 
     def _roles_init_with_xml(self, attribute, elem, charset, include_id=False, snapshot=False):
         if elem is None:
@@ -1233,11 +1235,13 @@ class XmlSerialisable:
         if not hasattr(self, attribute) or not getattr(self, attribute):
             return
         role_id = str(getattr(self, attribute))
-        role = get_role_name(role_id, charset)
+        role_name, role_slug = get_role_name_and_slug(role_id)
         sub = ET.SubElement(item, attribute)
+        if role_slug:
+            sub.attrib['slug'] = role_slug
         if include_id:
             sub.attrib['role_id'] = role_id
-        sub.text = role
+        sub.text = role_name
 
     def _get_role_id_from_xml(self, elem, charset, include_id=False, snapshot=False):
         if elem is None:
@@ -1257,10 +1261,11 @@ class XmlSerialisable:
             if WorkflowStatusItem.get_expression(role_id)['type'] in ('python', 'template'):
                 return role_id
 
-        # if not using id, look up on the name
-        for role in get_publisher().role_class.select(ignore_errors=True):
-            if role.name == value:
-                return role.id
+        # if not using id, look up on the slug or name
+        role_slug = elem.attrib.get('slug')
+        role = get_publisher().role_class.resolve(uuid=None, slug=role_slug, name=value)
+        if role:
+            return role.id
 
         # if a computed value is possible and value looks like
         # an expression, use it
@@ -2705,14 +2710,15 @@ def get_role_translation_label(workflow, role_id):
             return
 
 
-def get_role_name(role_id, charset=None):
+def get_role_name_and_slug(role_id):
     role_id = str(role_id)
     if role_id.startswith('_') or role_id == 'logged-users':
-        return force_text(role_id, charset)
+        return (str(role_id), None)
     try:
-        return force_text(get_publisher().role_class.get(role_id).name, charset)
+        role = get_publisher().role_class.get(role_id)
+        return (role.name, role.slug)
     except KeyError:
-        return force_text(role_id, charset)
+        return (str(role_id), None)
 
 
 def render_list_of_roles(workflow, roles):