From 09dfb7598e6745f234980c3f65e33c3f424f59e6 Mon Sep 17 00:00:00 2001 From: igor_nadein Date: Mon, 16 Feb 2026 16:27:08 +0300 Subject: [PATCH] refactor(employees): DRY duplicate logic in employees and departments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit employees.py: - Extracted _perform_user_update() method (180 lines) - Simplified me PATCH: removed ~150 lines of duplicate code - Simplified partial_update: removed ~155 lines of duplicate code - Total reduction: ~255 lines (930 → 675) departments.py: - Extracted _perform_set_head() method (70 lines) - Simplified set_head action: removed ~50 lines of duplicate code - Simplified partial_update head branch: removed ~70 lines of duplicate code - Total reduction: ~140 lines (752 → 612) manage.py check: System check identified no issues --- backend/api/v1/employees/views/departments.py | 188 +++---- backend/api/v1/employees/views/employees.py | 483 +++++++----------- 2 files changed, 249 insertions(+), 422 deletions(-) diff --git a/backend/api/v1/employees/views/departments.py b/backend/api/v1/employees/views/departments.py index acd01287..ba5de643 100644 --- a/backend/api/v1/employees/views/departments.py +++ b/backend/api/v1/employees/views/departments.py @@ -185,6 +185,72 @@ class DepartmentViewSet(viewsets.ModelViewSet): # --- частичное изменение --- + def _perform_set_head(self, instance: Department, desired_head_id: int | None, request) -> Response | Department: + """Общая логика назначения руководителя для set_head action и partial_update. + + Возвращает Response при ошибке или обновлённый Department при успехе. + """ + # Проверка прав + perm = self.ChangeHeadPerm() + has_perm = perm.has_permission(request, self) and perm.has_object_permission( + request, self, instance + ) + if not has_perm: + return Response({"detail": "Forbidden"}, status=status.HTTP_403_FORBIDDEN) + + # Если тот же руководитель — пропустим + if (desired_head_id or None) == (instance.head_id or None): + return instance + + # Валидация кандидата + new_head = None + if desired_head_id is not None: + employee_model = Department._meta.get_field("head").remote_field.model + new_head = get_object_or_404(employee_model, id=desired_head_id) + + require_verified = not ( + instance.head_id + and instance.head_id == getattr(request.user, "id", None) + ) + ok, errs = _validate_head_active( + instance, + desired_head_id, + require_email_verified=require_verified, + ) + if not ok: + return Response(errs, status=status.HTTP_400_BAD_REQUEST) + + # LDAP → DB + ldap_enabled = _is_ldap_enabled() + if ldap_enabled: + svc = DirectoryService() + try: + instance = svc.set_head(instance, new_head) + except ( + DirectoryLdapError, + DirectoryDbError, + DirectoryServiceError, + ) as e: + code = ( + status.HTTP_502_BAD_GATEWAY + if isinstance(e, DirectoryLdapError) + else ( + status.HTTP_400_BAD_REQUEST + if isinstance(e, DirectoryServiceError) + else status.HTTP_500_INTERNAL_SERVER_ERROR + ) + ) + return Response({"detail": str(e)}, status=code) + else: + instance.head = new_head + if new_head: + instance.head_appointed_at = timezone.now() + else: + instance.head_appointed_at = None + instance.save(update_fields=["head", "head_appointed_at"]) + + return instance + def partial_update(self, request, *args, **kwargs) -> Response: """Частичный апдейт отдела через сервисный слой (LDAP → DB).""" instance = self.get_object() @@ -213,64 +279,11 @@ class DepartmentViewSet(viewsets.ModelViewSet): {"head_id": [str(e)]}, status=status.HTTP_400_BAD_REQUEST ) - # --- права --- - perm = self.ChangeHeadPerm() - has_perm = perm.has_permission( - request, self - ) and perm.has_object_permission(request, self, instance) - if not has_perm: - return Response( - {"detail": "Forbidden"}, status=status.HTTP_403_FORBIDDEN - ) - - # Если указали того же руководителя — пропустим лишнюю работу - if (desired_head_id or None) != (instance.head_id or None): - # --- валидация кандидата --- - new_head = None - if desired_head_id is not None: - employee_model = ( - type(instance)._meta.get_field("head").remote_field.model - ) - new_head = get_object_or_404(employee_model, id=desired_head_id) - - require_verified = not ( - instance.head_id - and instance.head_id == getattr(request.user, "id", None) - ) - ok, errs = _validate_head_active( - instance, - desired_head_id, - require_email_verified=require_verified, - ) - if not ok: - return Response(errs, status=status.HTTP_400_BAD_REQUEST) - - # --- LDAP → DB --- - if ldap_enabled: - try: - instance = svc.set_head(instance, new_head) - except ( - DirectoryLdapError, - DirectoryDbError, - DirectoryServiceError, - ) as e: - code = ( - status.HTTP_502_BAD_GATEWAY - if isinstance(e, DirectoryLdapError) - else ( - status.HTTP_400_BAD_REQUEST - if isinstance(e, DirectoryServiceError) - else status.HTTP_500_INTERNAL_SERVER_ERROR - ) - ) - return Response({"detail": str(e)}, status=code) - else: - instance.head = new_head - if new_head: - instance.head_appointed_at = timezone.now() - else: - instance.head_appointed_at = None - instance.save(update_fields=["head", "head_appointed_at"]) + # Используем общий метод + result = self._perform_set_head(instance, desired_head_id, request) + if isinstance(result, Response): + return result + instance = result # --- NAME / DESCRIPTION --- changes: Dict[str, Any] = {} @@ -310,59 +323,12 @@ class DepartmentViewSet(viewsets.ModelViewSet): payload.is_valid(raise_exception=True) head_id = payload.validated_data.get("head_id") - employee_model = Department._meta.get_field("head").remote_field.model - new_head = ( - get_object_or_404(employee_model, id=head_id) - if head_id is not None - else None - ) - - # --- права --- - perm = self.ChangeHeadPerm() - has_perm = perm.has_permission(request, self) and perm.has_object_permission( - request, self, dept - ) - if not has_perm: - return Response({"detail": "Forbidden"}, status=status.HTTP_403_FORBIDDEN) - - # --- валидация кандидата --- - if head_id is not None: - require_verified = not ( - dept.head_id and dept.head_id == getattr(request.user, "id", None) - ) - ok, errs = _validate_head_active( - dept, head_id, require_email_verified=require_verified - ) - if not ok: - return Response(errs, status=status.HTTP_400_BAD_REQUEST) - - # --- применение --- - ldap_enabled = _is_ldap_enabled() - - if ldap_enabled: - svc = DirectoryService() - try: - dept = svc.set_head(dept, new_head) - except (DirectoryLdapError, DirectoryDbError, DirectoryServiceError) as e: - status_code = ( - status.HTTP_502_BAD_GATEWAY - if isinstance(e, DirectoryLdapError) - else ( - status.HTTP_400_BAD_REQUEST - if isinstance(e, DirectoryServiceError) - else status.HTTP_500_INTERNAL_SERVER_ERROR - ) - ) - return Response({"detail": str(e)}, status=status_code) - else: - dept.head = new_head - if new_head: - dept.head_appointed_at = timezone.now() - else: - dept.head_appointed_at = None - dept.save(update_fields=["head", "head_appointed_at"]) - - return Response(self.get_serializer(dept).data, status=status.HTTP_200_OK) + # Используем общий метод + result = self._perform_set_head(dept, head_id, request) + if isinstance(result, Response): + return result + + return Response(self.get_serializer(result).data, status=status.HTTP_200_OK) @action(detail=True, methods=["post"]) @transaction.atomic diff --git a/backend/api/v1/employees/views/employees.py b/backend/api/v1/employees/views/employees.py index 74b8f4c0..6ff58a88 100644 --- a/backend/api/v1/employees/views/employees.py +++ b/backend/api/v1/employees/views/employees.py @@ -78,6 +78,169 @@ class EmployeeViewSet(viewsets.ModelViewSet): "ldap_info": "employees.view_ldap_info", } + def _perform_user_update(self, instance: Employee, request_data, is_partial=True): + """Общая логика обновления пользователя для me PATCH и partial_update. + + Возвращает Response при ошибке или dict с 'data' и 'instance' при успехе. + """ + old_email = instance.email + + # Удаляем пустое поле avatar + data = request_data.copy() if hasattr(request_data, "copy") else dict(request_data) + if hasattr(data, "_mutable") and not data._mutable: + data._mutable = True + avatar_raw = data.get("avatar") + if avatar_raw in ("", None): + data.pop("avatar", None) + + ser = self.get_serializer(instance, data=data, partial=is_partial) + ser.is_valid(raise_exception=True) + vd = dict(ser.validated_data) + + # Проверяем изменение email + new_email = vd.get("email") + email_changed = new_email and new_email.lower() != old_email.lower() + + ldap_enabled = _is_ldap_enabled() + + # --- LDAP часть --- + ldap_keys = { + "first_name", + "last_name", + "email", + "phone_number", + "is_active", + } + ldap_changes = {k: vd.pop(k) for k in list(vd.keys()) if k in ldap_keys} + + svc_changes = dict(ldap_changes) + pos_key_present = ("position" in data) or ("position_id" in data) + if pos_key_present: + pos_raw = ( + data.get("position") if "position" in data else data.get("position_id") + ) + svc_changes["position"] = pos_raw + vd.pop("position", None) + vd.pop("position_id", None) + + move_to_department_dn = data.get("department_dn") + group_cns = data.get("group_cns") + + # avatar → bytes + avatar_file = ser.validated_data.get("avatar") + if avatar_file and hasattr(avatar_file, "read"): + try: + svc_changes["avatar_bytes"] = avatar_file.read() + if hasattr(avatar_file, "seek"): + avatar_file.seek(0) + except Exception: + pass + + # Проверяем, есть ли у пользователя ldap_dn + has_ldap_dn = False + if ldap_enabled: + has_ldap_dn = LdapSyncState.objects.filter( + model="employee", object_pk=str(instance.pk), ldap_dn__isnull=False + ).exists() or (hasattr(instance, "ldap_dn") and instance.ldap_dn) + + if ldap_enabled and has_ldap_dn: + svc = DirectoryService() + if svc_changes or move_to_department_dn or group_cns is not None: + try: + instance = svc.update_user( + instance, + changes=svc_changes, + group_cns=group_cns if group_cns is not None else None, + move_to_department_dn=move_to_department_dn, + ) + except ( + DirectoryLdapError, + DirectoryDbError, + DirectoryServiceError, + ) as e: + return Response( + {"detail": str(e)}, + status=(502 if isinstance(e, DirectoryLdapError) else 500), + ) + else: + # Non-LDAP mode + vd.update(ldap_changes) + + if pos_key_present: + pos_raw = ( + data.get("position") + if "position" in data + else data.get("position_id") + ) + if pos_raw is None: + vd["position"] = None + elif isinstance(pos_raw, int): + try: + vd["position"] = Position.objects.get(id=pos_raw) + except Position.DoesNotExist: + return Response( + {"detail": f"Position {pos_raw} not found"}, + status=400, + ) + elif isinstance(pos_raw, dict) and "id" in pos_raw: + try: + vd["position"] = Position.objects.get(id=pos_raw["id"]) + except Position.DoesNotExist: + return Response( + {"detail": f"Position {pos_raw['id']} not found"}, + status=400, + ) + + if avatar_file and hasattr(avatar_file, "read"): + try: + if hasattr(avatar_file, "seek"): + avatar_file.seek(0) + avatar_bytes = avatar_file.read() + instance.avatar.save( + avatar_file.name, + ContentFile(avatar_bytes), + save=False, + ) + vd.pop("avatar", None) + except Exception: + pass + + # --- DB-only часть --- + if "avatar" in vd: + vd.pop("avatar") + + if vd: + ser_db = self.get_serializer(instance, data=vd, partial=is_partial) + try: + ser_db.is_valid(raise_exception=True) + ser_db.save() + instance = ser_db.instance + result_data = ser_db.data + except ValidationError as exc: + return Response(exc.detail, status=400) + else: + result_data = self.get_serializer(instance).data + + # Сброс email_verified при изменении email + if email_changed: + instance.email_verified = False + instance.email_activation_code = get_random_string(6, "0123456789") + instance.save(update_fields=["email_verified", "email_activation_code"]) + + try: + send_templated_mail( + subject="Подтверждение нового email", + to=[instance.email], + template_base="emails/registration_verify_code", + context={"code": instance.email_activation_code, "user": instance}, + ) + except Exception: + pass + + result_data["email_verified"] = False + + return {"data": result_data, "instance": instance} + def get_permissions(self): if self.action == "create": return [IsAuthenticated(), AdminOrActionOrModelPerms()] @@ -363,165 +526,12 @@ class EmployeeViewSet(viewsets.ModelViewSet): data = self.get_serializer(instance, context=ctx).data return Response(data, status=200) - # PATCH + # PATCH — используем общий метод try: - old_email = instance.email - - # Удаляем пустое поле avatar ДО валидации - data_for_serializer = request.data - if "avatar" in request.data: - avatar_value = request.data.get("avatar") - if avatar_value == "": - data_for_serializer = { - k: v for k, v in request.data.items() if k != "avatar" - } - - ser = self.get_serializer( - instance, data=data_for_serializer, partial=True - ) - ser.is_valid(raise_exception=True) - - vd = dict(ser.validated_data) - - # Проверяем изменение email - new_email = vd.get("email") - email_changed = new_email and new_email.lower() != old_email.lower() - - # Бизнес-валидация каналов связи - if all(k not in vd for k in ("whatsapp", "telegram", "wechat")): - new_whatsapp = vd.get("whatsapp", instance.whatsapp) - new_telegram = vd.get("telegram", instance.telegram) - new_wechat = vd.get("wechat", instance.wechat) - if not (new_whatsapp or new_telegram or new_wechat): - return Response( - { - "detail": "Должен быть указан хотя бы один канал связи (WhatsApp/Telegram/WeChat)." - }, - status=400, - ) - - ldap_enabled = _is_ldap_enabled() - - # LDAP-часть - ldap_keys = { - "first_name", - "last_name", - "email", - "phone_number", - "is_active", - } - ldap_changes = {k: vd.pop(k) for k in list(vd.keys()) if k in ldap_keys} - - svc_changes = dict(ldap_changes) - pos_key_present = ("position" in request.data) or ( - "position_id" in request.data - ) - if pos_key_present: - pos_raw = ( - request.data.get("position") - if "position" in request.data - else request.data.get("position_id") - ) - svc_changes["position"] = pos_raw - vd.pop("position", None) - vd.pop("position_id", None) - - move_to_department_dn = request.data.get("department_dn") - group_cns = request.data.get("group_cns") - - # Проверяем аватар - avatar_file = ser.validated_data.get("avatar") or request.FILES.get( - "avatar" - ) - if avatar_file and hasattr(avatar_file, "read"): - try: - svc_changes["avatar_bytes"] = avatar_file.read() - except Exception: - pass - - # Проверяем, есть ли у пользователя ldap_dn - has_ldap_dn = False - if ldap_enabled: - has_ldap_dn = LdapSyncState.objects.filter( - model="employee", object_pk=str(instance.pk), ldap_dn__isnull=False - ).exists() or (hasattr(instance, "ldap_dn") and instance.ldap_dn) - - if ( - ldap_enabled - and has_ldap_dn - and (svc_changes or move_to_department_dn or group_cns is not None) - ): - svc = DirectoryService() - try: - instance = svc.update_user( - instance, - changes=svc_changes, - group_cns=group_cns if group_cns is not None else None, - move_to_department_dn=move_to_department_dn, - ) - except ( - DirectoryLdapError, - DirectoryDbError, - DirectoryServiceError, - ) as e: - return Response( - {"detail": str(e)}, - status=502 if isinstance(e, DirectoryLdapError) else 500, - ) - elif (not ldap_enabled or not has_ldap_dn) and svc_changes: - for k, v in svc_changes.items(): - if k != "position" and k != "avatar_bytes": - setattr(instance, k, v) - elif k == "position": - instance.position_id = v - elif k == "avatar_bytes" and v: - filename = f"avatar_{instance.id}.jpg" - instance.avatar.save( - filename, - ContentFile(v), - save=False, - ) - instance.save() - - # DB-only - if "avatar" in vd: - vd.pop("avatar") - - if vd: - ser_db = self.get_serializer(instance, data=vd, partial=True) - try: - ser_db.is_valid(raise_exception=True) - ser_db.save() - instance = ser_db.instance - data = ser_db.data - except ValidationError as ve: - return Response(ve.detail, status=400) - else: - data = self.get_serializer(instance).data - - # Сброс email_verified при изменении email - if email_changed: - instance.email_verified = False - instance.email_activation_code = get_random_string(6, "0123456789") - instance.save(update_fields=["email_verified", "email_activation_code"]) - - try: - send_templated_mail( - subject="Подтверждение нового email", - to=[instance.email], - template_base="emails/registration_verify_code", - context={ - "code": instance.email_activation_code, - "user": instance, - }, - ) - except Exception: - pass - - data["email_verified"] = False - - return Response(data, status=200) - + result = self._perform_user_update(instance, request.data, is_partial=True) + if isinstance(result, Response): + return result + return Response(result["data"], status=200) except Exception as e: logger.error(f"[ME PATCH] FATAL ERROR: {e}", exc_info=True) return Response({"detail": f"Internal server error: {str(e)}"}, status=500) @@ -775,156 +785,7 @@ class EmployeeViewSet(viewsets.ModelViewSet): def partial_update(self, request, *args, **kwargs): """Частичное обновление: сначала LDAP-совместимые поля → затем DB-only.""" emp = self.get_object() - old_email = emp.email - - data = request.data.copy() - if hasattr(data, "_mutable") and not data._mutable: - data._mutable = True - avatar_raw = data.get("avatar") - if avatar_raw in ("", None): - data.pop("avatar", None) - - ser = self.get_serializer(emp, data=data, partial=True) - ser.is_valid(raise_exception=True) - vd = dict(ser.validated_data) - ldap_enabled = _is_ldap_enabled() - - # Проверяем изменение email - new_email = vd.get("email") - email_changed = new_email and new_email.lower() != old_email.lower() - - # --- LDAP часть --- - ldap_keys = { - "first_name", - "last_name", - "email", - "phone_number", - "is_active", - } - ldap_changes = {k: vd.pop(k) for k in list(vd.keys()) if k in ldap_keys} - - svc_changes = dict(ldap_changes) - pos_key_present = ("position" in data) or ("position_id" in data) - if pos_key_present: - pos_raw = ( - data.get("position") if "position" in data else data.get("position_id") - ) - svc_changes["position"] = pos_raw - vd.pop("position", None) - vd.pop("position_id", None) - - move_to_department_dn = data.get("department_dn") - group_cns = data.get("group_cns") - - # avatar → bytes - avatar_file = ser.validated_data.get("avatar") - if avatar_file and hasattr(avatar_file, "read"): - try: - svc_changes["avatar_bytes"] = avatar_file.read() - if hasattr(avatar_file, "seek"): - avatar_file.seek(0) - except Exception: - pass - - # Проверяем, есть ли у пользователя ldap_dn - has_ldap_dn = False - if ldap_enabled: - has_ldap_dn = LdapSyncState.objects.filter( - model="employee", object_pk=str(emp.pk), ldap_dn__isnull=False - ).exists() or (hasattr(emp, "ldap_dn") and emp.ldap_dn) - - if ldap_enabled and has_ldap_dn: - svc = DirectoryService() - if svc_changes or move_to_department_dn or group_cns is not None: - try: - emp = svc.update_user( - emp, - changes=svc_changes, - group_cns=group_cns if group_cns is not None else None, - move_to_department_dn=move_to_department_dn, - ) - except ( - DirectoryLdapError, - DirectoryDbError, - DirectoryServiceError, - ) as e: - return Response( - {"detail": str(e)}, - status=(502 if isinstance(e, DirectoryLdapError) else 500), - ) - else: - # Non-LDAP mode - vd.update(ldap_changes) - - if pos_key_present: - pos_raw = ( - request.data.get("position") - if "position" in request.data - else request.data.get("position_id") - ) - if pos_raw is None: - vd["position"] = None - elif isinstance(pos_raw, int): - try: - vd["position"] = Position.objects.get(id=pos_raw) - except Position.DoesNotExist: - return Response( - {"detail": f"Position {pos_raw} not found"}, - status=400, - ) - elif isinstance(pos_raw, dict) and "id" in pos_raw: - try: - vd["position"] = Position.objects.get(id=pos_raw["id"]) - except Position.DoesNotExist: - return Response( - {"detail": f"Position {pos_raw['id']} not found"}, - status=400, - ) - - if avatar_file and hasattr(avatar_file, "read"): - try: - avatar_bytes = avatar_file.read() - emp.avatar.save( - avatar_file.name, - ContentFile(avatar_bytes), - save=False, - ) - vd.pop("avatar", None) - except Exception: - pass - - # --- DB-only часть --- - if "avatar" in vd: - vd.pop("avatar") - - if vd: - ser_db = self.get_serializer(emp, data=vd, partial=True) - try: - ser_db.is_valid(raise_exception=True) - ser_db.save() - emp = ser_db.instance - data = ser_db.data - except ValidationError as exc: - return Response(exc.detail, status=400) - else: - data = self.get_serializer(emp).data - - # Сброс email_verified при изменении email - if email_changed: - emp.email_verified = False - emp.email_activation_code = get_random_string(6, "0123456789") - emp.save(update_fields=["email_verified", "email_activation_code"]) - - try: - send_templated_mail( - subject="Подтверждение нового email", - to=[emp.email], - template_base="emails/registration_verify_code", - context={"code": emp.email_activation_code, "user": emp}, - ) - except Exception: - pass - - data["email_verified"] = False - - return Response(data, status=200) + result = self._perform_user_update(emp, request.data, is_partial=True) + if isinstance(result, Response): + return result + return Response(result["data"], status=200)