From eec98248f640a68110dc4a2a010ee8eacefb568c Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Thu, 29 Apr 2021 08:52:59 +0200
Subject: [PATCH] wcs: use category_slugs filter in wcs calls (#53376)

---
 combo/apps/wcs/models.py |  62 +++++----------
 tests/test_wcs.py        | 164 +++++++++++++++++++++++++--------------
 2 files changed, 128 insertions(+), 98 deletions(-)

diff --git a/combo/apps/wcs/models.py b/combo/apps/wcs/models.py
index fcccd0cd..b542e617 100644
--- a/combo/apps/wcs/models.py
+++ b/combo/apps/wcs/models.py
@@ -430,8 +430,25 @@ class CategoriesValidityMixin(object):
         self.mark_as_valid()
 
 
+class CategoriesFilteringMixin(object):
+    def get_api_url_for_site(self, context, wcs_slug):
+        url = self.get_api_url(context)
+
+        if self.categories and self.categories.get('data') or []:
+            categories_by_site = collections.defaultdict(list)
+            for category in self.categories['data']:
+                key, slug = category.split(':')
+                categories_by_site[key].append(slug)
+            if not categories_by_site.get(wcs_slug):
+                return None
+            separator = '?' if '?' not in url else '&'
+            url += '%scategory_slugs=%s' % (separator, ','.join(categories_by_site.get(wcs_slug)))
+
+        return url
+
+
 @register_cell_class
-class WcsCurrentFormsCell(WcsUserDataBaseCell, CategoriesValidityMixin):
+class WcsCurrentFormsCell(CategoriesValidityMixin, CategoriesFilteringMixin, WcsUserDataBaseCell):
     variable_name = 'user_forms'
 
     categories = JSONField(_('Categories'), blank=True)
@@ -490,19 +507,12 @@ class WcsCurrentFormsCell(WcsUserDataBaseCell, CategoriesValidityMixin):
     def get_cell_extra_context(self, context):
         context = super(WcsCurrentFormsCell, self).get_cell_extra_context(context)
 
-        categories_filter = {}
-        if self.categories:
-            for category in self.categories.get('data', []):
-                categories_filter[tuple(category.split(':'))] = True
-
         for wcs_site in context['user_forms']:
             if not context['user_forms'].get(wcs_site):
                 continue
             if not context['user_forms'][wcs_site].get('data'):
                 continue
             forms = context['user_forms'][wcs_site]['data']
-            if categories_filter:
-                forms = [x for x in forms if (wcs_site, x.get('category_slug')) in categories_filter]
             if self.current_forms and self.done_forms:
                 pass  # keep them all
             elif self.current_forms:
@@ -528,7 +538,7 @@ class WcsCurrentFormsCell(WcsUserDataBaseCell, CategoriesValidityMixin):
 
 
 @register_cell_class
-class WcsCurrentDraftsCell(WcsUserDataBaseCell, CategoriesValidityMixin):
+class WcsCurrentDraftsCell(CategoriesValidityMixin, CategoriesFilteringMixin, WcsUserDataBaseCell):
     variable_name = 'current_drafts'
     template_name = 'combo/wcs/current_drafts.html'
 
@@ -553,19 +563,12 @@ class WcsCurrentDraftsCell(WcsUserDataBaseCell, CategoriesValidityMixin):
     def get_cell_extra_context(self, context):
         context = super(WcsCurrentDraftsCell, self).get_cell_extra_context(context)
 
-        categories_filter = {}
-        if self.categories:
-            for category in self.categories.get('data', []):
-                categories_filter[tuple(category.split(':'))] = True
-
         for wcs_site in context['current_drafts']:
             if not context['current_drafts'].get(wcs_site):
                 continue
             if not context['current_drafts'][wcs_site].get('data'):
                 continue
             forms = context['current_drafts'][wcs_site]['data']
-            if categories_filter:
-                forms = [x for x in forms if (wcs_site, x.get('category_slug')) in categories_filter]
             context['current_drafts'][wcs_site]['data'] = forms  # put it back
 
         # regroup all forms in a flat list
@@ -702,7 +705,7 @@ class WcsFormsOfCategoryCell(WcsCommonCategoryCell, WcsBlurpMixin):
 
 
 @register_cell_class
-class WcsCareFormsCell(WcsDataBaseCell, CategoriesValidityMixin):
+class WcsCareFormsCell(CategoriesValidityMixin, CategoriesFilteringMixin, WcsDataBaseCell):
     categories = JSONField(_('Categories'), blank=True)
 
     api_url = '/api/forms/?limit=10'
@@ -719,20 +722,6 @@ class WcsCareFormsCell(WcsDataBaseCell, CategoriesValidityMixin):
 
         return WcsCareFormsCellForm
 
-    def get_api_url_for_site(self, context, wcs_slug):
-        url = self.get_api_url(context)
-
-        if self.categories and self.categories.get('data') or []:
-            categories_by_site = collections.defaultdict(list)
-            for category in self.categories['data']:
-                key, slug = category.split(':')
-                categories_by_site[key].append(slug)
-            if not categories_by_site.get(wcs_slug):
-                return None
-            url += '&category_slugs=%s' % ','.join(categories_by_site.get(wcs_slug))
-
-        return url
-
     def get_cell_extra_context(self, context):
         context = super().get_cell_extra_context(context)
 
@@ -1019,7 +1008,7 @@ class TrackingCodeInputCell(CellBase):
 
 
 @register_cell_class
-class BackofficeSubmissionCell(WcsDataBaseCell, CategoriesValidityMixin):
+class BackofficeSubmissionCell(CategoriesValidityMixin, CategoriesFilteringMixin, WcsDataBaseCell):
     api_url = '/api/formdefs/?backoffice-submission=on'
     variable_name = 'all_formdefs'
     template_name = 'combo/wcs/backoffice_submission.html'
@@ -1047,19 +1036,10 @@ class BackofficeSubmissionCell(WcsDataBaseCell, CategoriesValidityMixin):
         all_formdefs = context.pop('all_formdefs')
         formdefs = {}
 
-        categories_filter = {}
-        if self.categories:
-            for category in self.categories.get('data', []):
-                categories_filter[tuple(category.split(':'))] = True
-
         # add a fake category where it's missing
         for key, site_formdefs in all_formdefs.items():
             if not site_formdefs.get('data'):
                 continue
-            if categories_filter:
-                site_formdefs['data'] = [
-                    x for x in site_formdefs['data'] if (key, x.get('category_slug')) in categories_filter
-                ]
             formdefs[key] = site_formdefs
             for formdef in site_formdefs['data']:
                 if 'category' not in formdef:
diff --git a/tests/test_wcs.py b/tests/test_wcs.py
index 95050904..85e542e7 100644
--- a/tests/test_wcs.py
+++ b/tests/test_wcs.py
@@ -516,54 +516,76 @@ def test_current_forms_cell_render(mock_send, context):
     assert 'http://127.0.0.2:8999/form/1/' not in result  # wip
     assert 'http://127.0.0.2:8999/form/4/' in result  # done
 
-    # limit to a category
-    cell.categories = {'data': ['default:test-3']}
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['forms']) == 0
-    cell.categories = {'data': ['default:test-9']}
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['forms']) == 1
-
-    # check both category limit and all forms
-    cell.current_forms = True
-    cell.done_forms = True
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['forms']) == 2
-
-    # check both category limit and no forms
-    cell.current_forms = False
-    cell.done_forms = False
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['forms']) == 0
+    # limit to categories
+    cell.categories = {'data': ['default:test-3', 'other:test-6']}
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
+    assert requests_get.call_args_list[0][0][0] == '/api/user/forms?limit=100&sort=desc&category_slugs=test-3'
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+    assert requests_get.call_args_list[1][0][0] == '/api/user/forms?limit=100&sort=desc&category_slugs=test-6'
+    assert requests_get.call_args_list[1][1]['remote_service']['url'] == 'http://127.0.0.2:8999/'
 
     # check include drafts
