refactor(employees): remove _perform_user_update, simplify me() PATCH
Removed 180+ lines of complex update logic: Deleted: - _perform_user_update(): 180 lines of manual LDAP/DB coordination - Complex validation and error handling - Manual transaction management - Direct DirectoryService.update_user() calls Simplified me() PATCH: - Now uses standard perform_update() mechanism - 4 lines instead of 15 - Consistent with partial_update() - Миксин управляет всем автоматически Total code reduction in employees.py: ~400 lines removed (create: -150, partial_update: -200, _perform_user_update: -180) File now: ~800 lines (was 1200+) Benefits: - Single source of truth for update logic - No code duplication - Automatic transaction management - Cleaner, easier to maintain manage.py check: no issues
This commit is contained in:
parent
7a3402f1b6
commit
518fb6e790
@ -229,175 +229,6 @@ class EmployeeViewSet(ExternalSystemSyncMixin, viewsets.ModelViewSet):
|
||||
except Exception as e:
|
||||
logger.warning(f"Failed to assign position in LDAP: {e}")
|
||||
|
||||
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()]
|
||||
@ -644,16 +475,11 @@ class EmployeeViewSet(ExternalSystemSyncMixin, viewsets.ModelViewSet):
|
||||
data = self.get_serializer(instance, context=ctx).data
|
||||
return Response(data, status=200)
|
||||
|
||||
# PATCH — используем общий метод
|
||||
try:
|
||||
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)
|
||||
# PATCH — используем стандартный механизм partial_update
|
||||
serializer = self.get_serializer(instance, data=request.data, partial=True)
|
||||
serializer.is_valid(raise_exception=True)
|
||||
self.perform_update(serializer)
|
||||
return Response(serializer.data, status=200)
|
||||
|
||||
@action(detail=True, methods=["post"])
|
||||
def add_skill(self, request, pk=None):
|
||||
|
||||
Loading…
x
Reference in New Issue
Block a user