Merge branch 'kerl/factor_autocompletion_views1' into 'master'

Mécanisme de dé-duplication des résultats plus souple pour l'autocomplétion

See merge request klub-dev-ens/gestioCOF!427

.gitlab-ci.yml

@@ -63,7 +63,7 @@ linters:
         - pip install --upgrade black isort flake8
     script:
         - black --check .
-        - isort --recursive --check-only --diff bda bds clubs cof events gestioncof kfet petitscours provisioning shared
+        - isort --check --diff .
         # Print errors only
         - flake8 --exit-zero bda bds clubs cof events gestioncof kfet petitscours provisioning shared
     cache:

.pre-commit.sh

@@ -48,7 +48,7 @@ if type isort &>/dev/null; then
         ISORT_OUTPUT="/tmp/gc-isort-output.log"
         touch $ISORT_OUTPUT
 
-        if ! echo "$STAGED_PYTHON_FILES" | xargs -d'\n' isort --check-only &>$ISORT_OUTPUT; then
+        if ! echo "$STAGED_PYTHON_FILES" | xargs -d'\n' isort --check &>$ISORT_OUTPUT; then
             echo "$STAGED_PYTHON_FILES" | xargs -d'\n' isort &>$ISORT_OUTPUT
             printf "Reformatted.\n"
             formatter_updated=1

bds/autocomplete.py

@@ -15,6 +15,9 @@ class BDSMemberSearch(autocomplete.ModelSearch):
         qset_filter &= Q(bds__is_member=True)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class BDSOthersSearch(autocomplete.ModelSearch):
     model = User
@@ -25,12 +28,15 @@ class BDSOthersSearch(autocomplete.ModelSearch):
         qset_filter &= Q(bds__isnull=True) | Q(bds__is_member=False)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class BDSSearch(autocomplete.Compose):
     search_units = [
-        ("members", "username", BDSMemberSearch),
-        ("others", "username", BDSOthersSearch),
-        ("clippers", "clipper", autocomplete.LDAPSearch),
+        ("members", BDSMemberSearch()),
+        ("others", BDSOthersSearch()),
+        ("clippers", autocomplete.LDAPSearch()),
     ]
 
 

cof/settings/bds_prod.py

@@ -2,12 +2,12 @@
 Django development settings for the cof project.
 The settings that are not listed here are imported from .common
 """
-
 import os
 
-from .common import *  # NOQA
 from .common import BASE_DIR, INSTALLED_APPS
 
+from .common import *  # NOQA
+
 # ---
 # BDS-only Django settings
 # ---

cof/settings/cof_prod.py

@@ -2,10 +2,8 @@
 Django development settings for the cof project.
 The settings that are not listed here are imported from .common
 """
-
 import os
 
