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

Tdl 13966 update custom fields #36

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
6 changes: 5 additions & 1 deletion tap_quickbooks/discover.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,14 @@ def do_discover():
))
# Set the replication_key MetaData to automatic as well
mdata = metadata.write(mdata, ('properties', stream.replication_keys[0]), 'inclusion', 'automatic')
custom_field = singer.utils.load_json(
os.path.normpath(
os.path.join(_get_abs_path("schemas/custom_field.json"))))
refs = {"custom_field.json": custom_field}
ref_schema = singer.utils.load_json(
os.path.normpath(
os.path.join(_get_abs_path("schemas/ref_schema.json"))))
refs = {"ref_schema.json": ref_schema}
refs.update({"ref_schema.json": ref_schema})
catalog_entry = {
"stream": stream_name,
"tap_stream_id": stream_name,
Expand Down
25 changes: 1 addition & 24 deletions tap_quickbooks/schemas/credit_memos.json
Original file line number Diff line number Diff line change
Expand Up @@ -256,30 +256,7 @@
},
"CustomField": {
"items": {
"properties": {
"Name": {
"type": [
"null",
"string"
]
},
"Type": {
"type": [
"null",
"string"
]
},
"DefinitionId": {
"type": [
"null",
"string"
]
}
},
"type": [
"null",
"object"
]
"$ref": "custom_field.json"
},
"type": [
"null",
Expand Down
32 changes: 32 additions & 0 deletions tap_quickbooks/schemas/custom_field.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
{
"properties": {
"Type": {
"type": [
"null",
"string"
]
},
"Name": {
"type": [
"null",
"string"
]
},
"StringValue": {
"type": [
"null",
"string"
]
},
"DefinitionId": {
"type": [
"null",
"string"
]
}
},
"type": [
"null",
"object"
]
}
25 changes: 1 addition & 24 deletions tap_quickbooks/schemas/estimates.json
Original file line number Diff line number Diff line change
Expand Up @@ -391,30 +391,7 @@
},
"CustomField": {
"items": {
"properties": {
"DefinitionId": {
"type": [
"null",
"string"
]
},
"Name": {
"type": [
"null",
"string"
]
},
"Type": {
"type": [
"null",
"string"
]
}
},
"type": [
"null",
"object"
]
"$ref": "custom_field.json"
},
"type": [
"null",
Expand Down
31 changes: 1 addition & 30 deletions tap_quickbooks/schemas/invoices.json
Original file line number Diff line number Diff line change
Expand Up @@ -176,36 +176,7 @@
},
"CustomField": {
"items": {
"properties": {
"Type": {
"type": [
"null",
"string"
]
},
"Name": {
"type": [
"null",
"string"
]
},
"StringValue": {
"type": [
"null",
"string"
]
},
"DefinitionId": {
"type": [
"null",
"string"
]
}
},
"type": [
"null",
"object"
]
"$ref": "custom_field.json"
},
"type": [
"null",
Expand Down
31 changes: 4 additions & 27 deletions tap_quickbooks/schemas/purchase_orders.json
Original file line number Diff line number Diff line change
Expand Up @@ -202,36 +202,13 @@
]
},
"CustomField": {
"items": {
"$ref": "custom_field.json"
},
"type": [
"null",
"array"
],
"items": {
"type": [
"null",
"object"
],
"properties": {
"Name": {
"type": [
"null",
"string"
]
},
"DefinitionId": {
"type": [
"null",
"string"
]
},
"Type": {
"type": [
"null",
"string"
]
}
}
}
]
},
"MetaData": {
"type": [
Expand Down
25 changes: 1 addition & 24 deletions tap_quickbooks/schemas/refund_receipts.json
Original file line number Diff line number Diff line change
Expand Up @@ -216,30 +216,7 @@
},
"CustomField": {
"items": {
"properties": {
"DefinitionId": {
"type": [
"null",
"string"
]
},
"Name": {
"type": [
"null",
"string"
]
},
"Type": {
"type": [
"null",
"string"
]
}
},
"type": [
"null",
"object"
]
"$ref": "custom_field.json"
},
"type": [
"null",
Expand Down
31 changes: 4 additions & 27 deletions tap_quickbooks/schemas/sales_receipts.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,36 +100,13 @@
}
},
"CustomField": {
"items": {
"$ref": "custom_field.json"
},
"type": [
"null",
"array"
],
"items": {
"type": [
"null",
"object"
],
"properties": {
"Name": {
"type": [
"null",
"string"
]
},
"DefinitionId": {
"type": [
"null",
"string"
]
},
"Type": {
"type": [
"null",
"string"
]
}
}
}
]
},
"BillAddr": {
"type": [
Expand Down
2 changes: 2 additions & 0 deletions tests/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ class TestQuickbooksBase(unittest.TestCase):
INCREMENTAL = "INCREMENTAL"
FULL = "FULL_TABLE"
START_DATE_FORMAT = "%Y-%m-%dT00:00:00Z" # %H:%M:%SZ
# list of streams which supports custom field
custom_command_streams = ['invoices','estimates','credit_memos','refund_receipts','sales_receipts','purchase_orders']

def setUp(self):
missing_envs = [x for x in [
Expand Down
13 changes: 13 additions & 0 deletions tests/test_quickbooks_discovery.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,7 @@ def test_run(self):
self.assertEqual(len(diff), 0, msg="discovered schemas do not match: {}".format(diff))
print("discovered schemas are OK")

custom_field_stream_count = 0
for stream in self.expected_streams():
with self.subTest(stream=stream):
catalog = next(iter([catalog for catalog in found_catalogs
Expand Down Expand Up @@ -146,3 +147,15 @@ def test_run(self):
and item.get("breadcrumb", ["properties", None])[1]
not in actual_automatic_fields}),
msg="Not all non key properties are set to available in metadata")

# verify custom field schema
if stream in self.custom_command_streams:
actual_custom_field_keys = list(schema["properties"]
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be testing this in metadata not annotated-schema. The menagerie method call is misleadingly named. But annotated-schema is no longer used by our front-end, only metadata is. You should be able to make the same assertion just change the way the actual value is accessed.

Copy link
Contributor

Choose a reason for hiding this comment

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

@kspeer825 This test is validating nested fields present under the CustomField field.
However, as metadata only contains first-level fields, can you please help us on how to retrieve nested fields from metadata?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure this is possible. The metadata is what drives table and field selection in our ui. And nested field selection is not a feature that I am aware of, so I think this would require a change to singer as well as our frontend. I will confirm this with a developer and get back to you.

Copy link
Contributor

Choose a reason for hiding this comment

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

I confirmed this with the devs. I think in this test discovering the top level key from metadata is sufficient. If you need to confirm that the subfields are picked up by the tap, you would need a test that performs a sync and you could pull the actual field values as well as the json schema from the messages sent to the target.

Copy link
Contributor

@savan-chovatiya savan-chovatiya Sep 13, 2021

Choose a reason for hiding this comment

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

@kspeer825, Updated test, and accessed top-level field value from metadata instead of annotated-schema. The subfields validation from sync is already added as part of the test_quickbooks_sync_all.py test.
NOTE: The CircleCI build is failing currently due to dependency installation failure which is observed in some taps from last week.

.get("CustomField").get("items").get("properties").keys())
expected_custom_field_keys = ['DefinitionId','Name','Type','StringValue']
actual_custom_field_keys.sort()
expected_custom_field_keys.sort()
self.assertListEqual(actual_custom_field_keys,expected_custom_field_keys)
custom_field_stream_count += 1

self.assertEqual(custom_field_stream_count,6)
20 changes: 20 additions & 0 deletions tests/test_quickbooks_sync_all.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,3 +42,23 @@ def test_run(self):
# Each stream should have 1 or more records returned
self.assertGreaterEqual(sync_record_count[stream], 1)

# Taking sync response
synced_records = runner.get_records_from_target_output()

# Taking all streams from the response
streams = synced_records.keys()

custom_field_stream_count = 0
for stream in streams:
if stream in self.custom_command_streams:
first_record = synced_records.get(stream).get('messages')[0].get('data')
actual_custom_field_keys = list(first_record.get("CustomField")[0].keys())
# For sand box only 3 fields are coming
expected_custom_field_keys = ['DefinitionId','Name','Type']
actual_custom_field_keys.sort()
expected_custom_field_keys.sort()
self.assertListEqual(actual_custom_field_keys,expected_custom_field_keys)
custom_field_stream_count +=1
else:
continue
self.assertEqual(custom_field_stream_count,6)