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

#260 add check password when delete profile #261

Merged
merged 4 commits into from
Oct 10, 2023

Conversation

Lvyshnevska
Copy link
Collaborator

Functionality of checking password when delete profile added (destroy method overridden). Tests (delete profile) updated due to changes.

@Lvyshnevska Lvyshnevska added this to the Sprint 5 milestone Oct 9, 2023
@Lvyshnevska Lvyshnevska requested review from ProDG and Shushunya October 9, 2023 15:43
@Lvyshnevska Lvyshnevska self-assigned this Oct 9, 2023
@Lvyshnevska Lvyshnevska linked an issue Oct 9, 2023 that may be closed by this pull request
@@ -174,6 +176,16 @@ def get_serializer_class(self):
else:
return ProfileOwnerDetailEditSerializer

def destroy(self, request, *args, **kwargs):
instance = self.get_object()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe it's better to use get_object_or_404 here?

Copy link

Choose a reason for hiding this comment

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

No. get_object_or_404() may be used in get_object() (if needed), but not here.

user = self.request.user
password = self.request.data.get("password")
if not password or not check_password(password, user.password):
return Response(status=status.HTTP_400_BAD_REQUEST)
Copy link

Choose a reason for hiding this comment

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

  1. 400 BAD REQUEST must have a body, with explanation what exactly is wrong with the request. So, I guess this logic (together with password check) should be in serializer validator.
  2. Don't, don't do return Response().

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wanted to do it in serializer, but there is no delete method in serializers, so I don't know how to implement it. I remember that it's not okay to return Response. But I just override original destroy method and it contains Response.

@Lvyshnevska Lvyshnevska merged commit 93486c4 into develop Oct 10, 2023
3 checks passed
@Lvyshnevska Lvyshnevska deleted the #260-AddCheckPasswordWhenDeleteProfile branch October 10, 2023 12:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add check password functionality when delete profile
3 participants