Merge branch 'dev' into 'main'

0.3.1 Release

See merge request hris-sync/hris-integration!49

README.rst

@@ -55,6 +55,17 @@ settings.
 Release Notes
 =============
 
+0.3.1 - Import Issues
+---------------------
+
+This is a bug fix release. Importing employees would result in errors being thrown for
+saving phone numbers and addresses, due the fact that the pk of all models is now id.
+
+There is a known issues with the employee page for adding phone numbers and addresses, 
+then trying to set them as primary, which will cause that page to be reloaded as the
+submit hook is not registering correctly. To work around this issue, simply refresh the
+page after you have added and phone number or addresses, then check the primary checkbox.
+
 0.3.0 - Quality of Life improvements and Github release
 -------------------------------------------------------
 

hris_integration/common/functions.py

@@ -1,45 +1,85 @@
 # Copyright: (c) 2022, Josh Carswell <josh.carswell@thecarswells.ca>
-# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt) 
+# GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
 from django.db.utils import ProgrammingError
 from warnings import warn
 from django.conf import settings
 from random import choice
+from typing import List, Tuple
 
-def pk_to_name(pk:int) -> str:
-    if not isinstance(pk,int):
-        raise TypeError(f"Expected int got \"{pk.__class__.__name__}\"")
+
+def pk_to_name(pk: int) -> str:
+    """
+    Convert a primary key to a string name used for Django html forms. This is not used
+    for API's which use the primary key as the primary key.
+
+    :param pk: the primary key to convert
+    :type pk: int
+    :raises TypeError: if the primary key is not an integer
+    :return: django form formatted field name
+    :rtype: str
+    """
+    if not isinstance(pk, int):
+        raise TypeError(f'Expected int got "{pk.__class__.__name__}"')
 
     return f"id_{pk}"
 
-def name_to_pk(name:str) -> int:
-    if isinstance(name,int):
+
+def name_to_pk(name: str) -> int:
+    """
+    Converts a Django form field name to a primary key. Note if the name is not a valid
+    primary key field name, a ValueError will be raised when converting the string to a
+    integer.
+
+    :param name: The string name of the field
+    :type name: str
+    :raises TypeError: If the name is not a string or integer
+    :return: The primary key extracted from the name
+    :rtype: int
+    """
+    if isinstance(name, int):
         return name
-    elif isinstance(name,str):
-        return int(name.replace("id_",""))
+    elif isinstance(name, str):
+        return int(name.replace("id_", ""))
     else:
-        raise TypeError(f"Expected str got \"{name.__class__.__name__}\"")
+        raise TypeError(f'Expected str got "{name.__class__.__name__}"')
+
+
+def model_to_choices(
+    data: "django.db.model.queryset", none: bool = False
+) -> List[Tuple[str, object]]:
+    """
+    Converts a model queryset to a list of choices.
+
+    :param data: The queryset
+    :type data: django.db.model.queryset
+    :param none: Include any empty value at the start, defaults to False
+    :type none: bool, optional
+    :return: a Django choice list
+    :rtype: List[Tuple(str, object)]
+    """
 
-def model_to_choices(data,none:bool =False):
     output = []
     if none:
-        output.append((None,""))
+        output.append((None, ""))
     try:
         for r in data:
-            output.append((pk_to_name(r.pk),str(r)))
-    except (ProgrammingError,AttributeError):
-            warn("Databases not initialized")
-            output = [('Not Loaded','System not initalized')]
+            output.append((pk_to_name(r.pk), str(r)))
+    except (ProgrammingError, AttributeError):
+        warn("Databases not initialized")
+        output = [("Not Loaded", "System not initialized")]
+
     return output
 
-def password_generator(length:int = None, chars:str = None) -> str:
+
+def password_generator(length: int = None, chars: str = None) -> str:
     """A simple password generator. If no length or chars are provided, the default
     is used from the configuration.
 
-    REF: 
+    REF:
     - config.PASSWORD_LENGTH
     - config.PASSWORD_CHARS
-    
+
     :param length: The length of the password to generate
     :type length: int, optional
     :param chars: The characters to use when generating the password
@@ -52,9 +92,10 @@ def password_generator(length:int = None, chars:str = None) -> str:
         length = settings.PASSWORD_LENGTH
     if not chars:
         chars = settings.PASSWORD_CHARS
-    return ''.join(choice(chars) for _ in range(length))
+    return "".join(choice(chars) for _ in range(length))
+
 
-def get_model_pk_name(model) -> str:
+def get_model_pk_name(model: "django.models.Model") -> str:
     """Gets the model primary key field name
 
     :param model: the model to get the primary key field name from
