From 19bb78f1713fa7a49d5685fa8d9d26b5b12cd1ce Mon Sep 17 00:00:00 2001
From: Nabilah Adani <nabilahadanin@gmail.com>
Date: Mon, 5 Apr 2021 01:45:53 +0700
Subject: [PATCH] [REFACTOR] create validation for district and sub-district
 objects

---
 .../migrations/0008_auto_20210404_2332.py     | 33 +++++++++
 apps/accounts/models.py                       | 19 ++++-
 apps/accounts/tests/factories/accounts.py     | 11 ++-
 .../tests/test_units/test_accounts.py         | 73 ++++++++-----------
 4 files changed, 87 insertions(+), 49 deletions(-)
 create mode 100644 apps/accounts/migrations/0008_auto_20210404_2332.py

diff --git a/apps/accounts/migrations/0008_auto_20210404_2332.py b/apps/accounts/migrations/0008_auto_20210404_2332.py
new file mode 100644
index 0000000..2221dee
--- /dev/null
+++ b/apps/accounts/migrations/0008_auto_20210404_2332.py
@@ -0,0 +1,33 @@
+# Generated by Django 3.0.1 on 2021-04-04 16:32
+
+from django.db import migrations, models
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('accounts', '0007_auto_20210403_0653'),
+    ]
+
+    operations = [
+        migrations.AlterField(
+            model_name='account',
+            name='district',
+            field=models.CharField(choices=[('Beji', 'Beji'), ('Bojongsari', 'Bojongsari'), ('Cilodong', 'Cilodong'), ('Cimanggis', 'Cimanggis'), ('Cinere', 'Cinere'), ('Cipayung', 'Cipayung'), ('Limo', 'Limo'), ('Pancoran Mas', 'Pancoran Mas'), ('Sawangan', 'Sawangan'), ('Sukmajaya', 'Sukmajaya'), ('Tapos', 'Tapos')], max_length=128),
+        ),
+        migrations.AlterField(
+            model_name='account',
+            name='sub_district',
+            field=models.CharField(choices=[('Beji', 'Beji'), ('Beji Timur', 'Beji Timur'), ('Kemirimuka', 'Kemirimuka'), ('Kukusan', 'Kukusan'), ('Pondok Cina', 'Pondok Cina'), ('Tanah Baru', 'Tanah Baru'), ('Bojongsari Baru', 'Bojongsari Baru'), ('Bojongsari Lama', 'Bojongsari Lama'), ('Curug', 'Curug'), ('Duren Mekar', 'Duren Mekar'), ('Duren Seribu', 'Duren Seribu'), ('Pondok Petir', 'Pondok Petir'), ('Serua', 'Serua'), ('Cilodong', 'Cilodong'), ('Jatimulya', 'Jatimulya'), ('Kalibaru', 'Kalibaru'), ('Kalimulya', 'Kalimulya'), ('Sukamaju', 'Sukamaju'), ('Cisalak Pasar', 'Cisalak Pasar'), ('Curug', 'Curug'), ('Harjamukti', 'Harjamukti'), ('Mekarsari', 'Mekarsari'), ('Pasir Gunung Selatan', 'Pasir Gunung Selatan'), ('Tugu', 'Tugu'), ('Cinere', 'Cinere'), ('Gandul', 'Gandul'), ('Pangkalan Jati', 'Pangkalan Jati'), ('Pangkalan Jati Baru', 'Pangkalan Jati Baru'), ('Bojong Pondok Terong', 'Bojong Pondok Terong'), ('Cipayung', 'Cipayung'), ('Cipayung Jaya', 'Cipayung Jaya'), ('Pondok Jaya', 'Pondok Jaya'), ('Ratujaya', 'Ratujaya'), ('Grogol', 'Grogol'), ('Krukut', 'Krukut'), ('Limo', 'Limo'), ('Meruyung', 'Meruyung'), ('Depok Jaya', 'Depok'), ('Depok Jaya', 'Depok Jaya'), ('Mampang', 'Mampang'), ('Pancoran Mas', 'Pancoran Mas'), ('Rangkapan Jaya', 'Rangkapan Jaya'), ('Rangkapan Jaya Baru', 'Rangkapan Jaya Baru'), ('Bedahan', 'Bedahan'), ('Cinangka', 'Cinangka'), ('Kedaung', 'Kedaung'), ('Pasir Putih', 'Pasir Putih'), ('Pengasinan', 'Pengasinan'), ('Sawangan Baru', 'Sawangan Baru'), ('Sawangan Lama', 'Sawangan Lama'), ('Abadijaya', 'Abadijaya'), ('Bakti Jaya', 'Bakti Jaya'), ('Cisalak', 'Cisalak'), ('Mekar Jaya', 'Mekar Jaya'), ('Sukamaju', 'Sukmajaya'), ('Tirtajaya', 'Tirtajaya'), ('Cilangkap', 'Cilangkap'), ('Cimpaeun', 'Cimpaeun'), ('Jatijajar', 'Jatijajar'), ('Leuwinanggung', 'Leuwinanggung'), ('Sukamaju Baru', 'Sukamaju Baru'), ('Sukatani', 'Sukatani'), ('Tapos', 'Tapos')], max_length=128),
+        ),
+        migrations.AlterField(
+            model_name='accounthistory',
+            name='district',
+            field=models.CharField(choices=[('Beji', 'Beji'), ('Bojongsari', 'Bojongsari'), ('Cilodong', 'Cilodong'), ('Cimanggis', 'Cimanggis'), ('Cinere', 'Cinere'), ('Cipayung', 'Cipayung'), ('Limo', 'Limo'), ('Pancoran Mas', 'Pancoran Mas'), ('Sawangan', 'Sawangan'), ('Sukmajaya', 'Sukmajaya'), ('Tapos', 'Tapos')], max_length=128),
+        ),
+        migrations.AlterField(
+            model_name='accounthistory',
+            name='sub_district',
+            field=models.CharField(choices=[('Beji', 'Beji'), ('Beji Timur', 'Beji Timur'), ('Kemirimuka', 'Kemirimuka'), ('Kukusan', 'Kukusan'), ('Pondok Cina', 'Pondok Cina'), ('Tanah Baru', 'Tanah Baru'), ('Bojongsari Baru', 'Bojongsari Baru'), ('Bojongsari Lama', 'Bojongsari Lama'), ('Curug', 'Curug'), ('Duren Mekar', 'Duren Mekar'), ('Duren Seribu', 'Duren Seribu'), ('Pondok Petir', 'Pondok Petir'), ('Serua', 'Serua'), ('Cilodong', 'Cilodong'), ('Jatimulya', 'Jatimulya'), ('Kalibaru', 'Kalibaru'), ('Kalimulya', 'Kalimulya'), ('Sukamaju', 'Sukamaju'), ('Cisalak Pasar', 'Cisalak Pasar'), ('Curug', 'Curug'), ('Harjamukti', 'Harjamukti'), ('Mekarsari', 'Mekarsari'), ('Pasir Gunung Selatan', 'Pasir Gunung Selatan'), ('Tugu', 'Tugu'), ('Cinere', 'Cinere'), ('Gandul', 'Gandul'), ('Pangkalan Jati', 'Pangkalan Jati'), ('Pangkalan Jati Baru', 'Pangkalan Jati Baru'), ('Bojong Pondok Terong', 'Bojong Pondok Terong'), ('Cipayung', 'Cipayung'), ('Cipayung Jaya', 'Cipayung Jaya'), ('Pondok Jaya', 'Pondok Jaya'), ('Ratujaya', 'Ratujaya'), ('Grogol', 'Grogol'), ('Krukut', 'Krukut'), ('Limo', 'Limo'), ('Meruyung', 'Meruyung'), ('Depok Jaya', 'Depok'), ('Depok Jaya', 'Depok Jaya'), ('Mampang', 'Mampang'), ('Pancoran Mas', 'Pancoran Mas'), ('Rangkapan Jaya', 'Rangkapan Jaya'), ('Rangkapan Jaya Baru', 'Rangkapan Jaya Baru'), ('Bedahan', 'Bedahan'), ('Cinangka', 'Cinangka'), ('Kedaung', 'Kedaung'), ('Pasir Putih', 'Pasir Putih'), ('Pengasinan', 'Pengasinan'), ('Sawangan Baru', 'Sawangan Baru'), ('Sawangan Lama', 'Sawangan Lama'), ('Abadijaya', 'Abadijaya'), ('Bakti Jaya', 'Bakti Jaya'), ('Cisalak', 'Cisalak'), ('Mekar Jaya', 'Mekar Jaya'), ('Sukamaju', 'Sukmajaya'), ('Tirtajaya', 'Tirtajaya'), ('Cilangkap', 'Cilangkap'), ('Cimpaeun', 'Cimpaeun'), ('Jatijajar', 'Jatijajar'), ('Leuwinanggung', 'Leuwinanggung'), ('Sukamaju Baru', 'Sukamaju Baru'), ('Sukatani', 'Sukatani'), ('Tapos', 'Tapos')], max_length=128),
+        ),
+    ]
diff --git a/apps/accounts/models.py b/apps/accounts/models.py
index a4973f5..e6b5f59 100644
--- a/apps/accounts/models.py
+++ b/apps/accounts/models.py
@@ -2,18 +2,20 @@ import uuid
 from django.contrib.auth.models import User
 from django.db import models
 from django.core.validators import RegexValidator
