From 1ab9684edb006cff5aabca97079dd159b567834a Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Laur=C3=A9line=20Gu=C3=A9rin?= <zebuline@entrouvert.com>
Date: Mon, 6 Apr 2020 09:07:41 +0200
Subject: [PATCH 1/2] search: new manager selection SearchCell engines (#40224)

---
 combo/apps/search/forms.py                    |  11 +-
 combo/apps/search/manager_views.py            |  75 ++++++++++
 combo/apps/search/models.py                   |  11 +-
 .../combo/manager/search-cell-form.html       |  44 +++++-
 combo/apps/search/urls.py                     |  19 ++-
 tests/test_search.py                          | 139 +++++++++++-------
 6 files changed, 228 insertions(+), 71 deletions(-)
 create mode 100644 combo/apps/search/manager_views.py

diff --git a/combo/apps/search/forms.py b/combo/apps/search/forms.py
index 2664dbf..56d913f 100644
--- a/combo/apps/search/forms.py
+++ b/combo/apps/search/forms.py
@@ -16,19 +16,10 @@
 
 from django import forms
 
-from combo.utils.forms import MultiSortWidget
-
-from . import engines
 from .models import SearchCell
 
 
 class SearchCellForm(forms.ModelForm):
     class Meta:
         model = SearchCell
-        fields = ('_search_services', 'autofocus', 'input_placeholder')
-
-    def __init__(self, *args, **kwargs):
-        super(SearchCellForm, self).__init__(*args, **kwargs)
-        options = [(x, engines.get_engines()[x]['label']) for x in engines.get_engines().keys()]
-        self.fields['_search_services'].widget = MultiSortWidget(choices=options,
-                                                                 with_checkboxes=True)
+        fields = ('autofocus', 'input_placeholder')
diff --git a/combo/apps/search/manager_views.py b/combo/apps/search/manager_views.py
new file mode 100644
index 0000000..4d9a6f5
--- /dev/null
+++ b/combo/apps/search/manager_views.py
@@ -0,0 +1,75 @@
+# combo - content management system
+# Copyright (C) 2014-2020  Entr'ouvert
+#
+# This program is free software: you can redistribute it and/or modify it
+# under the terms of the GNU Affero General Public License as published
+# by the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU Affero General Public License for more details.
+#
+# You should have received a copy of the GNU Affero General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+from django.http import HttpResponse
+from django.http import HttpResponseRedirect
+from django.shortcuts import get_object_or_404
+from django.urls import reverse
+from django.utils.translation import ugettext_lazy as _
+
+from combo.apps.search.models import SearchCell
+from combo.data.models import PageSnapshot
+
+
+def page_search_cell_add_engine(request, page_pk, cell_reference, engine_slug):
+    cell = get_object_or_404(SearchCell, pk=cell_reference.split('-')[1], page=page_pk)
+
+    if engine_slug in cell.available_engines:
+        if not cell._search_services or not cell._search_services.get('data'):
+            cell._search_services = {'data': []}
+        cell._search_services['data'].append(engine_slug)
+        cell.save()
+        PageSnapshot.take(cell.page, request=request, comment=_('changed cell "%s"') % cell)
+
+    return HttpResponseRedirect('%s#cell-%s' % (
+        reverse('combo-manager-page-view', kwargs={'pk': page_pk}),
+        cell_reference))
+
+
+def page_search_cell_delete_engine(request, page_pk, cell_reference, engine_slug):
+    cell = get_object_or_404(SearchCell, pk=cell_reference.split('-')[1], page=page_pk)
+
+    if engine_slug in cell._search_services.get('data'):
+        cell._search_services['data'].remove(engine_slug)
+        cell.save()
+        PageSnapshot.take(cell.page, request=request, comment=_('changed cell "%s"') % cell)
+
+    return HttpResponseRedirect('%s#cell-%s' % (
+        reverse('combo-manager-page-view', kwargs={'pk': page_pk}),
+        cell_reference))
+
+
+def search_engines_order(request, page_pk, cell_reference):
+    cell = get_object_or_404(SearchCell, pk=cell_reference.split('-')[1], page=page_pk)
+
+    if not cell._search_services.get('data'):
+        return HttpResponse(status=204)
+
+    engines = []
+    for i, engine_slug in enumerate(cell._search_services['data']):
+        try:
+            new_order = int(request.GET.get('pos_' + str(engine_slug)))
+        except TypeError:
+            new_order = 0
+        engines.append((new_order, engine_slug))
+
+    ordered_engines = [a[1] for a in sorted(engines, key=lambda a: a[0])]
+    if ordered_engines != cell._search_services['data']:
+        cell._search_services['data'] = ordered_engines
+        cell.save()
+        PageSnapshot.take(cell.page, request=request, comment=_('changed cell "%s"') % cell)
+
+    return HttpResponse(status=204)
diff --git a/combo/apps/search/models.py b/combo/apps/search/models.py
index 3af855f..f38ed86 100644
--- a/combo/apps/search/models.py
+++ b/combo/apps/search/models.py
@@ -14,8 +14,6 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-import os
-
 from django.contrib.auth.models import Group
 from django.contrib.contenttypes import fields
 from django.contrib.contenttypes.models import ContentType
@@ -24,6 +22,7 @@ from django.utils.translation import ugettext_lazy as _
 from django import template
 from django.http import HttpResponse
 from django.core.exceptions import PermissionDenied
+from django.utils.functional import cached_property
 from django.utils.http import quote
 from django.template import RequestContext, Template
 
@@ -65,7 +64,7 @@ class SearchCell(CellBase):
             return self.slug.replace('-', '_')
         return ''
 
-    @property
+    @cached_property
     def search_services(self):
         services = []
         for service_slug in self._search_services.get('data') or []:
@@ -75,6 +74,12 @@ class SearchCell(CellBase):
                 services.append(service)
         return services
 
+    @cached_property
+    def available_engines(self):
+        all_engines = engines.get_engines()
+        current_engines = [e['slug'] for e in self.search_services]
+        return {k: v for k, v in all_engines.items() if k not in current_engines}
+
     @property
     def has_multiple_search_services(self):
         return len(self._search_services.get('data') or []) > 1
diff --git a/combo/apps/search/templates/combo/manager/search-cell-form.html b/combo/apps/search/templates/combo/manager/search-cell-form.html
index 8e71fb3..6e03542 100644
--- a/combo/apps/search/templates/combo/manager/search-cell-form.html
+++ b/combo/apps/search/templates/combo/manager/search-cell-form.html
@@ -12,5 +12,47 @@
   </p>
 </div>
 {% endif %}
-{{ block.super }}
+{{ form.as_p }}
+{% with cell.search_services as engines %}
+{% if engines %}
+<p><label>{% trans "Engines:" %}</label></p>
+<div>
+  <ul class="objects-list list-of-links" id="list-of-links-{{ cell.pk }}"
+     data-link-list-order-url="{% url 'combo-manager-search-engines-order' page_pk=page.pk cell_reference=cell.get_reference %}">
+    {% for engine in engines %}
+    <li data-link-item-id="{{ engine.slug }}"><span class="handle">⣿</span>
+      <span>{{ engine.label }}</span>
+      <a title="{% trans "Delete" %}" class="link-action-icon delete" href="{% url 'combo-manager-page-search-cell-delete-engine' page_pk=page.pk cell_reference=cell.get_reference engine_slug=engine.slug %}">{% trans "Delete" %}</a>
+    </li>
+    {% endfor %}
+  </ul>
+</div>
+<script>
+$(function () {
+  $('#list-of-links-{{ cell.pk }}').sortable({
+    handle: '.handle',
+    update: function(event, ui) {
+       var new_order = Object();
+       $(this).find('li').each(function(i, x) {
+           var suffix = $(x).data('link-item-id');
+           new_order['pos_' + suffix] = i;
+       });
+       $.ajax({
+           url: $(this).data('link-list-order-url'),
+           data: new_order
+       });
+    }
+  });
+});
+</script>
+{% endif %}
+{% endwith %}
+{% if cell.available_engines %}
+<div class="buttons">
+  {% trans "Add an engine:" %}
+  {% for key, engine in cell.available_engines.items %}
+  <a href="{% url 'combo-manager-page-search-cell-add-engine' page_pk=page.pk cell_reference=cell.get_reference engine_slug=key %}">{{ engine.label }}</a> {% if not forloop.last %}|{% endif %}
+  {% endfor %}
+</div>
+{% endif %}
 {% endblock %}
diff --git a/combo/apps/search/urls.py b/combo/apps/search/urls.py
index 33f9c4a..27876ed 100644
--- a/combo/apps/search/urls.py
+++ b/combo/apps/search/urls.py
@@ -14,11 +14,26 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
-from django.conf.urls import url
+from django.conf.urls import url, include
 
+from combo.urls_utils import decorated_includes, manager_required
 from .models import SearchCell
+from . import manager_views
+
+search_manager_urls = [
+    url(r'^pages/(?P<page_pk>\d+)/cell/(?P<cell_reference>[\w_]+-\d+)/engine/(?P<engine_slug>[\w_-]+)/add/$',
+        manager_views.page_search_cell_add_engine,
+        name='combo-manager-page-search-cell-add-engine'),
+    url(r'^pages/(?P<page_pk>\d+)/cell/(?P<cell_reference>[\w_]+-\d+)/engine/(?P<engine_slug>[\w_-]+)/delete/$',
+        manager_views.page_search_cell_delete_engine,
+        name='combo-manager-page-search-cell-delete-engine'),
+    url(r'^pages/(?P<page_pk>\d+)/cell/(?P<cell_reference>[\w_]+-\d+)/engine/order$',
+        manager_views.search_engines_order,
+        name='combo-manager-search-engines-order'),
+]
 
 urlpatterns = [
-    url(r'^ajax/search/(?P<cell_pk>\w+)/(?P<service_slug>[\w:-]+)/$', SearchCell.ajax_results_view,
+    url(r'^manage/search/', decorated_includes(manager_required, include(search_manager_urls))),
+    url(r'^ajax/search/(?P<cell_pk>\w+)/(?P<service_slug>[\w_-]+)/$', SearchCell.ajax_results_view,
         name='combo-search-ajax-results'),
 ]
diff --git a/tests/test_search.py b/tests/test_search.py
index 324ddca..20a2fb5 100644
--- a/tests/test_search.py
+++ b/tests/test_search.py
@@ -1,8 +1,6 @@
 import json
 import os
 import pytest
-import re
-import shutil
 import mock
 
 from django.conf import settings
@@ -11,8 +9,8 @@ from django.db import connection
 from django.test import override_settings
 from django.test.client import RequestFactory
 from django.test.utils import CaptureQueriesContext
-from django.core.management import call_command
 from django.urls import reverse
+from django.utils.http import urlencode
 
 from combo.apps.search.engines import engines
 from combo.apps.search.models import SearchCell, IndexedCell
@@ -217,6 +215,7 @@ def test_search_custom_templates(app):
             assert '<li><a href="http://example.net/123/">A B</a>' in resp.text
             assert '<div>description A</div>' in resp.text
 
+
 def test_search_cell_visibility(app):
     page = Page(title='example page', slug='example-page')
     page.save()
@@ -226,8 +225,10 @@ def test_search_cell_visibility(app):
         assert not cell.is_visible()
 
         cell._search_services = {'data': ['_text']}
+        del cell.search_services  # clear cache
         assert cell.is_visible()
 
+
 def test_search_contents():
     page = Page(title='example page', slug='example-page')
     page.save()
@@ -349,70 +350,98 @@ def test_search_external_links(app):
     assert hits[0]['url'] == 'http://example.net'
 
 
-def test_manager_search_cell(app, admin_user):
-    Page.objects.all().delete()
-    page = Page(title='One', slug='one', template_name='standard')
-    page.save()
+def test_manager_search_cell(settings, app, admin_user):
+    page = Page.objects.create(title='One', slug='one', template_name='standard')
+    cell = SearchCell.objects.create(page=page, placeholder='content', order=0)
     app = login(app)
-    resp = app.get('/manage/pages/%s/' % page.id)
-    resp = app.get(resp.html.find('option',
-                   **{'data-add-url': re.compile('search_searchcell')})['data-add-url'])
 
-    cells = Page.objects.get(id=page.id).get_cells()
-    assert len(cells) == 1
-    assert isinstance(cells[0], SearchCell)
+    settings.KNOWN_SERVICES = {}
+    assert cell._search_services == {}
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/delete/' % (page.pk, cell.pk) not in resp.text
+
+    settings.COMBO_SEARCH_SERVICES = SEARCH_SERVICES
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/delete/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/delete/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/delete/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/delete/' % (page.pk, cell.pk) not in resp.text
+
+    # add engines
+    resp = resp.click(href='.*/search_searchcell-%s/engine/_text/add/' % cell.pk)
+    assert resp.status_int == 302
+    assert resp.location.endswith('/manage/pages/%s/#cell-%s' % (page.pk, cell.get_reference()))
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    resp = resp.click(href='.*/search_searchcell-%s/engine/search1/add/' % cell.pk)
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    resp = resp.click(href='.*/search_searchcell-%s/engine/search_tmpl/add/' % cell.pk)
+    cell.refresh_from_db()
+    assert cell._search_services == {'data': ['_text', 'search1', 'search_tmpl']}
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/add/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/_text/delete/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/delete/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/delete/' % (page.pk, cell.pk) in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/delete/' % (page.pk, cell.pk) not in resp.text
+
+    # delete engines
+    resp = resp.click(href='.*/search_searchcell-%s/engine/_text/delete/' % cell.pk)
+    assert resp.status_int == 302
+    assert resp.location.endswith('/manage/pages/%s/#cell-%s' % (page.pk, cell.get_reference()))
+    cell.refresh_from_db()
+    assert cell._search_services == {'data': ['search1', 'search_tmpl']}
+
+    settings.COMBO_SEARCH_SERVICES = {}
+    # check there's no crash if search engines are removed from config
+    resp = app.get('/manage/pages/%s/' % page.pk)
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/add/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search1/delete/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_tmpl/delete/' % (page.pk, cell.pk) not in resp.text
+    assert '/manage/search/pages/%s/cell/search_searchcell-%s/engine/search_alternate_key/delete/' % (page.pk, cell.pk) not in resp.text
+
+
+def test_manager_search_cell_order(settings, app, admin_user):
+    settings.COMBO_SEARCH_SERVICES = SEARCH_SERVICES
+    page = Page.objects.create(title='One', slug='one', template_name='standard')
+    cell = SearchCell.objects.create(
+        page=page, placeholder='content', order=0,
+        _search_services={'data': ['_text', 'search1', 'search_tmpl']})
+
+    params = []
+    new_order = [2, 3, 1]
+    for i, (service_slug, new_pos) in enumerate(zip(cell._search_services['data'], new_order)):
+        params.append(('pos_%s' % service_slug, str(new_pos)))
 
-    with override_settings(KNOWN_SERVICES={}):
-        resp = app.get('/manage/pages/%s/' % page.id)
-        assert ('data-cell-reference="%s"' % cells[0].get_reference()) in resp.text
-        assert len(resp.form['c%s-_search_services' % cells[0].get_reference()].options) == 1
-
-        with SearchServices(SEARCH_SERVICES):
-            resp = app.get('/manage/pages/%s/' % page.id)
-            assert len(resp.form['c%s-_search_services' % cells[0].get_reference()].options) == 4
-            # simulate reordering of options
-            resp.form['c%s-_search_services' % cells[0].get_reference()].options = [
-                    (u'search_tmpl', False, u'Search with template'),
-                    (u'search_alternate_key', False, u'Search with alternate key'),
-                    (u'_text', False, u'Page Contents'),
-                    (u'search1', False, u'Search 1')]
-            resp.form['c%s-_search_services' % cells[0].get_reference()].value = ['search_tmpl', '_text']
-            resp.form['c%s-input_placeholder' % cells[0].get_reference()] = 'my placeholder'
-            resp = resp.form.submit()
-            assert resp.status_int == 302
-
-            # check selected engines are selected and the first items of the list
-            resp = app.get('/manage/pages/%s/' % page.id)
-            assert set(resp.form['c%s-_search_services' % cells[0].get_reference()].value) == set(['search_tmpl', '_text'])
-            assert resp.form['c%s-_search_services' % cells[0].get_reference()].options[0][0] == 'search_tmpl'
-            assert resp.form['c%s-_search_services' % cells[0].get_reference()].options[1][0] == '_text'
-
-            # check placeholder
-            resp.form['c%s-input_placeholder' % cells[0].get_reference()] == 'my placeholder'
-
-        # check there's no crash if search engines are removed from config
-        resp = app.get('/manage/pages/%s/' % page.id)
-        assert resp.form['c%s-_search_services' % cells[0].get_reference()].value == ['_text']
+    app = login(app)
+    resp = app.get('/manage/search/pages/%s/cell/%s/engine/order?%s' % (page.pk, cell.get_reference(), urlencode(params)))
+    assert resp.status_code == 204
+    cell.refresh_from_db()
+    assert cell._search_services == {'data': ['search_tmpl', '_text', 'search1']}
 
 
 def test_manager_waiting_index_message(app, admin_user):
-    Page.objects.all().delete()
-    page = Page(title='One', slug='one', template_name='standard')
-    page.save()
+    page = Page.objects.create(title='One', slug='one', template_name='standard')
+    cell = SearchCell.objects.create(page=page, placeholder='content', order=0)
     app = login(app)
-    resp = app.get('/manage/pages/%s/' % page.id)
-    resp = app.get(resp.html.find('option',
-                   **{'data-add-url': re.compile('search_searchcell')})['data-add-url'])
-    resp = resp.follow()
+    resp = app.get('/manage/pages/%s/' % page.pk)
     assert 'Content indexing has been scheduled' not in resp.text
 
-    cells = Page.objects.get(id=page.id).get_cells()
-    resp.form['c%s-_search_services' % cells[0].get_reference()] = ['_text']
-    resp = resp.form.submit().follow()
+    resp = resp.click(href='.*/search_searchcell-%s/engine/_text/add/' % cell.pk)
+    resp = app.get('/manage/pages/%s/' % page.pk)
     assert 'Content indexing has been scheduled' in resp.text
 
     index_site()
-    resp = app.get('/manage/pages/%s/' % page.id)
+    resp = app.get('/manage/pages/%s/' % page.pk)
     assert 'Content indexing has been scheduled' not in resp.text
 
 
-- 
2.20.1

