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 #130

Merged
merged 10 commits into from
Oct 27, 2020

Conversation

judahrand
Copy link
Contributor

Description of change

This is an extension of PR #94. I have added two tests, one which tests that nested fields in an object are filtered and one which tests that nested fields in an array of objects are filtered.

Manual QA steps

  • Wrote new tests which pass

Risks

  • Should not be any breaking changes as all previous tests pass unaltered.

Rollback steps

  • revert this branch

chrisgoddard and others added 8 commits February 20, 2019 16:50
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 :)
@judahrand judahrand changed the title Bug/filter record Filter nested properties based on metadata Oct 27, 2020
@judahrand
Copy link
Contributor Author

judahrand commented Oct 27, 2020

Seems to be Pylint failing which isn't a result of this PR but I'll fix the issues.

Copy link
Contributor

@dmosorast dmosorast left a comment

Choose a reason for hiding this comment

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

Looks great, good tests, and nice formatting change 👍

@dmosorast dmosorast merged commit 9953adb into singer-io:master Oct 27, 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.

None yet

3 participants