From 7a3d70a30e457d921f349f1d3b7a5ee19ab242db Mon Sep 17 00:00:00 2001
From: Emmanuel Cazenave <ecazenave@entrouvert.com>
Date: Fri, 17 May 2019 15:51:53 +0200
Subject: [PATCH] planitec: filters homogenisation (#32828)

---
 passerelle/contrib/planitech/models.py | 81 +++++++++++++-------------
 tests/test_planitech.py                | 77 +++++++++++++++++++++---
 2 files changed, 109 insertions(+), 49 deletions(-)

diff --git a/passerelle/contrib/planitech/models.py b/passerelle/contrib/planitech/models.py
index 6ff17690..ac2ed2cc 100644
--- a/passerelle/contrib/planitech/models.py
+++ b/passerelle/contrib/planitech/models.py
@@ -283,15 +283,7 @@ class PlanitechConnector(BaseResource):
             raise APIError(error_msg)
         return mste.decode(response.json())
 
-    def _get_places_by_capacity(self, min_capacity, max_capacity):
-        places = self._get_places_referential()
-        min_capacity = int(min_capacity)
-        max_capacity = int(max_capacity)
-        return [place['identifier'] for place in places.values()
-                if (min_capacity <= place['capacity'] <= max_capacity)
-                and place['capacity']]
-
-    def _get_places_referential(self, refresh_cache=False):
+    def _raw_get_places_referential(self, refresh_cache=False):
         cache_key = 'planitech-%s-places' % self.id
         ref = cache.get(cache_key)
         if ref is None or refresh_cache:
@@ -342,28 +334,36 @@ class PlanitechConnector(BaseResource):
             cache.set(cache_key, ref)
         return ref
 
-    def _filter_places_referential(self, places_ref, ref_filters):
-        # Strip filter name from their prefix
-        filters = {}
-        for p_name, p_value in ref_filters.items():
-            if p_name.startswith('referential_'):
-                p_name = p_name.replace('referential_', '')
-                filters[p_name] = p_value
-
-        # Filter on custom attributes
-        if filters:
-            res = {}
-            for place_id, place in places_ref.items():
-                for filter_name, filter_value in filters.items():
-                    for field in self.custom_fields:
-                        if filter_name == field['name']:
-                            if field['type'] == 'int':
-                                filter_value = int(filter_value)
-                            if place.get(filter_name) == filter_value:
-                                res[place_id] = place
-            places_ref = res
-
-        return places_ref
+    def _get_places_referential(self, min_capacity=0, max_capacity=100000, **kwargs):
+
+        ref = self._raw_get_places_referential()
+        res = {}
+
+        try:
+            min_capacity, max_capacity = int(min_capacity), int(max_capacity)
+        except ValueError:
+            raise APIError("min_capacity and max_capacity must be integers")
+
+        for place_id, place_data in ref.items():
+            # Filter on capacity
+            if 'capacity' in place_data:
+                if place_data['capacity'] < min_capacity or place_data['capacity'] > max_capacity:
+                    continue
+
+            # Filter on custom fields
+            skip = False
+            for filter_name, filter_value in kwargs.items():
+                for field in self.custom_fields:
+                    if filter_name == field['name']:
+                        if field['type'] == 'int':
+                            filter_value = int(filter_value)
+                        if place_data.get(filter_name) != filter_value:
+                            skip = True
+            if skip:
+                continue
+            res[place_id] = place_data
+
+        return res
 
     def _login(self):
         try:
@@ -485,7 +485,7 @@ class PlanitechConnector(BaseResource):
         }
 
     def hourly(self):
-        self._get_places_referential(refresh_cache=True)
+        self._raw_get_places_referential(refresh_cache=True)
 
     def _date_display(self, raw_data):
         available_dates = set()
@@ -507,14 +507,14 @@ class PlanitechConnector(BaseResource):
         available_places = []
         for place in raw_data.get('availablePlaces', []):
             available_places.append(int(place['placeIdentifier']))
-        places_ref = self._get_places_referential()
+        places_ref = self._raw_get_places_referential()
         res = []
         for place in available_places:
             res.append({"id": place, "text": places_ref[place]['label']})
         return res
 
     def _full_display(self, raw_data):
