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

Users | Refactors/about yourself #397

Open
wants to merge 6 commits into
base: users
Choose a base branch
from

Conversation

Edgar-84
Copy link

No description provided.

@kieled kieled added the users label Jul 19, 2023
Comment on lines 33 to 40
@staticmethod
def get_user_model():
return Admin

@staticmethod
def get_reverse_url():
return "user-account"

Copy link
Collaborator

Choose a reason for hiding this comment

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

а зачем это?

}

def retrieve(self, request, user_uuid=None):
user = BaseAbstractUser.objects.filter(uuid=user_uuid).first()
Copy link
Collaborator

Choose a reason for hiding this comment

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

user = BaseAbstractUser.objects.filter(uuid=user_uuid).first()
так не сработает
надо переопределить метод get_queryset

def get_queryset(self):
qs = {
'administrator': Admin.objects.all(),
'developer': CompanyUser.objects.all(),
'customer': CustomerUser.objects.all(),
}
role = self.request.user._meta.app_label
return qs.get(role, [])

и в методе retrive получать пользователя
user = self.get_object()

Comment on lines 28 to 33
permission_map = {
"default": [IsAuthenticated],
"retrieve": [IsAuthenticated],
"partial_update": [IsAuthenticated],
"destroy": [IsAuthenticated],
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

можно один permission_classes сделать

elif role == 'developer':
return CompanyUser.objects.all()

return BaseAbstractUser.objects.none()
Copy link
Collaborator

Choose a reason for hiding this comment

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

лучше пустой список возвращать

Comment on lines 73 to 83
def get_queryset(self):
role = self.request.user._meta.app_label

if role == 'administrator':
return Admin.objects.all()
elif role == 'customer':
return CustomerUser.objects.all()
elif role == 'developer':
return CompanyUser.objects.all()

return []
Copy link
Collaborator

Choose a reason for hiding this comment

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

давай тут тоже вот так сделаем
def get_queryset(self):
qs = {
'administrator': Admin.objects.all(),
'developer': CompanyUser.objects.all(),
'customer': CustomerUser.objects.all(),
}

    role = self.request.user._meta.app_label
    return qs.get(role, [])

)
class AccountMultipleUsersViewSet(viewsets.ViewSet):
http_method_names = ["post"]
permission_map = UserPermissionClass.get_permission_map()
Copy link
Collaborator

Choose a reason for hiding this comment

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

это лишнее
я имел ввиду сделать так
permission_class = [IsAuthenticated]
так как все методы для авторизованного пользователя


def create(self, request):
uuid_list = request.data.get('uuid_list', [])
users = BaseAbstractUser.objects.filter(uuid__in=uuid_list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

нельзя фильтровать по абстрактному классу
можно сделать отдельный метод который будет возвращать всех пользователей
def get_users_queryset(users_uuid: list):
admins = Admin.objects.filter(id__in=users_uuid)
customers = CustomerUser.objects.filter(id__in=users_uuid)
developers = CompanyUser.objects.filter(id__in=users_uuid)
return admin | customers | developers

Copy link
Collaborator

Choose a reason for hiding this comment

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

я описал как это можно сделать по другому, так что можешь удалить этот файл

Comment on lines 13 to 15
administrators = instance.filter(role='administrator')
developers = instance.filter(role='developer')
customers = instance.filter(role='customer')
Copy link
Collaborator

Choose a reason for hiding this comment

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

по ролям вряд-ли у нас отфильтруется

Comment on lines +37 to +41
return administrators, customers, developers

def create(self, request):
uuid_list = request.data.get('uuid_list', [])
administrators, customers, developers = self.get_users_queryset(users_uuid=uuid_list)
Copy link
Collaborator

Choose a reason for hiding this comment

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

лучше возвращать dict вместо tuple кверисетов
return {'administrator': administrator...}
так как случайно можем поменять порядок и данные перемешаются
и в методе create просто доставатб из словаря

Comment on lines 33 to +34
def get_permissions(self):
return [
permission()
for permission in self.permission_map.get(self.action, self.permission_map["default"])
]
return [permission() for permission in self.permission_classes]
Copy link
Collaborator

Choose a reason for hiding this comment

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

можно полностью удалить этот метод
плюс надо перенести все методы из вьюшки PersonalAccount в класс AccountViewSet
так как эта вьюшка наследуется от PersonalAccount

path(
"account/me/<uuid:user_id>/",
AccountSingleUserViewSet.as_view(
{"get": "retrieve", "put": "partial_update", "delete": "destroy"},
Copy link
Collaborator

@oxpaoff oxpaoff Jul 25, 2023

Choose a reason for hiding this comment

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

давай сделаем так во вьюшки AccountViewSet добавишь action bu_uuid для GET и для POST методов
для GET перенесешь функционал вьюшки AccountSingleUserViewSet
а для POST из вьюшки AccountMultipleUsersViewSet

Copy link
Collaborator

Choose a reason for hiding this comment

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

так будет удобнее, все будет в одной вьюшке, а не в трех

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants