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

Delete OutstandingToken via django-admin #237

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

mjlabe
Copy link
Contributor

@mjlabe mjlabe commented Apr 13, 2020

On user deletion, set OutstandingToken.user to NULL and blacklist user tokens

Fix for #201

I still need to figure out how to run unit tests, but if anyone else wants to look this over and give me feedback, I'd appreciate it.

@mjlabe
Copy link
Contributor Author

mjlabe commented Apr 13, 2020

==== 124 passed in 1.73s ====

Doesn't effect unit tests. I'll test is out locally today or tomorrow.

Copy link
Member

@Andrew-Chen-Wang Andrew-Chen-Wang left a comment

Choose a reason for hiding this comment

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

I think the deletion of these tokens shouldn't be included in the package. Some people have custom User models. Others have their own way of deleting these tokens. It should just be up to the dev to include this kind of code.

@@ -3,7 +3,7 @@


class OutstandingToken(models.Model):
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.CASCADE, null=True, blank=True)
user = models.ForeignKey(settings.AUTH_USER_MODEL, on_delete=models.SET_NULL, null=True, blank=True)
Copy link
Member

Choose a reason for hiding this comment

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

If this happens then you should change the SET_NULL in the migrations (deliberately without creating a new file).

Copy link
Member

Choose a reason for hiding this comment

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

Also, I'm not entirely sure if you should change it to be SET_NULL...

@@ -33,7 +35,9 @@ def get_readonly_fields(self, *args, **kwargs):
def has_add_permission(self, *args, **kwargs):
return False

def has_delete_permission(self, *args, **kwargs):
def has_delete_permission(self, request, *args, **kwargs):
if request.user.is_superuser:
Copy link
Member

Choose a reason for hiding this comment

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

And is_staff?

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.

2 participants