Clean-up autocomplete API, non-expired results first

Author: gdaviet

collectives/api/autocomplete_user.py

@@ -8,7 +8,7 @@
 
 from flask import request, abort
 from flask_login import current_user
-from sqlalchemy.sql import text
+from sqlalchemy.orm import Query
 from sqlalchemy import func, and_
 from marshmallow import fields
 
@@ -31,39 +31,18 @@ class Meta:
         fields = (
             "id",
             "full_name",
-            "license_expiry_date",
+            "is_active",
         )
 
 
-def find_users_by_fuzzy_name(pattern, limit=8):
-    """Find user for autocomplete from a part of their full name.
+def _make_autocomplete_query(pattern: str) -> Query:
+    """Builds the autocomplete query for the provided pattern"""
 
-    Comparison are case insensitive.
+    query = db.session.query(User)
+    query = query.filter(func.lower(User.full_name()).like(f"%{pattern}%"))
+    query = query.order_by(User.is_active.desc(), User.full_name(), User.id)
 
-    :param string pattern: Part of the name that will be searched.
-    :param int limit: Maximum number of response.
-    :return: List of users corresponding to ``pattern``
-    :rtype: list(:py:class:`collectives.models.user.User`)
-    """
-    if "sqlite" in db.engine.name:
-        # SQLlite does not follow SQL standard
-        concat_clause = "(first_name || ' ' || last_name)"
-    else:
-        concat_clause = "CONCAT(first_name, ' ', last_name)"
-
-    sql = (
-        f"SELECT id, first_name, last_name from users WHERE "
-        f"LOWER({concat_clause}) LIKE :pattern LIMIT :limit"
-    )
-
-    sql_pattern = f"%{pattern.lower()}%"
-    found_users = (
-        db.session.query(User)
-        .from_statement(text(sql))
-        .params(pattern=sql_pattern, limit=limit)
-    )
-
-    return found_users
+    return query
 
 
 @blueprint.route("/users/autocomplete/create_rental")
@@ -90,8 +69,8 @@ def autocomplete_users_create_rental():
     if pattern is None or (len(pattern) < 2):
         found_users = []
     else:
-        limit = request.args.get("l", type=int) or 8
-        found_users = find_users_by_fuzzy_name(pattern, limit)
+        limit = request.args.get("l", default=8, type=int)
+        found_users = _make_autocomplete_query(pattern).limit(limit)
 
     content = json.dumps(AutocompleteUserSchema(many=True).dump(found_users))
     return content, 200, {"content-type": "application/json"}
@@ -121,8 +100,8 @@ def autocomplete_users():
     if pattern is None or (len(pattern) < 2):
         found_users = []
     else:
-        limit = request.args.get("l", type=int) or 8
-        found_users = find_users_by_fuzzy_name(pattern, limit)
+        limit = request.args.get("l", default=8, type=int)
+        found_users = _make_autocomplete_query(pattern).limit(limit)
 
     content = json.dumps(AutocompleteUserSchema(many=True).dump(found_users))
     return content, 200, {"content-type": "application/json"}
@@ -148,16 +127,11 @@ def autocomplete_leaders():
     if len(pattern) < 2:
         found_users = []
     else:
-        limit = request.args.get("l", type=int) or 8
+        limit = request.args.get("l", default=8, type=int)
 
-        query = db.session.query(User)
-        condition = func.lower(User.first_name + " " + User.last_name).like(
-            f"%{pattern}%"
-        )
-        query = query.filter(condition)
+        query = _make_autocomplete_query(pattern)
         query = query.filter(User.led_events)
-        found_users = query.order_by(User.id).all()
-        found_users = found_users[0:limit]
+        found_users = query.limit(limit)
 
     content = json.dumps(AutocompleteUserSchema(many=True).dump(found_users))
     return content, 200, {"content-type": "application/json"}
@@ -188,15 +162,15 @@ def autocomplete_available_leaders():
     if pattern is None or (len(pattern) < 2):
         found_users = []
     else:
-        limit = request.args.get("l", type=int) or 8
+        limit = request.args.get("l", default=8, type=int)
+
+        query = _make_autocomplete_query(pattern)
         event_type = db.session.get(
             EventType, request.args.get("etype", type=int, default=0)
         )
         activity_ids = request.args.getlist("aid", type=int)
         existing_ids = request.args.getlist("eid", type=int)
 
-        query = db.session.query(User)
-
         if event_type and event_type.requires_activity:
             ok_roles = RoleIds.all_activity_leader_roles()
         else:
@@ -206,12 +180,7 @@ def autocomplete_available_leaders():
             role_condition = and_(role_condition, Role.activity_id.in_(activity_ids))
         query = query.filter(User.roles.any(role_condition))
         query = query.filter(~User.id.in_(existing_ids))
