Skip to content

Option to coerce "null" to Ruby nil#158

Merged
richmolj merged 1 commit into
graphiti-api:masterfrom
zeisler:allow_null_to_be_passed_to_filter
Jun 13, 2019
Merged

Option to coerce "null" to Ruby nil#158
richmolj merged 1 commit into
graphiti-api:masterfrom
zeisler:allow_null_to_be_passed_to_filter

Conversation

@zeisler

@zeisler zeisler commented Jun 11, 2019

Copy link
Copy Markdown
Contributor

Filters have new options allow_nil: true
Option can be set at the resource level Resource.filters_accept_nil_by_default = true
By default this is set to false.

#142

@richmolj richmolj left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I love you for this PR ❤️ 😃 ! Thank you!

My only thought is validating this will work for other types than strings (particularly hash, though not the biggest deal). Seems like it would, but maybe a test on age would be helpful?

@@ -53,6 +53,7 @@ def each_filter
type = Types[filter.values[0][:type]]
unless type[:canonical_name] == :hash || !value.is_a?(String)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Seems like this would not apply to hash types - would it be possible to move out of this block so all types could support null?

@zeisler zeisler force-pushed the allow_null_to_be_passed_to_filter branch from 79a1383 to 5c7087c Compare June 12, 2019 18:59
@zeisler

zeisler commented Jun 12, 2019

Copy link
Copy Markdown
Contributor Author

@richmolj changes made.

zeisler added a commit to zeisler/graphiti-api.github.io that referenced this pull request Jun 12, 2019
zeisler added a commit to zeisler/graphiti-api.github.io that referenced this pull request Jun 12, 2019
@richmolj

Copy link
Copy Markdown
Contributor

Great stuff 🎉 ! If you could add to CHANGELOG I'm ready to merge + associated docs. Thank you!

@zeisler zeisler force-pushed the allow_null_to_be_passed_to_filter branch 2 times, most recently from acf70e3 to 4e32763 Compare June 12, 2019 21:07
@zeisler

zeisler commented Jun 13, 2019

Copy link
Copy Markdown
Contributor Author

@richmolj change log added.

@richmolj

Copy link
Copy Markdown
Contributor

Thanks @zeisler, you rock ❤️ There's a conflict I can't fix though GH atm, but if you can resolve I'm ready to merge.

Filters have new options `allow_nil: true`
Option can be set at the resource level `Resource.filters_accept_nil_by_default = true`
By default this is set to false.

graphiti-api#142
@zeisler zeisler force-pushed the allow_null_to_be_passed_to_filter branch from 4e32763 to 99f5d9d Compare June 13, 2019 20:15
@zeisler

zeisler commented Jun 13, 2019

Copy link
Copy Markdown
Contributor Author

@richmolj resolved.

@richmolj richmolj merged commit d6d9f69 into graphiti-api:master Jun 13, 2019
richmolj pushed a commit to graphiti-api/graphiti-api.github.io that referenced this pull request Aug 8, 2019
* Allow "null" to be passed in filters Docs for PR graphiti-api/graphiti#158

* Update guides/concepts/resources.md

Grammar fix
richmolj pushed a commit to graphiti-api/graphiti-api.github.io that referenced this pull request Apr 9, 2020
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