-        places_ref = self._get_places_referential()
+        places_ref = self._raw_get_places_referential()
         res = {
             'date': self._date_display(raw_data),
             'place': self._place_display(raw_data)
@@ -624,11 +624,9 @@ class PlanitechConnector(BaseResource):
         # Places restriction
         if place_id is not None:
             places_id = [int(place_id)]
-        elif kwargs:
-            places_id = self._filter_places_referential(
-                self._get_places_referential(), kwargs).keys()
         else:
-            places_id = self._get_places_by_capacity(int(min_capacity), int(max_capacity))
+            places_id = self._get_places_referential(
+                min_capacity=min_capacity, max_capacity=max_capacity, **kwargs).keys()
 
         params = {
             "placeIdentifiers": [float(p_id) for p_id in places_id],
@@ -673,18 +671,17 @@ class PlanitechConnector(BaseResource):
             id_ = int(id)
         except ValueError:
             raise APIError('ID must be an integer')
-        ref = self._get_places_referential()
+        ref = self._raw_get_places_referential()
         if id_ not in ref:
             raise APIError('No place with ID %s' % id_)
         return {
-            'data': self._get_places_referential()[int(id_)]
+            'data': ref[int(id_)]
         }
 
     @endpoint(description_get=_('Get places referential'), methods=['get'], perm='can_access')
     def getplacesreferential(self, request, **kwargs):
         return {
-            'data': self._filter_places_referential(
-                self._get_places_referential(), kwargs)
+            'data': self._get_places_referential(**kwargs)
         }
 
     @endpoint(description_get=_('Get reservation infos'), methods=['get'], perm='can_access')
diff --git a/tests/test_planitech.py b/tests/test_planitech.py
index d1261e91..3ffb0566 100644
--- a/tests/test_planitech.py
+++ b/tests/test_planitech.py
@@ -95,7 +95,7 @@ def mock_planitech(monkeypatch, return_value=None, side_effect=None, referential
     if referential is not None:
         mock_get_referential = mock.Mock(return_value=referential)
         monkeypatch.setattr(
-            models.PlanitechConnector, '_get_places_referential', mock_get_referential)
+            models.PlanitechConnector, '_raw_get_places_referential', mock_get_referential)
 
     return mock_call_planitech
 
@@ -329,7 +329,8 @@ def test_getplaces_referential(app, connector, monkeypatch):
         {
             'placesList': [
                 {'identifier': 1.0, 'label': 'salle 1'},
-                {'identifier': 2.0, 'label': 'salle 2'}
+                {'identifier': 2.0, 'label': 'salle 2'},
+                {'identifier': 3.0, 'label': 'salle 3'}
             ]
         },
         {
@@ -342,6 +343,10 @@ def test_getplaces_referential(app, connector, monkeypatch):
                 {
                     'identifier': 2.0, 'capacity': 20.0,
                     'some_custom_field': 'Yes'
+                },
+                {
+                    'identifier': 3.0, 'capacity': 30.0,
+                    'some_custom_field': 'Yes'
                 }
             ]
         }
@@ -349,6 +354,11 @@ def test_getplaces_referential(app, connector, monkeypatch):
     mock_planitech(monkeypatch, side_effect=side_effect)
     response = app.get('/planitech/slug-planitech/getplacesreferential')
     expected_res = {
+        '3': {
+            u'capacity': 30, u'label': u'salle 3', u'identifier': 3,
+            'street_number': None, 'address': None,
+            'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        },
         '2': {
             u'capacity': 20, u'label': u'salle 2', u'identifier': 2,
             'street_number': None, 'address': None,
@@ -362,16 +372,56 @@ def test_getplaces_referential(app, connector, monkeypatch):
     }
     assert response.json['data'] == expected_res
 
+    # Filter on custom fields
+    mock_planitech(monkeypatch, side_effect=side_effect)
+    response = app.get(
+        '/planitech/slug-planitech/getplacesreferential?some_custom_field=Yes')
+    assert response.json['data'] == {
+        '3': {
+            u'capacity': 30, u'label': u'salle 3', u'identifier': 3,
+            'street_number': None, 'address': None,
+            'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        },
+        '2': {
+            u'capacity': 20, u'label': u'salle 2', u'identifier': 2,
+            'street_number': None, 'address': None,
+            'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        }
+    }
+
+    # Filter on custom fields and capacity
     mock_planitech(monkeypatch, side_effect=side_effect)
     response = app.get(
-        '/planitech/slug-planitech/getplacesreferential?referential_some_custom_field=Yes')
+        '/planitech/slug-planitech/getplacesreferential?some_custom_field=Yes'
+        '&min_capacity=25')
     assert response.json['data'] == {
+        '3': {
+            u'capacity': 30, u'label': u'salle 3', u'identifier': 3,
+            'street_number': None, 'address': None,
+            'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        }
+    }
+
+    # Unkown filter filters nothing
+    response = app.get('/planitech/slug-planitech/getplacesreferential?unkownfilter=foo')
+    expected_res = {
+        '3': {
+            u'capacity': 30, u'label': u'salle 3', u'identifier': 3,
+            'street_number': None, 'address': None,
+            'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        },
         '2': {
             u'capacity': 20, u'label': u'salle 2', u'identifier': 2,
             'street_number': None, 'address': None,
             'city': None, 'zipcode': None, 'some_custom_field': 'Yes'
+        },
+        '1': {
+            u'capacity': 10, u'label': u'salle 1', u'identifier': 1,
+            'street_number': 1, 'address': 'rue planitech',
+            'city': 'thecity', 'zipcode': '00000', 'some_custom_field': None
         }
     }
+    assert response.json['data'] == expected_res
 
 
 def test_getplace(app, connector, monkeypatch):
@@ -510,6 +560,10 @@ def test_get_reservations_infos(app, connector, monkeypatch):
 
 def freegaps_data():
     referential = {
+        3.0: {
+            u'capacity': 30.0, u'label': u'salle 2', u'identifier': 2.0,
+            u'some_custom_field': u'Yes'
+        },
         2.0: {
             u'capacity': 20.0, u'label': u'salle 2', u'identifier': 2.0,
             u'some_custom_field': u'Yes'
@@ -619,7 +673,7 @@ def test_get_freegaps(app, connector, monkeypatch, settings):
     call_params = mock_call_planitech.call_args[0][2]
     assert call_params['startingDate'] == datetime(2018, 11, 11, 10, 0)
     assert call_params['endingDate'] == datetime(2018, 11, 12, 00, 0)
-    assert call_params['placeIdentifiers'] == [1.0, 2.0]
+    assert call_params['placeIdentifiers'] == [1.0, 2.0, 3.0]
     assert call_params['requestedStartingTime'] == 0.0  # means startingDate
     assert call_params['requestedEndingTime'] == 60.0  # means startingDate + 60 minutes
     assert call_params['reservationDays'] == [0, 6]  # means every day of the week
@@ -645,7 +699,7 @@ def test_get_freegaps(app, connector, monkeypatch, settings):
     call_params = mock_call_planitech.call_args[0][2]
     assert call_params['placeIdentifiers'] == [1.0]
 
-    # place_id parameter override capacity
+    # place_id disable filtering
     mock_call_planitech.reset_mock()
     response = app.get(
         '/planitech/slug-planitech/getfreegaps?start_time=11:00&&end_time=14:00'
@@ -668,10 +722,19 @@ def test_get_freegaps(app, connector, monkeypatch, settings):
     mock_call_planitech.reset_mock()
     response = app.get(
         '/planitech/slug-planitech/getfreegaps?start_time=11:00&&end_time=14:00'
-        '&start_date=2018-11-11&referential_some_custom_field=Yes&display=place'
+        '&start_date=2018-11-11&some_custom_field=Yes&display=place'
     )
     call_params = mock_call_planitech.call_args[0][2]
-    assert call_params['placeIdentifiers'] == [2.0]
+    assert call_params['placeIdentifiers'] == [2.0, 3.0]
+
+    # custom field and capacity restriction
+    mock_call_planitech.reset_mock()
+    response = app.get(
+        '/planitech/slug-planitech/getfreegaps?start_time=11:00&&end_time=14:00'
+        '&start_date=2018-11-11&some_custom_field=Yes&min_capacity=25&display=place'
+    )
+    call_params = mock_call_planitech.call_args[0][2]
+    assert call_params['placeIdentifiers'] == [3.0]
 
     # BAD REQUEST
     # bad date format
-- 
2.20.1