-        condition = func.lower(User.first_name + " " + User.last_name).ilike(
-            f"%{pattern}%"
-        )
-        query = query.filter(condition)
 
-        query = query.order_by(User.first_name, User.last_name, User.id)
         found_users = query.limit(limit)
 
     content = json.dumps(AutocompleteUserSchema(many=True).dump(found_users))

collectives/models/user/misc.py

@@ -14,7 +14,6 @@
 from collectives.models.registration import Registration, RegistrationStatus
 from collectives.models.reservation import ReservationStatus
 from collectives.models.user.enum import Gender, UserType
-from collectives.utils.time import current_time
 from collectives.utils.misc import is_valid_image
 
 
@@ -115,13 +114,6 @@ def has_signed_legal_text(self):
 
     # Format
 
-    def full_name(self):
-        """Get user full name.
-
-        :rtype: String
-        """
-        return f"{self.first_name} {self.last_name.upper()}"
-
     def __str__(self) -> str:
         """Displays the user name."""
         return self.full_name() + f" (ID {self.id})"
@@ -158,25 +150,6 @@ def get_reservations_completed(self):
                 reservation_list.append(reservation)
         return reservation_list
 
-    @property
-    def is_active(self):
-        """Check if user is currently active.
-
-        An active user is not disabled, its license is valid and
-        is not candidate user.
-
-        :return: True if user is active.
-        :rtype: boolean
-        """
-        if not self.enabled:
-            return False
-        if not self.check_license_valid_at_time(current_time()):
-            return False
-        if self.type == UserType.UnverifiedLocal:
-            return False
-
-        return True
-
     def has_valid_phone_number(self, emergency=False):
         """Check if the user has a valid phone number.
 

collectives/models/user/model.py

@@ -5,12 +5,14 @@
 from wtforms.validators import Email
 from sqlalchemy_utils import PasswordType
 from sqlalchemy.orm import validates
+from sqlalchemy.sql import and_, case
 from sqlalchemy.ext.declarative import declared_attr
-
+from sqlalchemy.ext.hybrid import hybrid_property, hybrid_method
 
 from collectives.models.globals import db
 from collectives.models.user.enum import Gender, UserType
 from collectives.utils.misc import truncate
+from collectives.utils.time import current_time
 
 
 class UserModelMixin:
@@ -317,3 +319,57 @@ def question_answers(self):
             backref="user",
             lazy=True,
         )
+
+    @hybrid_method
+    def full_name(self) -> str:
+        """Returns the user full name as a string"""
+        return self.first_name + " " + self.last_name.upper()
+
+    # pylint: disable=no-self-argument
+
+    @full_name.expression
+    def full_name(cls):
+        """Returns the user full name as a SQL expression"""
+        return cls.first_name + " " + cls.last_name
+
+    @hybrid_property
+    def is_active(self) -> bool:
+        """Check if user is currently active.
+
+        An active user is not disabled, its license is valid and
+        is not candidate user.
+
+        :return: True if user is active.
+        """
+        if not self.enabled:
+            return False
+        if self.type == UserType.Extranet and not self.check_license_valid_at_time(
+            current_time()
+        ):
+            return False
+        if self.type == UserType.UnverifiedLocal:
+            return False
+
+        return True
+
+    @is_active.expression
+    def is_active(cls):
+        """Check if user is currently active.
+
+        An active user is not disabled, its license is valid and
+        is not candidate user.
+
+        :return: A SQL expression
+        """
+
+        return and_(
+            cls.enabled,
+            case(
+                {
+                    UserType.UnverifiedLocal: False,
+                    UserType.Extranet: cls.license_expiry_date >= current_time(),
+                },
+                value=cls.type,
+                else_=True,
+            ),
+        )

collectives/static/js/utils/autocomplete.js

@@ -54,7 +54,7 @@ function setupAutoComplete(
     const renderItem = function (item) {
         var val = itemValue(item);
         var innerHTML = settings.itemInnerHTML(item, val);
-        if (new Date(item.license_expiry_date) < new Date() ){
+        if (item.is_active === false){
             var style = "text-decoration-line: line-through; color: grey; font-style: italic;"
             var misc = `(expiré)`
         }

collectives/templates/event/partials/admin.html

@@ -127,7 +127,7 @@ <h5 class="heading-5 collective-display--administration-title"> Inscrire un adh
     <form action="{{url_for('event.register_user', event_id=event.id)}}" id="user-search-form" method="POST" class="form form-vertical" >
     <div id="user-search-data" class="form-search search-data">
         <p>
-            <input id = "user-search"  autocomplete="off" class="search-input" placeholder="Nom…" >
+            <input id = "user-search" size="32" autocomplete="off" class="search-input" placeholder="Nom…" >
         </p>
         {{ register_user_form.user_id(id = 'user-search-resultid', type = 'hidden') }}
         {{ register_user_form.hidden_tag() }}