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

Modify Model's deleted field to be a date #2041

Closed
lonnieezell opened this issue Jun 6, 2019 · 7 comments
Closed

Modify Model's deleted field to be a date #2041

lonnieezell opened this issue Jun 6, 2019 · 7 comments

Comments

@lonnieezell
Copy link
Member

I've been thinking about this a lot for the past few months, and I was in error to simply use a deleted (bool) field to represent soft deletions. Using a deleted_at field that is a date and treated the same as created_at and updated_at is how it should be done. This allows you to actually investigate when it was deleted if you need to, without needing to build a complete action logging system.

If this is going to happen, it has to be now, before the first RC.

@lonnieezell lonnieezell added this to the 4.0.0-rc.1 milestone Jun 6, 2019
@diegoldev
Copy link
Contributor

I'm agree whit this point. I think we could use 'deleted_at' as flag field but also as a log field.

@MGatner
Copy link
Member

MGatner commented Jun 7, 2019

I'm glad to work on this some, but it seems like a significant enough change that it might merit some collaboration. Could we maybe get a new branch for this?

@lonnieezell
Copy link
Member Author

@MGatner I'm not sure it's completely necessary, but I have setup a new branch over here for you.

@MGatner
Copy link
Member

MGatner commented Jun 10, 2019

Maybe it's overkill - when I first looked it seemed that this would need changes in a bunch of places but I don't have my head around the whole thing yet. I'll take a look soon.

@lonnieezell
Copy link
Member Author

Cool. If you run into anything definitely let me know and I'll see what I can do.

@MGatner
Copy link
Member

MGatner commented Jun 11, 2019

I think #2051 is ready to go for this. It didn't end up being nearly as big a change as I thought, but has a lot of downstream implications so I would appreciate a second set of eyes.

@lonnieezell
Copy link
Member Author

@MGatner Thanks so much! I ran out of time to look at it last night. Very close. Just a couple of small items on it.

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

No branches or pull requests

3 participants