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

Filter nested properties based on metadata #94

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

chrisgoddard
Copy link
Contributor

Not sure if this will be too big a change, but I wanted to be able to filter out nested properties based on metadata rather than just top level properties.

So if I have a field billing_address of type object with the properties street, state, zip, and country, I could have selected: True for billing_address, but billing_address.street could be selected: False.

The result would be the three other fields (state, zip and country) would be synced.

All tests pass - it shouldn't break any existing use cases because it will only filter out nested fields that are explicitly unselected or unsupported.

Update filter_data_by_metadata function to allow filtering of nested fields - e.g. if property `address` has selected set to True, but property `address.street` has selected set to False, only the street would be excluded.

Processes data recursively.
make formatting a little clearer
Fix array type breadcrumb name
breadcrumb path documentation
change based on tests - must remove field from data object, not just set value to None.
line lenght :)
@KAllan357
Copy link
Contributor

There are tests in for the Transform code. Do you think you could write a few to show this works as expected?

https://github.com/singer-io/singer-python/blob/master/tests/test_transform.py#L263

@chrisgoddard
Copy link
Contributor Author

Yes - will try to get to this next week.

@tmonks
Copy link

tmonks commented Sep 4, 2020

Hi Chris, would this also work to filter the properties of nested arrays of objects?

@judahrand
Copy link
Contributor

@dmosorast @cmerrick @KAllan357

If I write some tests for this could this be merged? I need this functionality for tap-shopify. I'm happy to discuss my use case with one of you on Slack?

@judahrand
Copy link
Contributor

@dmosorast @cmerrick @KAllan357
Tests added in #130

@judahrand
Copy link
Contributor

Worth closing this now that #130 is merged?

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.

None yet

4 participants