diff --git a/eox_nelp/programs/api/v1/tests/test_views.py b/eox_nelp/programs/api/v1/tests/test_views.py index dadec8b6..0d732a4d 100644 --- a/eox_nelp/programs/api/v1/tests/test_views.py +++ b/eox_nelp/programs/api/v1/tests/test_views.py @@ -386,73 +386,22 @@ def setUp(self): self.course1 = CourseOverview.objects.create(id=f"{self.BASE_COURSE_ID}1") self.course2 = CourseOverview.objects.create(id=f"{self.BASE_COURSE_ID}2") - @patch("eox_nelp.programs.api.v1.views.CourseDetailSerializer") - @patch("eox_nelp.programs.api.v1.utils.get_program_metadata") - def test_get_programs_list_authenticated( - self, - mock_get_program_metadata, - mock_course_serializer, - ): + def test_get_programs_list_authenticated_without_national_id(self): """ - Test GET returns program list for authenticated user. + Test GET returns program list for authenticated user. But with not national_id query param. Expected behavior: - - Status code 200. - - Response is a list of program dicts matching expected_data. + - Status code 400. + - Error message indicating that national_id is required. """ - serializer_side_effect = [] - for course_data in COURSE_API_SERIALIZER_DATA: - mock_serializer_instance = MagicMock() - mock_serializer_instance.data = course_data - serializer_side_effect.append(mock_serializer_instance) - mock_course_serializer.side_effect = serializer_side_effect - mock_get_program_metadata.return_value = { - "trainer_type": 10, - "type_of_activity": 165, - "mandatory": "01", - "program_approve": "00", - "program_code": "eltesst", + expected_data = { + "error": "MISSING_NATIONAL_ID", + "message": "national_id query parameter is required." } - expected_data = [ - { - "program_name": "testigngg", - "program_code": "eltesst", - "type_of_activity": "برنامج الاستثمار الأمثل (برامج قصيرة)", - "type_of_activity_id": 165, - "mandatory": "01", - "program_approve": "00", - "code": "course-v1:edx+cd101+2020323", - "date_start": "2030-01-01", - "date_end": None, - "date_start_hijri": '1451-08-26', - "date_end_hijri": None, - "duration": 1, - "training_location": "FutureX", - "trainer_type": 10, - "unit": "hour", - }, - { - "program_name": "small-graded", - "program_code": "eltesst", - "type_of_activity": "برنامج الاستثمار الأمثل (برامج قصيرة)", - "type_of_activity_id": 165, - "mandatory": "01", - "program_approve": "00", - "code": "course-v1:edx+cd101+2023-t1", - "date_start": "2020-01-01", - "date_end": "2034-12-25", - "date_start_hijri": "1441-05-06", - "date_end_hijri": "1456-10-14", - "duration": 2, - "training_location": "FutureX", - "trainer_type": 10, - "unit": "hour", - }, - ] response = self.client.get(self.url) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertListEqual(response.data["results"], expected_data) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertDictEqual(response.data, expected_data) @patch("eox_nelp.programs.api.v1.views.CourseDetailSerializer") @patch("eox_nelp.programs.api.v1.utils.get_program_metadata") @@ -547,10 +496,11 @@ def test_get_programs_list_unauthenticated_national_id(self): self.assertEqual(response.status_code, status.HTTP_401_UNAUTHORIZED) self.assertIn("Authentication credentials were not provided", str(response.data)) + @patch("eox_nelp.programs.api.v1.views.courses") @patch("eox_nelp.programs.api.v1.views.CourseDetailSerializer") @patch("eox_nelp.programs.api.v1.utils.get_program_metadata") def test_get_programs_list_missing_data( - self, mock_get_program_metadata, mock_course_serializer, + self, mock_get_program_metadata, mock_course_serializer, mock_courses ): """ Test GET returns error if ProgramLookupSerializer is invalid. @@ -559,7 +509,16 @@ def test_get_programs_list_missing_data( - Response result is alist. - Expected data matches with null values """ + user_by_national_id_instance, _ = User.objects.get_or_create(username="user3", password="pass3") + national_id = "1222888555" + ExtraInfo.objects.get_or_create( # pylint: disable=no-member + user=user_by_national_id_instance, + arabic_name="مسؤل", + national_id=national_id, + ) serializer_side_effect = [] + course = MagicMock(id="course-v1:edx+special+2024") + mock_courses.get_courses.return_value = [course, course] for course_data in COURSE_API_SERIALIZER_DATA: mock_serializer_instance = MagicMock() mock_serializer_instance.data = course_data @@ -603,7 +562,7 @@ def test_get_programs_list_missing_data( }, ] - response = self.client.get(self.url) + response = self.client.get(self.url, {"national_id": national_id}) self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertIsInstance(response.data["results"], list) diff --git a/eox_nelp/programs/api/v1/views.py b/eox_nelp/programs/api/v1/views.py index df31972f..b24bfb97 100644 --- a/eox_nelp/programs/api/v1/views.py +++ b/eox_nelp/programs/api/v1/views.py @@ -46,25 +46,32 @@ def wrapper(self, request, *args, **kwargs): return decorator -def process_national_id_query_param(): - """Decorator to check if national_id query parameter is valid if provided. - If not provided, the view will proceed without filtering by national_id. - Using the request user if not provided. Added to the request object the user_by_national_id attribute. +def require_national_id_query_param(): + """Decorator to check if national_id query parameter is valid and provided. + If not provided, the view will return a 400 error. + Added to the request object the user_by_national_id attribute. Returns 422 if invalid. """ def decorator(func): @wraps(func) def wrapper(self, request, *args, **kwargs): - if national_id := self.request.query_params.get("national_id"): - if not is_valid_national_id(national_id): - return Response( - { - "error": "INVALID_NATIONAL_ID", - "message": "national_id must be digits only and 10–15 characters." - }, - status=status.HTTP_422_UNPROCESSABLE_ENTITY, - ) - setattr(request, 'user_by_national_id', get_object_or_404(User, extrainfo__national_id=national_id)) + if not (national_id := self.request.query_params.get("national_id")): + return Response( + { + "error": "MISSING_NATIONAL_ID", + "message": "national_id query parameter is required.", + }, + status=status.HTTP_400_BAD_REQUEST, + ) + if not is_valid_national_id(national_id): + return Response( + { + "error": "INVALID_NATIONAL_ID", + "message": "national_id must be digits only and 10–15 characters." + }, + status=status.HTTP_422_UNPROCESSABLE_ENTITY, + ) + setattr(request, 'user_by_national_id', get_object_or_404(User, extrainfo__national_id=national_id)) return func(self, request, *args, **kwargs) return wrapper @@ -162,7 +169,7 @@ class ProgramsListView(CourseListView): permission_classes = [IsAuthenticated, ProgramsLookupPermission] serializer_class = ProgramLookupSerializer - @process_national_id_query_param() + @require_national_id_query_param() def get(self, request, *args, **kwargs): """Override get to apply some filtering and pre processing.""" return super().get(request, *args, **kwargs) @@ -193,29 +200,20 @@ def get_queryset(self): lazy sequence as used in https://github.com/openedx/edx-platform/blob/258f3fc/lms/djangoapps/course_api/api.py#L111 """ - if getattr(self.request, 'user_by_national_id', None): - visible_courses_queryset = courses.get_courses(user=self.request.user_by_national_id) - else: - visible_courses_queryset = super().get_queryset() + visible_courses_queryset = courses.get_courses(user=self.request.user_by_national_id) program_queryset = [] for course in visible_courses_queryset: course_data = CourseDetailSerializer(course, context={'request': self.request}).data program_lookup = get_program_lookup_representation(course_data) - program_queryset.append(program_lookup) + if CourseEnrollment.is_enrolled(self.request.user_by_national_id, program_lookup["code"]): + program_queryset.append(program_lookup) return program_queryset def filter_queryset(self, queryset): """ - Filter the queryset by course enrolled if the national_id query param is present. + Filter the queryset... Returns: - List of enrolled courses for the user qs + QuerySet: filtered queryset """ - if getattr(self.request, 'user_by_national_id', None): - queryset = [ - program_data for program_data in queryset if CourseEnrollment.is_enrolled( - self.request.user_by_national_id, - program_data["code"], - ) - ] return queryset