feat: profile api save extrainfo arabic fields (#194)

* feat: profile api save extranfo arabic fields

* refactor!: reduce complexity reorder for

fix C901 'update_user_data' is too complex (11)

* feat: dont import ExtraInfo in initialization

* feat: for improvement for user profile

* feat: rename setting and pylint

* feat: unpack test

* feat: suggestions from code review

Co-authored-by: Andrey Cañon <[email protected]>

* chore: remove unused import

---------

Co-authored-by: Andrey Cañon <[email protected]>

eox_nelp/one_time_password/view_decorators.py

@@ -7,12 +7,13 @@
 import functools
 import logging
 
-from custom_reg_form.models import ExtraInfo
 from django.conf import settings
 from django.core.cache import cache
 from django.http import HttpResponseForbidden, JsonResponse
 from rest_framework import status
 
+from eox_nelp.utils import save_extrainfo_field
+
 logger = logging.getLogger(__name__)
 
 
@@ -56,23 +57,9 @@ def wrapper(request):
             if not proposed_user_otp == cache.get(user_otp_key):
                 return HttpResponseForbidden(reason="Forbidden - wrong code")
 
-            save_successfull_phone_validation(request.user)
+            save_extrainfo_field(request.user, "is_phone_validated", True)
             cache.delete(user_otp_key)
 
         return func(request)
 
     return wrapper
-
-
-def save_successfull_phone_validation(user):
-    """Save in db the successfull phone_validation field with `True`
-    in extrainfo model of the user.
-
-    Args:
-        user (User): User instance to check
-    """
-    if extra_info := getattr(user, "extrainfo", None):
-        setattr(extra_info, "is_phone_validated", True)
-        extra_info.save()
-    else:
-        ExtraInfo.objects.create(user=user, is_phone_validated=True)  # pylint: disable=no-member

eox_nelp/tests/test_utils.py

@@ -4,12 +4,21 @@
     ExtractCourseIdFromStringTestCase: Tests cases for the extract_course_id_from_string method.
     GetCourseFromIdTestCase: Tests cases for the get_course_from_id method.
 """
-from ddt import data, ddt
+from ddt import data, ddt, unpack
+from django.contrib.auth import get_user_model
 from django.test import TestCase
 from mock import Mock, patch
 from opaque_keys.edx.keys import CourseKey
 
-from eox_nelp.utils import camel_to_snake, extract_course_id_from_string, get_course_from_id, get_item_label
+from eox_nelp.utils import (
+    camel_to_snake,
+    extract_course_id_from_string,
+    get_course_from_id,
+    get_item_label,
+    save_extrainfo_field,
+)
+
+User = get_user_model()
 
 
 @ddt
@@ -222,3 +231,43 @@ def test_invalid_input(self, input_value):
             - TypeError is raised
         """
         self.assertRaises(TypeError, camel_to_snake, input_value)
+
+
+@ddt
+class SaveExtraInfoFieldTestCase(TestCase):
+    """Test class for the save_extrainfo_field method."""
+    @data(
+        {"field": "arabic_name", "value": "أناكين سكاي ووكر"},
+        {"field": "is_phone_validated", "value": True},
+        {"field": "arabic_first_name", "value": " أناكين"},
+        {"field": "arabic_last_name", "value": "سكاي ووكر"},
+    )
+    @unpack
+    def test_save_extrainfo_field(self, field, value):
+        """ Test right functionality.
+
+        Expected behavior:
+            - Extrainfo related objed has  the expected value.
+        """
+        user, _ = User.objects.get_or_create(username="vader1798")
+
+        save_extrainfo_field(user, field, value)
+
+        self.assertEqual(getattr(user.extrainfo, field), value)
+
+    @data(
+        {"field": "arabic_name2", "value": "loool"},
+        {"field": "otp-crazy", "value": True},
+    )
+    @unpack
+    def test_wrong_extra_info_field(self, field, value):
+        """ Test when the input is not a extra info field.
+
+        Expected behavior:
+            - The user has no extra info model.
+        """
+        user, _ = User.objects.get_or_create(username="vader19")
+
+        save_extrainfo_field(user, field, value)
+
+        self.assertFalse(hasattr(user, "extrainfo"))

eox_nelp/user_profile/api/v1/tests/test_views.py

@@ -141,3 +141,36 @@ def test_account_update_extra_fields(self, cdd_task_mock):
         self.assertEqual(self.user.first_name, payload["first_name"])
         self.assertEqual(self.user.last_name, payload["last_name"])
         cdd_task_mock.delay.assert_called_with(user_id=self.user.id)
+
+    @override_settings(
+        ENABLE_OTP_VALIDATION=False,
+        USER_PROFILE_API_EXTRA_INFO_FIELDS=["arabic_first_name", "arabic_last_name"],
+        PEARSON_RTI_ACTIVATE_GRADED_GATE=True,
+    )
+    @patch("eox_nelp.user_profile.api.v1.views.cdd_task")
+    def test_update_extra_info_fields(self, cdd_task_mock):
+        """
+        Test that extra account user fields has been set.
+
+        Expected behavior:
+            - Check the response says that the field has been updated.
+            - Status code 200.
+            - Check that update_account_settings method has called once.
+            - Check that user extra info arabic_first_name has been updated.
+            - Check that user extra info arabic_last_name has been updated.
+        """
+        payload = {
+            "arabic_first_name": "أناكين",
+            "arabic_last_name": "سكاي ووكر",
+            "one_time_password": "correct26",
+        }
+        url_endpoint = reverse(self.reverse_viewname)
+
+        response = self.client.post(url_endpoint, payload, format="json")
+
+        self.assertDictEqual(response.json(), {"message": "User's fields has been updated successfully"})
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        accounts.api.update_account_settings.assert_called_once_with(self.user, payload)
+        self.assertEqual(self.user.extrainfo.arabic_first_name, payload["arabic_first_name"])
+        self.assertEqual(self.user.extrainfo.arabic_last_name, payload["arabic_last_name"])
+        cdd_task_mock.delay.assert_called_with(user_id=self.user.id)

eox_nelp/user_profile/api/v1/views.py

@@ -19,6 +19,7 @@
 from eox_nelp.edxapp_wrapper.user_api import accounts, errors
 from eox_nelp.one_time_password.view_decorators import validate_otp
 from eox_nelp.pearson_vue.tasks import cdd_task
+from eox_nelp.utils import save_extrainfo_field
 
 logger = logging.getLogger(__name__)
 
@@ -56,14 +57,20 @@ def update_user_data(request):
             # This extra code block is required since the method update_account_settings just
             # allows to update fields defined in the AccountUserSerializer and the AccountLegacyProfileSerializer
             # so some fields like first_name and last_name are not editable in the standad implementation.
+
             extra_account_user_fields = getattr(settings, "EXTRA_ACCOUNT_USER_FIELDS", [])
 
-            if extra_account_user_fields:
-                for field, value in request.data.items():
-                    if field in extra_account_user_fields and hasattr(request.user, field):
-                        setattr(request.user, field, value)
+            for field in extra_account_user_fields:
+                if (value := request.data.get(field)) and hasattr(request.user, field):
+                    setattr(request.user, field, value)
+                    request.user.save()
+            # Also some fields related ExtraInfo are not editable too  in the standard implementation. So we need
+            # save_extrainfo_field method with the desired settings.
+            required_user_extra_info_fields = getattr(settings, 'USER_PROFILE_API_EXTRA_INFO_FIELDS', [])
 
-                request.user.save()
+            for field in required_user_extra_info_fields:
+                if value := request.data.get(field):
+                    save_extrainfo_field(request.user, field, value)
 
     except errors.AccountValidationError as err:
         return Response({"field_errors": err.field_errors}, status=status.HTTP_400_BAD_REQUEST)

eox_nelp/utils.py

@@ -154,3 +154,23 @@ def camel_to_snake(string):
         String in snake case.
     """
     return re.sub(r'(?<!^)(?=[A-Z])', '_', string).lower()
+
+
+def save_extrainfo_field(user, field, value):
+    """Given a user save in extrainfo a value in the desired field.
+    If the extrainfo doesnt exist, the extrainfo model is created
+
+    Args:
+        user (User): user instace
+        field (string): extrainfo field to change
+        value (any): value set in extrainfo field
+    """
+    from custom_reg_form.models import ExtraInfo  # pylint: disable=import-outside-toplevel
+    if not hasattr(ExtraInfo, field):
+        return
+
+    if extra_info := getattr(user, "extrainfo", None):
+        setattr(extra_info, field, value)
+        extra_info.save()
+    else:
+        ExtraInfo.objects.create(user=user, **{field: value})  # pylint: disable=no-member