+    cell.categories = None
     cell.include_drafts = False
     with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
         mock_json = mock.Mock(status_code=200)
         requests_get.return_value = mock_json
         cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
     assert requests_get.call_args_list[0][0][0] == '/api/user/forms?limit=100&sort=desc'
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+    assert requests_get.call_args_list[1][0][0] == '/api/user/forms?limit=100&sort=desc'
+    assert requests_get.call_args_list[1][1]['remote_service']['url'] == 'http://127.0.0.2:8999/'
 
     cell.include_drafts = True
     with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
         mock_json = mock.Mock(status_code=200)
         requests_get.return_value = mock_json
         cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
     assert requests_get.call_args_list[0][0][0] == '/api/user/forms?limit=100&sort=desc&include-drafts=on'
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+    assert requests_get.call_args_list[1][0][0] == '/api/user/forms?limit=100&sort=desc&include-drafts=on'
+    assert requests_get.call_args_list[1][1]['remote_service']['url'] == 'http://127.0.0.2:8999/'
 
-    # check empty messages
     cell.categories = {'data': ['default:test-3']}
-    cell.current_forms = True
-    cell.done_forms = False
-    cell.include_drafts = False
-    result = cell.render(context)
-    assert 'There are no current forms.' in result
-    cell.done_forms = True
-    result = cell.render(context)
-    assert 'There are no forms.' in result
-    cell.current_forms = False
-    result = cell.render(context)
-    assert 'There are no done forms' in result
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 1
+    assert (
+        requests_get.call_args_list[0][0][0]
+        == '/api/user/forms?limit=100&sort=desc&include-drafts=on&category_slugs=test-3'
+    )
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+
+    cell.categories = {'data': []}
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
+
+    # check empty messages
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.current_forms = True
+        cell.done_forms = False
+        cell.include_drafts = False
+        result = cell.render(context)
+        assert 'There are no current forms.' in result
+        cell.done_forms = True
+        result = cell.render(context)
+        assert 'There are no forms.' in result
+        cell.current_forms = False
+        result = cell.render(context)
+        assert 'There are no done forms' in result
 
 
 @mock.patch('combo.apps.wcs.utils.requests.send', side_effect=mocked_requests_send)