+from django.utils.translation import gettext_lazy as _
 
 from apps.commons.managers import SoftDeleteManager
 from apps.commons.models import HistoryEnabledModel, HistoryModel
 
+from apps.cases.constants import ( DISTRICT_CHOICES, SUB_DISTRICT_CHOICES, SUB_DISTRICT_MAPPING )
 
 class AccountHistory(HistoryModel):
     user_id = models.IntegerField(null=False)
     name = models.CharField(max_length=128)
     email = models.EmailField(max_length=128)
     phone_number = models.CharField(max_length=64)
-    district = models.CharField(max_length=128, default="")
-    sub_district = models.CharField(max_length=128)
+    district = models.CharField(max_length=128, choices=DISTRICT_CHOICES)
+    sub_district = models.CharField(max_length=128, choices=SUB_DISTRICT_CHOICES)
     is_admin = models.BooleanField(default=False)
     is_verified = models.BooleanField(default=False)
     is_active = models.BooleanField(default=False)
@@ -28,6 +30,10 @@ class AccountHistory(HistoryModel):
 
     def __str__(self):
         return f"[History] Rev. {self.revision_id} - {self.name}"
+    
+    def clean(self):
+        if self.sub_district not in SUB_DISTRICT_MAPPING[self.district]:
+            raise ValidationError(_('Inconsistent district and sub district value'))
 
 
 class Account(HistoryEnabledModel):
