pages: sub_slug can contain only slug part, without regex (#50000)

combo/apps/wcs/__init__.py

@@ -53,7 +53,8 @@ class AppConfig(django.apps.AppConfig):
         for key, service in wcs_services.items():
             card_models = get_wcs_json(service, 'api/cards/@list')
             for card in card_models.get('data') or []:
-                matching_pages = ([p for p in pages_with_sub_slug if '<%s_id>' % card['id'] in p.sub_slug])
+                card_id = '%s_id' % card['id']
+                matching_pages = ([p for p in pages_with_sub_slug if '<%s>' % card_id in p.sub_slug or p.sub_slug == card_id])
                 if not matching_pages:
                     continue
                 card_page = matching_pages[0]

combo/apps/wcs/models.py

@@ -812,7 +812,8 @@ class WcsCardsCell(CardMixin, WcsBlurpMixin, CellBase):
         extra_context['title'] = self.cached_title
 
         pages_with_sub_slug = Page.objects.exclude(sub_slug='')
-        matching_pages = ([p for p in pages_with_sub_slug if '<%s_id>' % self.carddef_reference.split(':')[1] in p.sub_slug])
+        card_id = '%s_id' % self.carddef_reference.split(':')[1]
+        matching_pages = ([p for p in pages_with_sub_slug if '<%s>' % card_id in p.sub_slug or p.sub_slug == card_id])
         if matching_pages:
             card_page = matching_pages[0]
             extra_context['card_page_base_url'] = card_page.get_online_url()

combo/data/models.py

@@ -99,6 +99,10 @@ def django_template_validator(value):
 
 def format_sub_slug(sub_slug):
     mapping = {}
+
+    if 'P<' not in sub_slug:
+        # simple sub_slug without regex
+        sub_slug = '(?P<%s>[a-z0-9]+)' % sub_slug
     # search all named-groups in sub_slug
     for i, m in enumerate(re.finditer(r'P<[\w_-]+>', sub_slug)):
         # extract original name

combo/manager/forms.py

@@ -129,6 +129,8 @@ class PageEditSlugForm(forms.ModelForm):
 
     def clean_sub_slug(self):
         value = self.cleaned_data.get('sub_slug')
+        if not value:
+            return value
 
         try:
             compile_sub_slug(value)

combo/profile/__init__.py

@@ -45,7 +45,9 @@ class AppConfig(django.apps.AppConfig):
         from combo.data.models import Page
         from django.conf import settings
 
-        user_page = Page.objects.filter(sub_slug__contains='<name_id>').first()
+        user_page = Page.objects.filter(sub_slug='name_id').first()
+        if not user_page:
+            user_page = Page.objects.filter(sub_slug__contains='<name_id>').first()
         if not user_page:
             return
         user_page_base_url = user_page.get_online_url()

tests/test_manager.py

@@ -423,12 +423,31 @@ def test_page_edit_sub_slug(app, admin_user):
     assert page.slug == 'foobar'
     assert page.sub_slug == '(?P<card-foo_id>[0-9]+)'
 
+    resp = app.get('/manage/pages/%s/slug' % page.pk)
+    resp.form['sub_slug'].value = 'card-foo_id'
+    resp = resp.form.submit().follow()
+    page.refresh_from_db()
+    assert page.slug == 'foobar'
+    assert page.sub_slug == 'card-foo_id'
+
     # bad regexp
-    resp = resp.click(href='.*/slug')
+    resp = app.get('/manage/pages/%s/slug' % page.pk)
     resp.form['sub_slug'].value = '(?P< bad group name with spaces >[0-9]+)'
     resp = resp.form.submit()
     assert resp.context['form'].errors['sub_slug'] == ['Bad Regular expression.']
 
+    resp = app.get('/manage/pages/%s/slug' % page.pk)
+    resp.form['sub_slug'].value = ' bad group name with spaces '
+    resp = resp.form.submit()
+    assert resp.context['form'].errors['sub_slug'] == ['Bad Regular expression.']
+
+    # clear sub_slug
+    resp.form['sub_slug'].value = ''
+    resp = resp.form.submit().follow()
+    page.refresh_from_db()
+    assert page.slug == 'foobar'
+    assert page.sub_slug == ''
+
 
 def test_page_edit_picture(app, admin_user):
     Page.objects.all().delete()

tests/test_public.py

@@ -972,22 +972,21 @@ def test_sub_slug(app, john_doe, jane_doe):
         assert resp.text == 'AAwhateverBBhttp://testserver/users/whatever/blah/CC'
 
         # custom behaviour for <user_id>, it will add the user to context
-        page2.sub_slug = '(?P<user_id>[0-9]+)'
-        page2.save()
+        for sub_slug in ['(?P<user_id>[0-9a-z]+)', 'user_id']:
+            page2.sub_slug = sub_slug
+            page2.save()
 
-        cell.template_string = 'XX{{ selected_user.username }}YY'
-        cell.save()
+            cell.template_string = 'XX{{ selected_user.username }}YY'
+            cell.save()
 
-        resp = app.get('/users/%s/' % john_doe.id, status=200)
-        assert 'XXjohn.doeYY' in resp.text
+            resp = app.get('/users/%s/' % john_doe.id, status=200)
+            assert 'XXjohn.doeYY' in resp.text
 
-        # bad user id => no selected_user
-        page2.sub_slug = '(?P<user_id>[0-9a-z]+)'
-        page2.save()
-        resp = app.get('/users/9999999/', status=200)
-        assert 'XXYY' in resp.text
-        resp = app.get('/users/abc/', status=200)
-        assert 'XXYY' in resp.text
+            # bad user id => no selected_user
+            resp = app.get('/users/9999999/', status=200)
+            assert 'XXYY' in resp.text
+            resp = app.get('/users/abc/', status=200)
+            assert 'XXYY' in resp.text
 
         # custom behaviour for <name_id>, it will add the SAML user to context
         with mock.patch('combo.profile.utils.UserSAMLIdentifier') as user_saml:
@@ -1012,10 +1011,13 @@ def test_sub_slug(app, john_doe, jane_doe):
             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'
+    page3 = Page.objects.create(title='Card Foo', slug='foo', template_name='standard')
+    for sub_slug in ['(?P<card-foo-bar_id>[0-9]+)', 'card-foo-bar_id']:
+        page3.sub_slug = sub_slug
+        page3.save()
+        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):

