pages: sub_slug, regexp & '-' in group name (#47099)

combo/data/models.py

@@ -97,6 +97,31 @@ def django_template_validator(value):
         raise ValidationError(_('syntax error: %s') % e)
 
 
+def extract_context_from_sub_slug(sub_slug, sub_url):
+    mapping = {}
+    # search all named-groups in sub_slug
+    for i, m in enumerate(re.finditer(r'P<[\w_-]+>', sub_slug)):
+        # extract original name
+        original_group = m.group()[2:-1]
+        # rename it to remove all bad characters
+        new_group = 'g%i' % i
+        # update sub_slug
+        sub_slug = sub_slug[:m.start()] + 'P<%s>' % new_group + sub_slug[m.end():]
+        # keep a mapping
+        mapping[new_group] = original_group
+
+    # match url
+    match = re.match('^' + sub_slug + '$', sub_url)
+    if match is None:
+        return
+
+    # return a dict with original group names
+    context = {mapping[k]: v for k, v in match.groupdict().items()}
+    # format also key to replace - by _
+    context.update({mapping[k].replace('-', '_'): v for k, v in match.groupdict().items()})
+    return context
+
+
 class Placeholder(object):
     def __init__(self, key, name=None, acquired=False, optional=False,
                  render=True, cell=None, force_synchronous=False):

combo/public/views.py

@@ -48,8 +48,10 @@ else:
     get_idps = lambda: []
 
 from combo.apps.assets.models import Asset
-from combo.data.models import (CellBase, PostException, Page, Redirect,
-        ParentContentCell, TextCell, PageSnapshot)
+from combo.data.models import (
+    CellBase, PostException, Page, Redirect,
+    ParentContentCell, TextCell, PageSnapshot,
+    extract_context_from_sub_slug)
 from combo.profile.models import Profile
 from combo.profile.utils import get_user_from_name_id
 from combo.apps.search.models import SearchCell
@@ -490,11 +492,11 @@ def page(request):
                 # a sub slug is expected but was not found; redirect to parent
                 # page as a mitigation.
                 return HttpResponseRedirect('..')
-            match = re.match('^' + page.sub_slug + '$', parts[i+1])
-            if match is None:
+            extra = extract_context_from_sub_slug(page.sub_slug, parts[i+1])
+            if extra is None:
                 page = None
                 break
-            request.extra_context_data.update(match.groupdict())
+            request.extra_context_data.update(extra)
             parts = parts[:i+1] + parts[i+2:]  # skip variable component
         i += 1
         hierarchy_ids.append(page.id)

tests/test_public.py

@@ -925,6 +925,7 @@ def test_redirects(app):
     assert urlparse.urlparse(app.get('/second/third/', status=302).location).path == '/third2/'
     assert urlparse.urlparse(app.get('/second2/third2/', status=302).location).path == '/third2/'
 
+
 def test_sub_slug(app, john_doe, jane_doe):
     Page.objects.all().delete()
     page = Page(title='Home', slug='index', template_name='standard')
@@ -938,7 +939,7 @@ def test_sub_slug(app, john_doe, jane_doe):
 
     # without passing sub slug
     assert app.get('/users/', status=302).location in ('..', 'http://testserver/')
-        # (result vary between django versions)
+    # (result vary between django versions)
 
     # json cell so we can display the parameter value
     cell = JsonCell(page=page2, url='http://example.net', order=0, placeholder='content')
@@ -1010,6 +1011,13 @@ def test_sub_slug(app, john_doe, jane_doe):
             resp = app.get('/users/foo/', status=200)
             assert 'XXYY' in resp.text
 
+    # sub_slug can contain '-' (if card slug for example)
+    page3 = Page.objects.create(title='Card Foo', slug='foo', sub_slug='(?P<card-foo-bar_id>[0-9]+)', template_name='standard')
+    resp = app.get('/foo/42/', status=200)
+    assert resp.context['card-foo-bar_id'] == '42'
+    assert resp.context['card_foo_bar_id'] == '42'
+
+
 def test_cell_slugs(app):
     Page.objects.all().delete()
     page = Page(title='Home', slug='index', template_name='standard')

tests/test_search.py

@@ -670,17 +670,17 @@ def test_wcs_search_engines(settings, app):
         assert len([x for x in search_engines.keys() if x.startswith('cards:')]) == 0
 
         # card model found, but related page does not exist
-        mock_wcs.return_value = {'data': [{'id': 'bar', 'text': 'Bar'}]}
+        mock_wcs.return_value = {'data': [{'id': 'card-bar', 'text': 'Card Bar'}]}
         search_engines = engines.get_engines()
         assert len([x for x in search_engines.keys() if x.startswith('cards:')]) == 0
 
         # related page exists
-        Page.objects.create(slug='bar', title='Bar', sub_slug='(?P<bar_id>[a-z0-9]+)')
+        Page.objects.create(slug='bar', title='Bar', sub_slug='(?P<card-bar_id>[a-z0-9]+)')
         search_engines = engines.get_engines()
         assert len([x for x in search_engines.keys() if x.startswith('cards:')]) == 1
-        assert 'cards:c21f969b:bar' in search_engines.keys()
-        card_engine = search_engines['cards:c21f969b:bar']
-        assert card_engine['url'] == 'http://127.0.0.1:8999/api/cards/bar/list?NameID={{ user_nameid }}&q=%(q)s'
+        assert 'cards:c21f969b:card-bar' in search_engines.keys()
+        card_engine = search_engines['cards:c21f969b:card-bar']
+        assert card_engine['url'] == 'http://127.0.0.1:8999/api/cards/card-bar/list?NameID={{ user_nameid }}&q=%(q)s'
         assert card_engine['hit_url_template'] == '/bar/{{ id }}'