Skip to content

Account deletion must be confirmed #95

Merged
nuest merged 9 commits intomainfrom
feature/account_deletion_confirmation
Mar 19, 2025
Merged

Account deletion must be confirmed #95
nuest merged 9 commits intomainfrom
feature/account_deletion_confirmation

Conversation

@uxairibrar
Copy link
Collaborator

@uxairibrar uxairibrar commented Feb 21, 2025

Closes #25

  • Added UserSerializer to ensure deleted users are excluded from API responses.
  • Implemented CustomUser model to handle soft deletion properly.
  • Added three views (request_delete, confirm_account_deletion, finalize_account_deletion) to manage account deletion flow.
  • Integrated a new modal in the User Settings page for final deletion confirmation.
  • Fixed UI alignment issues in the delete account section to prevent overflow.

nuest
nuest requested changes Feb 24, 2025
View reviewed changes
Copy link
Member

@nuest nuest left a comment

Choose a reason for hiding this comment

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

I added a few comments, please take a look.

Also, the tests are still missing. Since you're working against API endpoints, I'm fine with tests directly talking to the API, but UI-based tests (or both) would be even better.

nuest
nuest requested changes Mar 16, 2025
View reviewed changes
@nuest
Copy link
Member

nuest commented Mar 17, 2025

@uxairibrar Sorry if I interrupted your development flow with my question.

Please let me know when the PR is ready for review!

An in the future: what about you converting a PR back to "draft" when you are pushing changes piecewise?

@uxairibrar
Copy link
Collaborator Author

uxairibrar commented Mar 17, 2025

@uxairibrar Sorry if I interrupted your development flow with my question.

Please let me know when the PR is ready for review!

An in the future: what about you converting a PR back to "draft" when you are pushing changes piecewise?

Yes, sure, it will be easy for both of us. However I also added the UI test for this PR, I think it is ready to be reviewed.

nuest
nuest requested changes Mar 17, 2025
View reviewed changes
Copy link
Member

@nuest nuest left a comment

Choose a reason for hiding this comment

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

A few comments here and there, but I'm happy overall.

publications/views.py Outdated
messages.error(request, "You are not authorized to delete this account.")
return redirect(reverse('optimap:main'))

user = get_object_or_404(User, id=user_id)
Copy link
Member

@nuest nuest Mar 17, 2025

Choose a reason for hiding this comment

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

The user object here is not used at all.

Copy link
Collaborator Author

@uxairibrar uxairibrar Mar 18, 2025

Choose a reason for hiding this comment

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

I have removed it

@nuest nuest merged commit aae93ac into main Mar 19, 2025
0 of 2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nuest nuest nuest requested changes

Assignees

@uxairibrar uxairibrar

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Account deletion must be confirmed

2 participants

@uxairibrar @nuest