refactor(positions,roles): упростить ViewSets - удалить LDAP логику

Упрощены ViewSets:
- PositionViewSet: 146 → 137 строк (-9 строк, -6%)
- DepartmentRoleViewSet: 362 → 248 строк (-114 строк, -31%)

Изменения PositionViewSet:
- Удалены вызовы DirectoryService().reconcile_position()
- Удалены вызовы DirectoryService().delete_position_group()
- Удалены импорты: DirectoryService, _is_ldap_enabled, _ldap_try
- Actions (set_groups, add_groups, remove_groups): только работа с БД
- perform_create/update/destroy: базовые методы без LDAP

Изменения DepartmentRoleViewSet:
- create(): только создание в БД, удалены LDAP блоки
- update(): упрощен до базового метода
- destroy(): только удаление из БД
- assign(): только update_or_create RoleAssignment
- revoke(): только update is_active=False
- Удалены все if ldap_enabled блоки
- Удалены импорты: DirectoryService, DirectoryLdapError, DirectoryDbError, _is_ldap_enabled
- Удалена обработка ошибок LDAP (try/except блоки)

Итого: -123 строки, все ViewSets теперь работают только с БД

Примечание: Полноценная LDAP синхронизация для Position и DepartmentRole
требует реализации отдельных сигналов (аналогично signals_group.py).
Это может быть следующим этапом рефакторинга.
This commit is contained in:
lizerk 2026-03-18 18:22:52 +03:00
parent e606a6ce2b
commit 51b878c32b
2 changed files with 57 additions and 180 deletions

View File

@ -1,9 +1,8 @@
"""PositionViewSet — CRUD должностей с LDAP-синхронизацией групп."""
"""PositionViewSet — CRUD должностей."""
from __future__ import annotations
from django.contrib.auth.models import Group, Permission
from employees.ldap.directory_service import DirectoryService
from employees.models import Position
from rest_framework import filters, status, viewsets
from rest_framework.decorators import action
@ -12,7 +11,7 @@ from rest_framework.response import Response
from ...permissions import AdminOrActionOrModelPerms
from ..serializers import PositionSerializer
from ._helpers import HistoryActionMixin, _is_ldap_enabled, _ldap_try
from ._helpers import HistoryActionMixin
class PositionViewSet(HistoryActionMixin, viewsets.ModelViewSet):
@ -62,14 +61,12 @@ class PositionViewSet(HistoryActionMixin, viewsets.ModelViewSet):
@action(detail=True, methods=["post"])
def set_groups(self, request, pk=None):
"""Устанавливает группы для должности. Синхронизация LDAP требует реализации сигналов."""
pos = self.get_object()
qs, err = self._validate_groups_payload(request)
if err:
return err
pos.groups.set(qs)
err2 = _ldap_try(lambda: DirectoryService().reconcile_position(pos))
if err2:
return err2
return Response(
{
"ok": True,
@ -79,14 +76,12 @@ class PositionViewSet(HistoryActionMixin, viewsets.ModelViewSet):
@action(detail=True, methods=["post"])
def add_groups(self, request, pk=None):
"""Добавляет группы к должности. Синхронизация LDAP требует реализации сигналов."""
pos = self.get_object()
qs, err = self._validate_groups_payload(request)
if err:
return err
pos.groups.add(*qs)
err2 = _ldap_try(lambda: DirectoryService().reconcile_position(pos))
if err2:
return err2
return Response(
{
"ok": True,
@ -96,14 +91,12 @@ class PositionViewSet(HistoryActionMixin, viewsets.ModelViewSet):
@action(detail=True, methods=["post"])
def remove_groups(self, request, pk=None):
"""Удаляет группы у должности. Синхронизация LDAP требует реализации сигналов."""
pos = self.get_object()
qs, err = self._validate_groups_payload(request)
if err:
return err
pos.groups.remove(*qs)
err2 = _ldap_try(lambda: DirectoryService().reconcile_position(pos))
if err2:
return err2
return Response(
{
"ok": True,
@ -132,15 +125,13 @@ class PositionViewSet(HistoryActionMixin, viewsets.ModelViewSet):
return Response({"count": len(data), "results": data}, status=200)
def perform_create(self, serializer):
pos = serializer.save()
err = _ldap_try(lambda: DirectoryService().reconcile_position(pos))
if err:
raise Exception(err.data["detail"])
"""Создание должности. Синхронизация LDAP требует реализации сигналов."""
serializer.save()
def perform_update(self, serializer):
pos = serializer.save()
_ldap_try(lambda: DirectoryService().reconcile_position(pos))
"""Обновление должности. Синхронизация LDAP требует реализации сигналов."""
serializer.save()
def perform_destroy(self, instance):
_ldap_try(lambda: DirectoryService().delete_position_group(instance))
"""Удаление должности. Синхронизация LDAP требует реализации сигналов."""
return super().perform_destroy(instance)

View File

@ -1,10 +1,8 @@
"""DepartmentRoleViewSet — CRUD ролей отделов с LDAP-синхронизацией."""
"""DepartmentRoleViewSet — CRUD ролей отделов."""
from __future__ import annotations
from django.shortcuts import get_object_or_404
from employees.ldap.directory_service import DirectoryService
from employees.ldap.errors import DirectoryDbError, DirectoryLdapError
from employees.models import (Department, DepartmentPermission, DepartmentRole,
DeptPerm, EmployeeDepartment, RoleAssignment)
from employees.utils import _ensure_department_permissions
@ -15,7 +13,7 @@ from rest_framework.response import Response
from ...permissions import AdminOrDeptAllowed
from ..serializers import DepartmentRoleSerializer
from ._helpers import Employee, _is_ldap_enabled
from ._helpers import Employee
class DepartmentRoleViewSet(viewsets.ModelViewSet):
@ -72,7 +70,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
return qs
def create(self, request, *args, **kwargs):
"""Создание роли: сначала группа в LDAP → затем запись в БД."""
"""Создание роли. Синхронизация LDAP требует реализации сигналов."""
ser = self.get_serializer(data=request.data)
ser.is_valid(raise_exception=True)
@ -86,78 +84,28 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
codes = ser.validated_data.pop("scoped_permission_codes", None)
perms = ser.validated_data.pop("scoped_permissions", None)
ldap_enabled = _is_ldap_enabled()
role = DepartmentRole.objects.create(
department=dept,
name=name,
)
if codes is not None:
qs = DepartmentPermission.objects.filter(code__in=codes)
role.scoped_permissions.set(list(qs))
elif perms is not None:
role.scoped_permissions.set(perms)
if ldap_enabled:
svc = DirectoryService()
permission_codes = None
if codes is not None:
permission_codes = codes
elif perms is not None:
permission_codes = [p.code for p in perms]
try:
role = svc.create_role(
dept=dept,
name=name,
permission_codes=permission_codes,
)
return Response(
self.get_serializer(
role).data, status=status.HTTP_201_CREATED
)
except DirectoryLdapError as e:
return Response(
{"detail": f"LDAP error: {e}"}, status=status.HTTP_502_BAD_GATEWAY
)
except DirectoryDbError as e:
return Response(
{"detail": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
else:
role = DepartmentRole.objects.create(
department=dept,
name=name,
)
if codes is not None:
qs = DepartmentPermission.objects.filter(code__in=codes)
role.scoped_permissions.set(list(qs))
elif perms is not None:
role.scoped_permissions.set(perms)
return Response(
self.get_serializer(role).data, status=status.HTTP_201_CREATED
)
return Response(
self.get_serializer(role).data, status=status.HTTP_201_CREATED
)
def update(self, request, *args, **kwargs):
"""Обновление роли: сначала LDAP → затем БД."""
"""Обновление роли. Синхронизация LDAP требует реализации сигналов."""
partial = kwargs.pop("partial", False)
instance = self.get_object()
ser = self.get_serializer(instance, data=request.data, partial=partial)
ser.is_valid(raise_exception=True)
ldap_enabled = _is_ldap_enabled()
if ldap_enabled:
svc = DirectoryService()
new_name = ser.validated_data.get("name")
try:
role = svc.update_role(instance, name=new_name)
return Response(self.get_serializer(role).data)
except DirectoryLdapError as e:
return Response(
{"detail": f"LDAP error: {e}"}, status=status.HTTP_502_BAD_GATEWAY
)
except DirectoryDbError as e:
return Response(
{"detail": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
else:
self.perform_update(ser)
return Response(ser.data)
self.perform_update(ser)
return Response(ser.data)
def partial_update(self, request, *args, **kwargs):
"""Частичное обновление роли."""
@ -165,28 +113,10 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
return self.update(request, *args, **kwargs)
def destroy(self, request, *args, **kwargs):
"""Удаление роли: сначала группа из LDAP → затем запись из БД."""
"""Удаление роли. Синхронизация LDAP требует реализации сигналов."""
instance = self.get_object()
ldap_enabled = _is_ldap_enabled()
if ldap_enabled:
svc = DirectoryService()
try:
svc.delete_role(instance)
return Response(status=status.HTTP_204_NO_CONTENT)
except DirectoryLdapError as e:
return Response(
{"detail": f"LDAP error: {e}"}, status=status.HTTP_502_BAD_GATEWAY
)
except DirectoryDbError as e:
return Response(
{"detail": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
else:
instance.delete()
return Response(status=status.HTTP_204_NO_CONTENT)
instance.delete()
return Response(status=status.HTTP_204_NO_CONTENT)
@action(detail=False, methods=["get"])
def perm_choices(self, request):
@ -263,7 +193,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
@action(detail=True, methods=["post"])
def assign(self, request, pk=None):
"""Назначает роль сотруднику."""
"""Назначает роль сотруднику. Синхронизация LDAP требует реализации сигналов."""
role = self.get_object()
employee_id = request.data.get("employee_id")
@ -277,60 +207,30 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
assigned_by = request.user if request.user.is_authenticated else None
ldap_enabled = _is_ldap_enabled()
if ldap_enabled:
try:
svc = DirectoryService()
assignment = svc.assign_role(
employee, role, assigned_by)
return Response(
{
"id": assignment.id,
"employee_id": assignment.employee_id,
"role_id": assignment.role_id,
"assigned_at": assignment.assigned_at.isoformat()
if assignment.assigned_at
else None,
"is_active": assignment.is_active,
},
status=201,
)
except DirectoryLdapError as e:
return Response(
{"detail": f"LDAP error: {e}"}, status=status.HTTP_502_BAD_GATEWAY
)
except DirectoryDbError as e:
return Response(
{"detail": str(e)}, status=status.HTTP_500_INTERNAL_SERVER_ERROR
)
else:
assignment, created = RoleAssignment.objects.update_or_create(
employee=employee,
role=role,
defaults={
"is_active": True,
"assigned_by": assigned_by,
},
)
return Response(
{
"id": assignment.id,
"employee_id": assignment.employee_id,
"role_id": assignment.role_id,
"assigned_at": assignment.assigned_at.isoformat()
if assignment.assigned_at
else None,
"is_active": assignment.is_active,
},
status=201,
)
assignment, created = RoleAssignment.objects.update_or_create(
employee=employee,
role=role,
defaults={
"is_active": True,
"assigned_by": assigned_by,
},
)
return Response(
{
"id": assignment.id,
"employee_id": assignment.employee_id,
"role_id": assignment.role_id,
"assigned_at": assignment.assigned_at.isoformat()
if assignment.assigned_at
else None,
"is_active": assignment.is_active,
},
status=201,
)
@action(detail=True, methods=["post"])
def revoke(self, request, pk=None):
"""Отзывает роль у сотрудника."""
"""Отзывает роль у сотрудника. Синхронизация LDAP требует реализации сигналов."""
role = self.get_object()
employee_id = request.data.get("employee_id")
@ -342,21 +242,7 @@ class DepartmentRoleViewSet(viewsets.ModelViewSet):
except Employee.DoesNotExist:
return Response({"detail": "Сотрудник не найден."}, status=404)
ldap_enabled = _is_ldap_enabled()
if ldap_enabled:
try:
svc = DirectoryService()
svc.revoke_role(employee, role)
return Response(status=204)
except DirectoryLdapError as e:
return Response(
{"detail": f"LDAP error: {e}"}, status=status.HTTP_502_BAD_GATEWAY
)
else:
RoleAssignment.objects.filter(employee=employee, role=role).update(
is_active=False
)
return Response(status=204)
RoleAssignment.objects.filter(employee=employee, role=role).update(
is_active=False
)
return Response(status=204)