laravel-json-api / eloquent

Serialize Eloquent models to JSON API resources
MIT License
12 stars 15 forks source link

add whereNull filter #27

Closed tommie1001 closed 1 year ago

tommie1001 commented 1 year ago

Just really missed this filter in my current project, thought it would be nice to add it to the collection. Especially since it is a nice base laravel functionality. If you want me to add it to the tests, let me know!

tommie1001 commented 1 year ago

Hi @lindyhopchris ,

I have updated the apply method since when using ->not() the null value would return false instead of the required true. Let me know if you want something to change.

lindyhopchris commented 1 year ago

@tommie1001 I think I might need to make some changes, as I don't find it intuitive currently. I've got an idea of what I might do with it, will find some time this weekend to look at it.

lindyhopchris commented 1 year ago

@tommie1001 have played around with this and decided to make some changes - just to make it really clear whether the filter will use whereNull or whereNotNull based on the value provided by the client.

I've split it into two classes - WhereNull and WhereNotNull.

Given a scenario where a Post model has a published_at column that indicates if it is a draft (null value) or is published (a date time value).

You would use WhereNull like this:

WhereNull::make('draft', 'published_at')

In this case, providing true as the filter value would result in models where the column value is null, i.e. they are draft. Providing false would give you models where the column value has a date time, i.e. is not-draft (aka published).

You would use WhereNotNull like this:

WhereNotNull::make('published', 'published_at')

In this case, providing true as the filter value would result in models where the column value is a date time, i.e. they are published. Providing false would give you models where the column value is null, i.e. is not-published (aka draft).

Thanks for the suggestion of adding this, as I think it's going to be a really good new feature. Do the changes I've made make sense, and do they work for your scenario?

tommie1001 commented 1 year ago

Looks good to me! Its better indeed to have 2 filters instead of the ->not method. Thanks for adding the refactor, nice work!

lindyhopchris commented 1 year ago

Great, I'll tag this now.