Autocompletion: new de-duplication mechanism

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()),
     ]
 
 

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/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()),
     ]
 
 

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 this avoids showing the same user twice (here and in the
+    ...         # 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