@@ -65,4 +106,67 @@ def get_model_pk_name(model) -> str:
 
     for f in model._meta.fields:
         if f.primary_key:
-            return f.name
+            return f.name
+
+
+class PhoneNumber:
+    """
+    A phone number helper to ensure that number are properly formatted and provides
+    a method for pretty print.
+
+    Available methods:
+    - __str__: Returns the phone number as a string
+    - eq, lt, gt: Compare the phone number to another phone number
+
+    Available attributes:
+    - number: The phone number as a string
+    - pretty: Returns the phone number as a string with a pretty format
+    """
+
+    def __init__(self, number=str) -> None:
+        if isinstance(number, int):
+            self.number = str(number)
+        elif isinstance(number, str):
+            parse = []
+            for i in number:
+                if i.isdigit():
+                    parse.append(i)
+            self.number = "".join(number)
+        else:
+            raise ValueError(f"Expected a string got {number.__class__.__name__}")
+
+    def __str__(self) -> str:
+        return self.number
+
+    def __repr__(self) -> str:
+        return self.number
+
+    @property
+    def pretty(self) -> str:
+        return "%s%s%s-%s%s%s-%s%s%s%s" % tuple(self.number)
+
+    def __eq__(self, o: object) -> bool:
+        if isinstance(o, PhoneNumber):
+            return self.number == o.number
+        else:
+            return False
+
+    def __gt__(self, o: object) -> bool:
+        if isinstance(o, int):
+            return int(self.number) < o
+        elif isinstance(o, str):
+            o = PhoneNumber(o)
+        if isinstance(o, PhoneNumber):
+            return int(self.number) < int(o.number)
+        else:
+            return False
+
+    def __lt__(self, o: object) -> bool:
+        if isinstance(o, int):
+            return int(self.number) > o
+        elif isinstance(o, str):
+            o = PhoneNumber(o)
+        if isinstance(o, PhoneNumber):
+            return int(self.number) > int(o.number)
+        else:
+            return False

hris_integration/employee/models/base.py

@@ -203,3 +203,7 @@ def givenname(self) -> str:
     def surname(self) -> str:
         warn("surname is deprecated, use last_name instead", DeprecationWarning)
         return self.last_name
+
+    @property
+    def full_name(self) -> str:
+        return f"{self.first_name} {self.last_name}"

hris_integration/employee/models/information.py

@@ -26,7 +26,10 @@ class Meta:
 
     def __str__(self) -> str:
         s = ("%s%s%s-%s%s%s-%s%s%s%s" % tuple(self.number),)
-        return f"{str(self.employee)} - {self.label} {s}"
+        return f"Phone: {self.employee.full_name} - {self.label} {s}"
+
+    def __repr__(self) -> str:
+        return f"{self.__class__.__name__}.objects.get(id={self.id})"
 
     @property
     def phone_label(self) -> str:
@@ -85,7 +88,10 @@ class Meta:
     primary: bool = models.BooleanField(default=False)
 
     def __str__(self) -> str:
-        return f"{str(self.employee)} - {self.label}"
+        return f"Address: {self.employee.full_name} - {self.label}"
+
+    def __repr__(self) -> str:
+        return f"{self.__class__.__name__}.objects.get(id={self.id})"
 
     @property
     def address_label(self) -> str:

hris_integration/ftp_import/forms/ftp_import.py

@@ -1,6 +1,7 @@
 # Copyright: (c) 2022, Josh Carswell <josh.carswell@thecarswells.ca>
 # GNU General Public License v3.0+ (see COPYING or https://www.gnu.org/licenses/gpl-3.0.txt)
 
+from cProfile import label
 import datetime
 import logging
 import re
@@ -14,7 +15,7 @@
 from organization.models import JobRole, Location, BusinessUnit
 from django.db.utils import IntegrityError
 from django.db.models import Q
-from common.functions import get_model_pk_name
+from common.functions import get_model_pk_name, PhoneNumber
 
 
 from ftp_import.helpers import config
@@ -811,7 +812,7 @@ def save_employee_new(self) -> None:
             except IntegrityError:
                 logger.error(f"Failed to save {self.employee}")
 
