singer-io / tap-shopify

Singer.io tap for extracting Shopify data
GNU Affero General Public License v3.0
57 stars 89 forks source link

no relationship between fulfillments and fulfillment line items #81

Open m1l0sz opened 3 years ago

m1l0sz commented 3 years ago

there is no relationship created between fulfillments and fulfillment line items (ordersfulfillments and ordersfulfillments__line_items). consequently it is impossible to know which line item belongs to which fulfillment.

bobbrodie commented 8 months ago

This issue is something that's been affecting us for some time as well. This is available from Shopify's API, so it seems like a bug in the tap.

I'd like to contribute, but I think this is something that needs a decision by the product team before any code is written, to consider historical context and backwards incompatibilities. If it's something Stitch would be willing to merge in, let me know if it'd be helpful for us to make a PR. My main hesitation is building something backwards incompatible and not knowing the internal processes around that.

There seem to be two ways to address this:

Method 1

Fix orders__fulfillments__line_items._sdc_source_key_id so that it references orders__fulfillments.id

This has great implications:

This would be in-line with the standardized setup of most of this tap, however bears heavy risk.

Method 2

Add a new field of orders__fulfillments__line_items.fulfillment_id that references orders__fulfillments.id

This is safer, and while not the intended way to approach it, we can see a case made for it with refund order adjustments, which have these:

@singer-io if method 2 something that would be accepted if a pull request was made?

I've also reached out to Stitch support and on the Slack workspace, but haven't made definitive progress through those channels. I'd be happy to help here if it's something that would be accepted and used in Stitch.

m1l0sz commented 8 months ago

i believe this affects refunds and refunds__line_items as well

gl getting a response from stitch about this LOL

we "fixed" this by writing our own etl for an index table.

but if you end up forking the repo to fix this lmk

bobbrodie commented 8 months ago

Yep, we ended up doing something similar, using the Shopify APIs and building a table of created_at from the fulfillment with fulfillment_line_item_id (that's mainly what we needed) as the second column, but it would be awesome to move away from that.

I'll definitely keep you posted if I hear anything!