@@ -43,8 +49,8 @@ class Account(HistoryEnabledModel):
             ),
         ]
     )
-    district = models.CharField(max_length=128, default="")
-    sub_district = models.CharField(max_length=128)
+    district = models.CharField(max_length=128, choices=DISTRICT_CHOICES)
+    sub_district = models.CharField(max_length=128, choices=SUB_DISTRICT_CHOICES)
     is_admin = models.BooleanField(default=False)
     is_verified = models.BooleanField(default=False)
     is_active = models.BooleanField(default=False)
@@ -69,3 +75,8 @@ class Account(HistoryEnabledModel):
     def __str__(self):
         account_role = "[Admin]" if self.is_admin else "[Officer]"
         return f"{account_role} {self.user.username}"
+
+    def clean(self):
+        if self.sub_district not in SUB_DISTRICT_MAPPING[self.district]:
+            raise ValidationError(_('Inconsistent district and sub district value'))
+
diff --git a/apps/accounts/tests/factories/accounts.py b/apps/accounts/tests/factories/accounts.py
index e369632..6f2fcf8 100644
--- a/apps/accounts/tests/factories/accounts.py
+++ b/apps/accounts/tests/factories/accounts.py
@@ -1,13 +1,16 @@
 from apps.accounts.models import Account
+from apps.cases.constants import (
+    DISTRICT_CHOICES,
+    SUB_DISTRICT_MAPPING
+)
 
 from django.contrib.auth.models import User
 from faker import Faker
 import factory
-
+import random
 
 faker = Faker()
 
-
 class UserFactory(factory.DjangoModelFactory):
     class Meta:
         model = User
@@ -26,7 +29,7 @@ class AccountFactory(factory.DjangoModelFactory):
     name = faker.name()
     email = faker.email()
     phone_number = "+999999999999"
-    district = faker.state()
-    sub_district = faker.city()
+    district = random.choice(DISTRICT_CHOICES)[0]
+    sub_district = factory.LazyAttribute(lambda o: random.choice(SUB_DISTRICT_MAPPING[o.district]))
     is_active = True
     is_verified = True