-    def _get_phone(self) -> Phone:
+    def _get_phone(self, label: str = None) -> Phone:
         """
         Attempts to get the Phone object for the employee. If one does not exist,
         a new one will be created with the base fields set. If one exists, it will be returned
@@ -823,18 +824,26 @@ def _get_phone(self) -> Phone:
         """
 
         if self.employee.employee:
-            phones_no = Phone.objects.filter(employee=self.employee.employee)
+            if label:
+                phone = Phone.objects.filter(
+                    Q(employee=self.employee.employee) & Q(label=label)
+                )
+            else:
+                phone = Phone.objects.filter(employee=self.employee.employee)
         else:
             return KeyError("No mutable employee")
 
-        if len(phones_no) > 1:
+        if len(phone) > 1:
             raise Phone.MultipleObjectsReturned
-        elif len(phones_no) < 1:
-            addr = Phone()
-            addr.employee = self.employee.employee
-            return addr
+        elif len(phone) < 1:
+            phone = Phone()
+            phone.employee = self.employee.employee
+            phone.label = label
+            logger.debug(f"Created new phone object")
+            return phone
         else:
-            return phones_no[0]
+            logger.debug(f"Found existing phone {phone[0]}")
+            return phone[0]
 
     def save_phone(self):
         """Parse the kwargs for phone number fields and update the phone number object."""
@@ -843,28 +852,28 @@ def save_phone(self):
             # This is a pending employee so we can't save the phone number
             return
 
-        try:
-            phone = self._get_phone()
-        except Phone.MultipleObjectsReturned:
-            logger.warning(
-                "More than one phone number exists. Cowardly not doing anything"
-            )
-            return
-        except KeyError:
-            # This should only happen if the employee is not matched but the flag is set
-            return
+        field_label = self.get_field_name("phone_label")
+        field_phone = self.get_field_name("number")
 
-        for key, value in self.kwargs.items():
-            map_val = self.get_map_to(key)
-            if hasattr(phone, map_val) and value:
-                setattr(phone, map_val, value)
-                phone.label = key
+        if field_phone and field_phone in self.kwargs.keys():
+            try:
+                if field_label and field_label in self.kwargs.keys():
+                    phone = self._get_phone(self.kwargs[field_label])
+                else:
+                    phone = self._get_phone()
+            except (Phone.MultipleObjectsReturned, KeyError):
+                return
+
+            if field_label and field_label in self.kwargs.keys():
+                phone.label = self.kwargs[field_label]
+            else:
+                phone.label = field_phone
 
-        if phone.number:
-            phone.primary = False
+            phone.number = PhoneNumber(self.kwargs[field_phone]).number
+            logger.debug(f"Saving phone {phone}")
             phone.save()
 
-    def _get_address(self) -> Address:
+    def _get_address(self, label: str = None) -> Address:
         """
         Similar to _get_phone, but for addresses. This method also filters based on the
         "Imported Address" label, so it should always return a single address new or existing.
@@ -875,33 +884,41 @@ def _get_address(self) -> Address:
         """
 
         addrs = Address.objects.filter(
-            Q(employee=self.employee.employee) & Q(label="Imported Address")
+            Q(employee=self.employee.employee) & Q(label=label or "Imported Address")
         )
         if len(addrs) > 1:
             raise Address.MultipleObjectsReturned
         elif len(addrs) < 1:
             addr = Address()
             addr.employee = self.employee.employee
             addr.label = "Imported Address"
+            logger.debug(f"Created new address")
             return addr
         else:
+            logger.debug(f"Found existing address {addrs[0].id}")
             return addrs[0]
 
-    def save_address(self):
+    def save_address(self) -> None:
         """Parse the kwargs for address fields and updates the address object."""
 
         if self.employee.is_matched == False:
             # This is a pending employee so we can't save the address
             return
 
+        label = self.get_field_name("address_label")
+
         try:
-            address = self._get_address()
+            address = self._get_address(label)
         except Address.MultipleObjectsReturned:
-            logger.warning("More than one address exists. Cowardly not doing anything")
+            logger.debug("More than one address exists. Cowardly not doing anything")
+            return
+        except KeyError:
+            # This should only happen if the employee is not matched but the flag is set
+            return
 
         for key, value in self.kwargs.items():
             map_val = self.get_map_to(key)
-            if hasattr(address, map_val):
+            if map_val != "id" and hasattr(address, map_val):
                 if map_val[:6] == "street":
                     if isinstance(value, list):
                         for x in range(len(value) if len(value) < 3 else 3):