tests/test_search.py

@@ -682,16 +682,19 @@ def test_wcs_search_engines(settings, app):
         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<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: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/'
-            '{% if search_service.selected_custom_view %}{{ search_service.selected_custom_view }}{% endif %}'
-            '?{% if not search_service.without_user %}NameID={{ user_nameid }}&{% endif %}q=%(q)s')
-        assert card_engine['hit_url_template'] == '/bar/{{ id }}'
+        page = Page.objects.create(slug='bar', title='Bar')
+        for sub_slug in ['(?P<card-bar_id>[a-z0-9]+)', 'card-bar_id']:
+            page.sub_slug = sub_slug
+            page.save()
+            search_engines = engines.get_engines()
+            assert len([x for x in search_engines.keys() if x.startswith('cards:')]) == 1
+            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/'
+                '{% if search_service.selected_custom_view %}{{ search_service.selected_custom_view }}{% endif %}'
+                '?{% if not search_service.without_user %}NameID={{ user_nameid }}&{% endif %}q=%(q)s')
+            assert card_engine['hit_url_template'] == '/bar/{{ id }}'
 
 
 def test_wcs_errors(settings, app):
@@ -718,7 +721,7 @@ def test_wcs_errors(settings, app):
 def test_wcs_add_search_engines(mock_wcs, settings, app, admin_user):
     settings.KNOWN_SERVICES = {'wcs': {'default': {'title': 'test', 'url': 'http://127.0.0.1:8999/'}}}
     settings.TEMPLATE_VARS['is_portal_agent'] = True
-    Page.objects.create(slug='bar', title='Bar', sub_slug='(?P<card-bar_id>[a-z0-9]+)')
+    Page.objects.create(slug='bar', title='Bar', sub_slug='card-bar_id')
     mock_wcs.return_value = {'data': [{'id': 'card-bar', 'text': 'Card Bar'}]}
 
     page = Page.objects.create(title='One', slug='one', template_name='standard')
@@ -857,7 +860,11 @@ def test_profile_search_engines(settings, app):
     search_engines = engines.get_engines()
     assert 'users' not in search_engines.keys()
 
-    page = Page(slug='users', title='Users', sub_slug='(?P<name_id>[a-z0-9]+)')
+    page = Page.objects.create(slug='users', title='Users', sub_slug='(?P<name_id>[a-z0-9]+)')
+    search_engines = engines.get_engines()
+    assert 'users' in search_engines.keys()
+
+    page.sub_slug = 'name_id'
     page.save()
     search_engines = engines.get_engines()
     assert 'users' in search_engines.keys()

tests/test_wcs.py

@@ -1191,7 +1191,7 @@ def test_cards_cell_render(mock_send, context):
     assert '<a href="http://127.0.0.1:8999/backoffice/data/card_model_1/13/"><span class="card-title">cc</span></a>' in result
 
     # create a page with the correct subslug
-    Page.objects.create(slug='foo', title='Foo', sub_slug='(?P<card_model_1_id>[a-z0-9]+)')
+    page = Page.objects.create(slug='foo', title='Foo', sub_slug='(?P<card_model_1_id>[a-z0-9]+)')
 
     result = cell.render(context)
     assert '<h2>Card Model 1</h2>' in result
@@ -1201,6 +1201,8 @@ def test_cards_cell_render(mock_send, context):
 
     cell.carddef_reference = u'default:card_model_1:foo'
     cell.save()
+    page.sub_slug = 'card_model_1_id'
+    page.save()
 
     result = cell.render(context)
     assert '<h2>Card Model 1 - bar</h2>' in result