-from .common import *  # NOQA
 from .common import (
     AUTHENTICATION_BACKENDS,
     BASE_DIR,
@@ -15,6 +13,8 @@ from .common import (
     import_secret,
 )
 
+from .common import *  # NOQA
+
 # ---
 # COF-specific secrets
 # ---

cof/settings/local.py

@@ -1,11 +1,11 @@
 """Django local development settings."""
-
 import os
 
 from . import bds_prod
-from .cof_prod import *  # NOQA
 from .cof_prod import BASE_DIR, INSTALLED_APPS, MIDDLEWARE, TESTING
 
+from .cof_prod import *  # NOQA
+
 # ---
 # Merge COF and BDS configs
 # ---

cof/urls.py

@@ -1,7 +1,6 @@
 """
 Fichier principal de configuration des urls du projet GestioCOF
 """
-
 from django.conf import settings
 from django.conf.urls.i18n import i18n_patterns
 from django.conf.urls.static import static
@@ -20,6 +19,10 @@ urlpatterns = [
 ]
 
 if "gestioncof" in settings.INSTALLED_APPS:
+    from django_js_reverse.views import urls_js
+    from wagtail.admin import urls as wagtailadmin_urls
+    from wagtail.documents import urls as wagtaildocs_urls
+
     from gestioncof import csv_views, views as gestioncof_views
     from gestioncof.autocomplete import autocomplete
     from gestioncof.urls import (
@@ -29,9 +32,6 @@ if "gestioncof" in settings.INSTALLED_APPS:
         export_patterns,
         surveys_patterns,
     )
-    from django_js_reverse.views import urls_js
-    from wagtail.admin import urls as wagtailadmin_urls
-    from wagtail.documents import urls as wagtaildocs_urls
 
     # Also includes BdA, K-Fêt, etc.
     urlpatterns += [

gestioncof/apps.py

@@ -12,6 +12,7 @@ class GestioncofConfig(AppConfig):
 
     def register_config(self):
         import djconfig
+
         from .forms import GestioncofConfigForm
 
         djconfig.register(GestioncofConfigForm)

gestioncof/autocomplete.py

@@ -18,6 +18,9 @@ class COFMemberSearch(autocomplete.ModelSearch):
         qset_filter &= Q(profile__is_cof=True)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class COFOthersSearch(autocomplete.ModelSearch):
     model = User
@@ -28,12 +31,15 @@ class COFOthersSearch(autocomplete.ModelSearch):
         qset_filter &= Q(profile__is_cof=False)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class COFSearch(autocomplete.Compose):
     search_units = [
-        ("members", "username", COFMemberSearch),
-        ("others", "username", COFOthersSearch),
-        ("clippers", "clipper", autocomplete.LDAPSearch),
+        ("members", COFMemberSearch()),
+        ("others", COFOthersSearch()),
+        ("clippers", autocomplete.LDAPSearch()),
     ]
 
 

kfet/apps.py

@@ -10,6 +10,7 @@ class KFetConfig(AppConfig):
 
     def register_config(self):
         import djconfig
+
         from kfet.forms import KFetConfigForm
 
         djconfig.register(KFetConfigForm)

kfet/autocomplete.py

@@ -23,6 +23,9 @@ class KfetAccountSearch(autocomplete.ModelSearch):
         qset_filter &= Q(profile__account_kfet__isnull=False)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class COFMemberSearch(autocomplete.ModelSearch):
     model = User
@@ -33,6 +36,9 @@ class COFMemberSearch(autocomplete.ModelSearch):
         qset_filter &= Q(profile__account_kfet__isnull=True) & Q(profile__is_cof=True)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class OthersSearch(autocomplete.ModelSearch):
     model = User
@@ -43,13 +49,16 @@ class OthersSearch(autocomplete.ModelSearch):
         qset_filter &= Q(profile__account_kfet__isnull=True) & Q(profile__is_cof=False)
         return qset_filter
 
+    def result_uuid(self, user):
+        return user.username
+
 
 class KfetAutocomplete(autocomplete.Compose):
     search_units = [
-        ("kfet", "username", KfetAccountSearch),
-        ("users_cof", "username", COFMemberSearch),
-        ("users_notcof", "username", OthersSearch),
-        ("clippers", "clipper", autocomplete.LDAPSearch),
+        ("kfet", KfetAccountSearch()),
+        ("users_cof", COFMemberSearch()),
+        ("users_notcof", OthersSearch()),
+        ("clippers", autocomplete.LDAPSearch()),
     ]
 
 

setup.cfg

@@ -39,5 +39,4 @@ known_django = django
 known_first_party = bda,bds,clubs,cof,events,gestioncof,kfet,petitscours,shared
 line_length = 88
 multi_line_output = 3
-not_skip = __init__.py
 sections = FUTURE,STDLIB,THIRDPARTY,FIRSTPARTY,LOCALFOLDER

shared/views/autocomplete.py

@@ -21,14 +21,32 @@ class SearchUnit:
 
     A search unit should implement a `search` method taking a list of keywords as
     argument and returning an iterable of search results.
+
+    It might optionally implement the following methods and attributes:
+
+    - result_uuid (method): a callable that takes one result as an input and returns an
+      identifier that is globally unique across search units for this object.
+      This is used to compare results coming from different search units in the
+      `Compose` class. For instance, if the same user can be returned by the LDAP
+      search and a model search instance, using the clipper login as a UUID in both
+      units avoids this user to be returned twice by `Compose`.
+      Returning `None` means that the object should be considered unique.
     """
 
+    # Mandatory method
+
     def search(self, _keywords):
         raise NotImplementedError(
-            "Class implementing the SeachUnit interface should implement the search "
+            "Class implementing the SearchUnit interface should implement the search "
             "method"
         )
 
+    # Optional attributes and methods
+
+    def result_uuid(self, result):
+        """A universal unique identifier for the search results."""
+        return None
+
 
 # ---
 # Model-based search
@@ -148,6 +166,9 @@ class LDAPSearch(SearchUnit):
             django_logger.error("An LDAP error occurred", exc_info=err)
             return []
 
+    def result_uuid(self, clipper):
+        return clipper.clipper
+
 
 # ---
 # Composition of autocomplete units
@@ -157,18 +178,13 @@ class LDAPSearch(SearchUnit):
 class Compose:
     """Search with several units and remove duplicate results.
 
-    The `search_units` class attribute should be a list of tuples of the form `(name,
-    uniq_key, search_unit)`.
+    The `search_units` class attribute should be a list of pairs of the form `(name,
+    search_unit)`.
 
     The `search` method produces a dictionary whose keys are the `name`s given in
     `search_units` and whose values are iterables produced by the different search
     units.
 
-    The `uniq_key`s are used to remove duplicates: for instance, say that search unit
-    1 has `uniq_key = "username"` and search unit 2 has `uniq_key = "clipper"`, then
-    search results from unit 2 whose `.clipper` attribute is equal to the
-    `.username` attribute of some result from unit 1 are omitted.
-
     Typical Example:
 
     >>> from django.contrib.auth.models import User
@@ -176,11 +192,17 @@ class Compose:
     >>> class UserSearch(ModelSearch):
     ...     model = User
     ...     search_fields = ["username", "first_name", "last_name"]
+    ...
+    ...     def result_uuid(self, user):
+    ...         # Assuming that `.username` stores the clipper login of already
+    ...         # registered users, this avoids showing the same user twice (here and in
+    ...         # then ldap unit).
+    ...         return user.username
     >>>
     >>> class UserAndClipperSearch(Compose):
     ...     search_units = [
-    ...         ("users", "username", UserSearch),
-    ...         ("clippers", "clipper", LDAPSearch),
+    ...         ("users", UserSearch()),
+    ...         ("clippers", LDAPSearch()),
     ...     ]
 
     In this example, clipper accounts that already have an associated user (i.e. with a
@@ -190,11 +212,15 @@ class Compose:
     search_units = []
 
     def search(self, keywords):
-        uniq_results = set()
+        seen_uuids = set()
         results = {}
-        for name, uniq_key, search_unit in self.search_units:
-            res = search_unit().search(keywords)
-            res = [r for r in res if getattr(r, uniq_key) not in uniq_results]
-            uniq_results |= set((getattr(r, uniq_key) for r in res))
-            results[name] = res
+        for name, search_unit in self.search_units:
+            uniq_res = []
+            for r in search_unit.search(keywords):
+                uuid = search_unit.result_uuid(r)
+                if uuid is None or uuid not in seen_uuids:
+                    uniq_res.append(r)
+                    if uuid is not None:
+                        seen_uuids.add(uuid)
+            results[name] = uniq_res
         return results