diff --git a/apps/accounts/tests/test_units/test_accounts.py b/apps/accounts/tests/test_units/test_accounts.py
index 289fc72..5c55552 100644
--- a/apps/accounts/tests/test_units/test_accounts.py
+++ b/apps/accounts/tests/test_units/test_accounts.py
@@ -1,4 +1,5 @@
 import json
+import random
 from faker import Faker
 from django.urls import reverse
 from django.core import mail
@@ -14,6 +15,10 @@ from apps.constants import (
     ACTIVITY_TYPE_EDIT,
     ACTIVITY_TYPE_DELETE,
 )
+from apps.cases.constants import (
+    DISTRICT_CHOICES,
+    SUB_DISTRICT_MAPPING
+)
 
 
 class AccountViewTest(APITestCase):
@@ -86,13 +91,13 @@ class AccountViewTest(APITestCase):
         admin_prev_count = Account.objects.filter(is_admin=True).count()
 
         data = {
-            "name": self.faker.name(),
+            "name": self.admin.name,
             "username": _account_id,
             "password": "justpass",
             "email": _account_id,
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.admin.district,
+            "sub_district": self.admin.sub_district,
             "is_admin": True,
         }
 
@@ -109,13 +114,13 @@ class AccountViewTest(APITestCase):
         officer_prev_count = Account.objects.filter(is_admin=False).count()
 
         data = {
-            "name": self.faker.name(),
+            "name": self.officer.name,
             "username": _account_id,
             "password": "justpass",
             "email": _account_id,
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": True,
             "is_active": True,
@@ -149,13 +154,13 @@ class AccountViewTest(APITestCase):
         officer_prev_count = Account.objects.filter(is_admin=False).count()
 
         data = {
-            "name": self.faker.name(),
+            "name": self.officer.name,
             "username": _account_id,
             "password": "justpass",
             "email": _account_id,
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": False,
             "is_active": False,
@@ -170,13 +175,13 @@ class AccountViewTest(APITestCase):
     def test_create_existing_user_fails(self):
         url = self.BASE_URL
         data = {
-            "name": self.faker.name(),
+            "name": self.officer.name,
             "username": "user_1",
             "password": "justpass",
-            "email": self.faker.email(),
+            "email": self.officer.email,
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": True,
             "is_active": True,
@@ -189,13 +194,13 @@ class AccountViewTest(APITestCase):
         _account_id = self.faker.email()
 
         data = {
-            "name": self.faker.name(),
+            "name": self.officer.name,
             "email": _account_id,
             "username": _account_id,
             "password": "12345678",
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
         }
         response = self.client.post(path=url, data=data, format="json",)
@@ -206,11 +211,11 @@ class AccountViewTest(APITestCase):
 
         data = {
             "id": str(self.officer.id),
-            "name": self.faker.name(),
-            "email": self.faker.email(),
+            "name": self.officer.name,
+            "email": self.officer.email,
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": True,
             "is_active": True,
@@ -248,8 +253,8 @@ class AccountViewTest(APITestCase):
             "name": self.faker.name(),
             "email": self.faker.email(),
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": True,
             "is_active": True,
@@ -272,8 +277,8 @@ class AccountViewTest(APITestCase):
         data = {
             "id": str(self.officer.id),
             "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
         }
 
         response = self.client.put(path=url, data=data, format="json",)
@@ -282,27 +287,13 @@ class AccountViewTest(APITestCase):
     def test_edit_account_fail_with_wrong_email_or_phone_format(self):
         url = self.BASE_URL + str(self.officer.id) + "/"
 
-        data = {
-            "id": str(self.officer.id),
-            "name": self.faker.name(),
-            "email": self.faker.email(),
-            "phone_number": "+999aaa999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
-            "is_admin": False,
-            "is_verified": True,
-            "is_active": True,
-        }
-
-        response = self.client.put(path=url, data=data, format="json",)
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
         data = {
             "id": str(self.officer.id),
             "name": self.faker.name(),
             "email": "email",
-            "phone_number": "+999999999999",
-            "district": self.faker.state(),
-            "sub_district": self.faker.city(),
+            "phone_number": "+9999999aaa99999",
+            "district": self.officer.district,
+            "sub_district": self.officer.sub_district,
             "is_admin": False,
             "is_verified": True,
             "is_active": True,
-- 
GitLab