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

feature/BE-27 Account deletion API #319

Merged
merged 6 commits into from
Dec 9, 2022
Merged

feature/BE-27 Account deletion API #319

merged 6 commits into from
Dec 9, 2022

Conversation

serdarakol
Copy link
Contributor

No description provided.

@serdarakol serdarakol added Effort: Low This issue can be easily handled Status: Review Needed A review is needed for this issue Team: Backend issues related to backend labels Dec 3, 2022
@serdarakol serdarakol self-assigned this Dec 3, 2022
@serdarakol serdarakol changed the title account deletion API is implemented feature/BE-27 Account deletion API Dec 3, 2022
@BElifb
Copy link
Contributor

BElifb commented Dec 5, 2022

  • Pulled the branch to local and tested.
  • Postman gives

    {"Failure":"The password did not match!"}
    error, even though I entered the correct password.

  • When I investigated the code, I saw that when checking the password, you're comparing the received string directly with user.password. Since we are keeping the encrypted versions of the passwords in the database, you should either encrypt the received password string and compare manually or use the password compare function provided (can't remember the exact name).
  • Other than that the serializer for the swagger input was named "new_password", while your view function used "password".
  • I fixed that and a few minor things about naming convertions.
  • Finally, I though we agreed on, not completely deleting the user account but rather negating the active flag. So that we can have access to user information in case of a future conflict.

@BElifb BElifb added the Bug Something isn't working label Dec 5, 2022
@serdarakol serdarakol added Status: In Progress The issue is being handled. and removed Status: Review Needed A review is needed for this issue labels Dec 5, 2022
@BElifb
Copy link
Contributor

BElifb commented Dec 5, 2022

  • Just realized, serializer I changed was being used for another API as well. Created and switched to a new one.

@BElifb
Copy link
Contributor

BElifb commented Dec 5, 2022

  • Tested the updates, API works. I can approve the PR with a promise to revisit the inactivation issue after the milestone (instead of fully deleting user data).

@BElifb BElifb removed the Bug Something isn't working label Dec 8, 2022
@BElifb BElifb merged commit 6f599c0 into master Dec 9, 2022
@KarahanS KarahanS deleted the feature/BE-27 branch December 9, 2022 14:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Effort: Low This issue can be easily handled Status: In Progress The issue is being handled. Team: Backend issues related to backend
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants