From c7ca96bce5c83d1a0a9df64c9360e650da1dc328 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sat, 4 Jul 2020 13:06:24 +0200 Subject: [PATCH 1/3] Autocompletion: new de-duplication mechanism --- bds/autocomplete.py | 12 ++++++-- gestioncof/autocomplete.py | 12 ++++++-- kfet/autocomplete.py | 17 ++++++++--- shared/views/autocomplete.py | 58 ++++++++++++++++++++++++++---------- 4 files changed, 73 insertions(+), 26 deletions(-) diff --git a/bds/autocomplete.py b/bds/autocomplete.py index 0a240cea..6660a7d6 100644 --- a/bds/autocomplete.py +++ b/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()), ] diff --git a/gestioncof/autocomplete.py b/gestioncof/autocomplete.py index 239317f8..648b239b 100644 --- a/gestioncof/autocomplete.py +++ b/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()), ] diff --git a/kfet/autocomplete.py b/kfet/autocomplete.py index c4e7a766..2a88ce1f 100644 --- a/kfet/autocomplete.py +++ b/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()), ] diff --git a/shared/views/autocomplete.py b/shared/views/autocomplete.py index 50d0d2c2..d7d61b13 100644 --- a/shared/views/autocomplete.py +++ b/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 From e9f00b4f06a02d20f205c7439839ac16d6e5b41c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sat, 4 Jul 2020 13:30:54 +0200 Subject: [PATCH 2/3] Update the isort config for version 5.* --- .gitlab-ci.yml | 2 +- .pre-commit.sh | 2 +- cof/settings/bds_prod.py | 4 ++-- cof/settings/cof_prod.py | 4 ++-- cof/settings/local.py | 4 ++-- cof/urls.py | 8 ++++---- gestioncof/apps.py | 1 + kfet/apps.py | 1 + setup.cfg | 1 - 9 files changed, 14 insertions(+), 13 deletions(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 3ef29950..28ab0748 100644 --- a/.gitlab-ci.yml +++ b/.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: diff --git a/.pre-commit.sh b/.pre-commit.sh index 0e0e3c1a..abf1fe7d 100755 --- a/.pre-commit.sh +++ b/.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 diff --git a/cof/settings/bds_prod.py b/cof/settings/bds_prod.py index d674a0a6..65245ad2 100644 --- a/cof/settings/bds_prod.py +++ b/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 # --- diff --git a/cof/settings/cof_prod.py b/cof/settings/cof_prod.py index fe60af24..4bca38d2 100644 --- a/cof/settings/cof_prod.py +++ b/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 # --- diff --git a/cof/settings/local.py b/cof/settings/local.py index b0ce5ae1..ee9fc407 100644 --- a/cof/settings/local.py +++ b/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 # --- diff --git a/cof/urls.py b/cof/urls.py index 12cf4f5a..0b03f2db 100644 --- a/cof/urls.py +++ b/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 += [ diff --git a/gestioncof/apps.py b/gestioncof/apps.py index 88e2fbfc..0ac33f93 100644 --- a/gestioncof/apps.py +++ b/gestioncof/apps.py @@ -12,6 +12,7 @@ class GestioncofConfig(AppConfig): def register_config(self): import djconfig + from .forms import GestioncofConfigForm djconfig.register(GestioncofConfigForm) diff --git a/kfet/apps.py b/kfet/apps.py index f3c7b07b..2843fd67 100644 --- a/kfet/apps.py +++ b/kfet/apps.py @@ -10,6 +10,7 @@ class KFetConfig(AppConfig): def register_config(self): import djconfig + from kfet.forms import KFetConfigForm djconfig.register(KFetConfigForm) diff --git a/setup.cfg b/setup.cfg index 1a9901cb..995af0a4 100644 --- a/setup.cfg +++ b/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 From fbbc9937f638fd513b347b82a9bf111fa4f65e96 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20P=C3=A9pin?= Date: Sun, 5 Jul 2020 11:14:51 +0200 Subject: [PATCH 3/3] Fix a typo --- shared/views/autocomplete.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/shared/views/autocomplete.py b/shared/views/autocomplete.py index d7d61b13..4aed6a2e 100644 --- a/shared/views/autocomplete.py +++ b/shared/views/autocomplete.py @@ -195,8 +195,8 @@ class Compose: ... ... 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). + ... # registered users, this avoids showing the same user twice (here and in + ... # then ldap unit). ... return user.username >>> >>> class UserAndClipperSearch(Compose):