@@ -1071,13 +1093,33 @@ def test_current_drafts_cell_render_logged_in(mock_send, context):
     assert len([x for x in extra_context['drafts'] if x['site_slug'] == 'default']) == 4
     assert len([x for x in extra_context['drafts'] if x['site_slug'] == 'other']) == 4
 
-    # limit to a category
+    # limit to categories
+    cell.categories = {'data': ['default:test-3', 'other:test-6']}
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
+    assert requests_get.call_args_list[0][0][0] == '/api/user/drafts?category_slugs=test-3'
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+    assert requests_get.call_args_list[1][0][0] == '/api/user/drafts?category_slugs=test-6'
+    assert requests_get.call_args_list[1][1]['remote_service']['url'] == 'http://127.0.0.2:8999/'
+
     cell.categories = {'data': ['default:test-3']}
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['drafts']) == 0
-    cell.categories = {'data': ['default:test-9']}
-    extra_context = cell.get_cell_extra_context(context)
-    assert len(extra_context['drafts']) == 2
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 1
+    assert requests_get.call_args_list[0][0][0] == '/api/user/drafts?category_slugs=test-3'
+    assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+
+    cell.categories = {'data': []}
+    with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
+        mock_json = mock.Mock(status_code=200)
+        requests_get.return_value = mock_json
+        cell.get_cell_extra_context(context)
+    assert len(requests_get.call_args_list) == 2
 
     # check empty message
     with mock.patch('combo.apps.wcs.models.requests.get') as requests_get:
@@ -1893,9 +1935,7 @@ def test_wcs_search_engines(app):
 def test_backoffice_submission_cell_render(context):
     page = Page(title='xxx', slug='test_backoffice_submission_cell_render', template_name='standard')
     page.save()
-    cell = BackofficeSubmissionCell(page=page, placeholder='content', order=0)
-    cell.wcs_site = 'default'
-    cell.save()
+    cell = BackofficeSubmissionCell.objects.create(page=page, placeholder='content', order=0)
 
     context['synchronous'] = True  # to get fresh content
 
@@ -1922,30 +1962,40 @@ def test_backoffice_submission_cell_render(context):
         )
         result = cell.render(context)
         assert '/backoffice/submission/a-private-form/' in result
-        assert list(context['all_formdefs'].keys()) == ['default']
+        assert list(context['all_formdefs'].keys()) == ['default', 'other']
         assert 'h2' in result
 
-        # limit to a category
-        cell.categories = {'data': ['default:test-3']}
+        # limit to categories
+        requests_get.reset_mock()
+        cell.categories = {'data': ['default:test-3', 'other:test-6']}
 
         result = cell.render(context)
-        assert '/backoffice/submission/a-private-form/' not in result
+        assert len(requests_get.call_args_list) == 2
+        assert (
+            requests_get.call_args_list[0][0][0]
+            == '/api/formdefs/?backoffice-submission=on&category_slugs=test-3'
+        )
+        assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+        assert (
+            requests_get.call_args_list[1][0][0]
+            == '/api/formdefs/?backoffice-submission=on&category_slugs=test-6'
+        )
+        assert requests_get.call_args_list[1][1]['remote_service']['url'] == 'http://127.0.0.2:8999/'
 
-        requests_get.return_value = MockedRequestResponse(
-            content=json.dumps(
-                {
-                    'data': [
-                        {
-                            'backoffice_submission_url': '/backoffice/submission/a-private-form/',
-                            'title': 'Foo',
-                            'category_slug': 'test-3',
-                        }
-                    ]
-                }
-            )
+        requests_get.reset_mock()
+        cell.categories = {'data': ['default:test-3']}
+        result = cell.render(context)
+        assert len(requests_get.call_args_list) == 1
+        assert (
+            requests_get.call_args_list[0][0][0]
+            == '/api/formdefs/?backoffice-submission=on&category_slugs=test-3'
         )
+        assert requests_get.call_args_list[0][1]['remote_service']['url'] == 'http://127.0.0.1:8999/'
+
+        requests_get.reset_mock()
+        cell.categories = {'data': []}
         result = cell.render(context)
-        assert '/backoffice/submission/a-private-form/' in result
+        assert len(requests_get.call_args_list) == 2
 
 
 @mock.patch('combo.apps.wcs.utils.requests.send', side_effect=mocked_requests_send)
-- 
2.30.2

