Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: profile api save extrainfo arabic fields #194

Merged
merged 8 commits into from
Jul 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 3 additions & 16 deletions eox_nelp/one_time_password/view_decorators.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)


Expand Down Expand Up @@ -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
53 changes: 51 additions & 2 deletions eox_nelp/tests/test_utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"))
33 changes: 33 additions & 0 deletions eox_nelp/user_profile/api/v1/tests/test_views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO we could remove this since the test purpose is to check if the extrainfo records was updated instead of checking if the CCD was called

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The profile API does the same, but I think that the cdd_task could also be verified.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 first_name has been updated.
- Check that user last_name has been updated.
- Check cdd_task async task is called with user.id

)
@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)
17 changes: 12 additions & 5 deletions eox_nelp/user_profile/api/v1/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -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__)

Expand Down Expand Up @@ -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)
Expand Down
20 changes: 20 additions & 0 deletions eox_nelp/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading