From e54270dec2b98d0fb902eb2f229b2d7fd72b5945 Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 11:22:01 +0100 Subject: [PATCH 1/6] fix user district --- location/models.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/location/models.py b/location/models.py index 3729c22..aadf070 100644 --- a/location/models.py +++ b/location/models.py @@ -262,6 +262,12 @@ def cache_location_graph(location_id=None): cache.set("location_types", location_types, timeout=None) +def cache_location_graph_if_empty(): + if cache.has_key("location_types") is None: + cache_location_graph() + + + def extend_allowed_locations(location_pks, strict=True, loc_types=None): """ Get underlying locations for given location PKs. @@ -271,11 +277,8 @@ def extend_allowed_locations(location_pks, strict=True, loc_types=None): logger.error( f"extend_allowed_locations is expecting a list but received {location_pks}" ) + cache_location_graph_if_empty() graph = cache.get("location_graph") - if not graph: - cache_location_graph() - graph = cache.get("location_graph") - result_pks = set() to_visit = set(location_pks) visited = set() @@ -591,10 +594,8 @@ def get_user_districts(cls, user): cachedata = cache.get(f"user_districts_{user.id}") districts = [] if cachedata is None: + cache_location_graph_if_empty() cache_district = cache.get("location_types") - if not cache_district: - cache_location_graph() - cache_district = cache.get("location_types") cachedata = [] if user.is_superuser: for loc in cache_district['D']: @@ -617,7 +618,7 @@ def get_user_districts(cls, user): .order_by("location__code") ) for d in districts: - cachedata.append([d.id, d.location]) + cachedata.append([d.id, d.location_id]) cache.set(f"user_districts_{user.id}", cachedata) From 772b81ac7d630111705a5347fa9805de861b56fe Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 11:32:54 +0100 Subject: [PATCH 2/6] Fixing error with district user and cache --- location/models.py | 2 +- location/tests/test.py | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/location/models.py b/location/models.py index aadf070..2850abd 100644 --- a/location/models.py +++ b/location/models.py @@ -263,7 +263,7 @@ def cache_location_graph(location_id=None): def cache_location_graph_if_empty(): - if cache.has_key("location_types") is None: + if not cache.has_key("location_types"): cache_location_graph() diff --git a/location/tests/test.py b/location/tests/test.py index 8b7b0d3..e363199 100644 --- a/location/tests/test.py +++ b/location/tests/test.py @@ -9,7 +9,7 @@ from claim.test_helpers import create_test_claim_admin from django.core.cache import caches -from location.models import LocationManager +from location.models import LocationManager, UserDistrict from core.services import ( create_or_update_interactive_user, create_or_update_core_user, @@ -123,6 +123,9 @@ def test_allowed_location(self): ) cached = caches["location"].get(f"user_locations_{self.test_user._u.id}") self.assertIsNotNone(cached) + districts = UserDistrict.get_user_districts(self.test_user) + self.assertIsNotNone(cached) + def test_cache_invalidation(self): LocationManager().is_allowed(self.test_user, []) From 968c1582dfcaf0f5492d7e783bb4d3b173cb4acc Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 11:38:09 +0100 Subject: [PATCH 3/6] flake8 --- location/tests/test.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/location/tests/test.py b/location/tests/test.py index e363199..040bf0b 100644 --- a/location/tests/test.py +++ b/location/tests/test.py @@ -124,8 +124,8 @@ def test_allowed_location(self): cached = caches["location"].get(f"user_locations_{self.test_user._u.id}") self.assertIsNotNone(cached) districts = UserDistrict.get_user_districts(self.test_user) - self.assertIsNotNone(cached) - + self.assertIsNotNone(districts) + def test_cache_invalidation(self): LocationManager().is_allowed(self.test_user, []) From aa0e7d6170c613023318c6fdcc92190164eea825 Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 11:49:54 +0100 Subject: [PATCH 4/6] flake 8 --- location/models.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/location/models.py b/location/models.py index 2850abd..66ff4f1 100644 --- a/location/models.py +++ b/location/models.py @@ -1,5 +1,3 @@ -from functools import reduce -import django from django.core.cache import caches from django_redis.cache import RedisCache import uuid @@ -72,7 +70,6 @@ def parents(self, location_id, loc_type=None): ) return self.get_location_from_ids((parents), loc_type) if loc_type else parents - def allowed(self, user_id, loc_types=["R", "D", "W", "V"], strict=True, qs=False): strict_sql = """ AND ( @@ -267,7 +264,6 @@ def cache_location_graph_if_empty(): cache_location_graph() - def extend_allowed_locations(location_pks, strict=True, loc_types=None): """ Get underlying locations for given location PKs. From 870774086ee130600d1b0b0af0af95c17514d80e Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 11:51:21 +0100 Subject: [PATCH 5/6] flake 8 --- location/gql_mutations.py | 1 - location/models.py | 1 - location/tests/test.py | 6 ------ 3 files changed, 8 deletions(-) diff --git a/location/gql_mutations.py b/location/gql_mutations.py index da318cd..3474336 100644 --- a/location/gql_mutations.py +++ b/location/gql_mutations.py @@ -7,7 +7,6 @@ from django.core.exceptions import ValidationError, PermissionDenied from django.utils.translation import gettext as _ from graphene import InputObjectType -from django.core.cache import cache import copy diff --git a/location/models.py b/location/models.py index 66ff4f1..71f7005 100644 --- a/location/models.py +++ b/location/models.py @@ -671,7 +671,6 @@ def location_deleted(sender, instance, **kwargs): free_cache_for_user() - class OfficerVillage(core_models.VersionedModel): id = models.AutoField(db_column="OfficerVillageId", primary_key=True) officer = models.ForeignKey( diff --git a/location/tests/test.py b/location/tests/test.py index 040bf0b..8b80e30 100644 --- a/location/tests/test.py +++ b/location/tests/test.py @@ -10,11 +10,6 @@ from django.core.cache import caches from location.models import LocationManager, UserDistrict -from core.services import ( - create_or_update_interactive_user, - create_or_update_core_user, - create_or_update_user_districts, -) from core.utils import filter_validity from core.models.user import Role @@ -126,7 +121,6 @@ def test_allowed_location(self): districts = UserDistrict.get_user_districts(self.test_user) self.assertIsNotNone(districts) - def test_cache_invalidation(self): LocationManager().is_allowed(self.test_user, []) cached = caches["location"].get(f"user_locations_{self.test_user._u.id}") From 90ba2c123f3440b93743c073efcc262c727bc767 Mon Sep 17 00:00:00 2001 From: delcroip Date: Fri, 8 Nov 2024 12:00:04 +0100 Subject: [PATCH 6/6] naming --- location/models.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/location/models.py b/location/models.py index 71f7005..a4ebfaf 100644 --- a/location/models.py +++ b/location/models.py @@ -259,7 +259,7 @@ def cache_location_graph(location_id=None): cache.set("location_types", location_types, timeout=None) -def cache_location_graph_if_empty(): +def cache_location_if_not_cached(): if not cache.has_key("location_types"): cache_location_graph() @@ -273,7 +273,7 @@ def extend_allowed_locations(location_pks, strict=True, loc_types=None): logger.error( f"extend_allowed_locations is expecting a list but received {location_pks}" ) - cache_location_graph_if_empty() + cache_location_if_not_cached() graph = cache.get("location_graph") result_pks = set() to_visit = set(location_pks) @@ -590,11 +590,11 @@ def get_user_districts(cls, user): cachedata = cache.get(f"user_districts_{user.id}") districts = [] if cachedata is None: - cache_location_graph_if_empty() - cache_district = cache.get("location_types") + cache_location_if_not_cached() + cache_location_type = cache.get("location_types") cachedata = [] if user.is_superuser: - for loc in cache_district['D']: + for loc in cache_location_type['D']: cachedata.append([0, loc]) elif not isinstance(user, core_models.InteractiveUser): if isinstance(user, core_models.TechnicalUser):