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

Disable soft deletes on LoginModel #82

Merged
merged 1 commit into from
Aug 26, 2019

Conversation

MGatner
Copy link
Collaborator

@MGatner MGatner commented Aug 26, 2019

No description provided.

@lonnieezell
Copy link
Owner

The thought here was that it was better to never be able to easily delete a login attempt for logging reasons. Granted, someone could simply call purge as easily, but at least then they know what they're committing to :)

Maybe it would be better to update the migration to create the table that way?

@MGatner
Copy link
Collaborator Author

MGatner commented Aug 26, 2019

Hmm I see. The problem is that it essentially disables using any find() methods from the model since it adds the deleted_at field into the query causing a database error for "column not found". I think making them soft deletes is a fine solution for a little extra safety, or maybe overriding the model's delete() function with a dummy?

@lonnieezell
Copy link
Owner

Ah, then this solution is fine, actually. I'll merge. As long as the package itself doesn't delete any, there's really nothing to stop others from bad acts.

@lonnieezell lonnieezell merged commit eba1ec7 into lonnieezell:develop Aug 26, 2019
@MGatner
Copy link
Collaborator Author

MGatner commented Aug 26, 2019

Thanks! Though the idea of throwing an exception at those scoundrels from an overloaded model->delete() was granting me a pleasant sense of justice 😆 👨‍⚖

@MGatner MGatner deleted the login-deletes branch August 26, 2019 21:23
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