From 56bc8c6dcda7d52eb6b580a4c7ecf69883bfed42 Mon Sep 17 00:00:00 2001 From: Jamie Rodriguez <65564846+fivetran-jamie@users.noreply.github.com> Date: Fri, 6 Jan 2023 13:58:08 -0800 Subject: [PATCH] joe feedback --- .buildkite/scripts/run_models.sh | 6 ++---- DECISIONLOG.md | 16 +++++++++++++++- README.md | 12 +----------- dbt_project.yml | 6 +++--- ...ql => int_shopify__customer_email_rollup.sql} | 0 ...=> int_shopify__emails__order_aggregates.sql} | 0 ...int_shopify__inventory_level__aggregates.sql} | 2 +- ...=> int_shopify__products_with_aggregates.sql} | 0 models/intermediate/intermediate.yml | 6 +++--- models/shopify__customer_emails.sql | 6 +++--- models/shopify__inventory_levels.sql | 2 +- models/shopify__order_lines.sql | 8 +------- models/shopify__orders.sql | 8 -------- models/shopify__products.sql | 2 +- 14 files changed, 31 insertions(+), 43 deletions(-) rename models/intermediate/{shopify__customer_email_rollup.sql => int_shopify__customer_email_rollup.sql} (100%) rename models/intermediate/{shopify__emails__order_aggregates.sql => int_shopify__emails__order_aggregates.sql} (100%) rename models/intermediate/{shopify__inventory_level__aggregates.sql => int_shopify__inventory_level__aggregates.sql} (96%) rename models/intermediate/{shopify__products__with_aggregates.sql => int_shopify__products_with_aggregates.sql} (100%) diff --git a/.buildkite/scripts/run_models.sh b/.buildkite/scripts/run_models.sh index 3828b34..3e08693 100644 --- a/.buildkite/scripts/run_models.sh +++ b/.buildkite/scripts/run_models.sh @@ -19,7 +19,5 @@ dbt deps dbt seed --target "$db" --full-refresh dbt run --target "$db" --full-refresh dbt test --target "$db" -dbt run --vars '{shopify__using_order_adjustment: false, shopify__using_order_line_refund: false, shopify__using_refund: false}' --target "$db" --full-refresh -dbt test --target "$db" -dbt run --vars '{shopify__using_order_line_refund: false}' --target "$db" --full-refresh -dbt run --vars '{shopify__using_refund: false}' --target "$db" --full-refresh \ No newline at end of file +dbt run --vars '{shopify_timezone: "America/New_York"}' --target "$db" --full-refresh +dbt test --target "$db" \ No newline at end of file diff --git a/DECISIONLOG.md b/DECISIONLOG.md index 95bebf6..a5e6ae2 100644 --- a/DECISIONLOG.md +++ b/DECISIONLOG.md @@ -1,7 +1,21 @@ ## Decision Log +In creating this package, which is meant for a wide range of use cases, we had to take opinionated stances on a few different questions we came across during development. We've consolidated significant choices we made here, and will continue to update as the package evolves. + ### Refund/Return Timestamp Mismatch In validating metrics with the Sales over Time reports in the Shopify UI, you may detect discrepancies in reported revenue. A known difference between this package's reporting and the Shopify UI is that Shopify's UI will report refunded revenue on the date that the _return was processed_ (see Shopify [docs](https://help.shopify.com/en/manual/reports-and-analytics/shopify-reports/report-types/sales-report)), whereas this package reports on the date the _order was placed_. So, if a customer placed an order amounting to $50 on November 30th, 2022 and fully returned it on December 1st, 2022, the package would report $0 net sales for this customer on November 30th, while Shopify would report $50 in sales on November 30th and -$50 on December 1st. -We felt that reporting on the order date made more sense in reality, but, if you feel differently, please reach out and create a Feature Request. To align with the Shopify method yourself, this would most likely involve aggregating `transactions` data (relying on the `kind` column to determine sales vs returns) instead of `orders`. \ No newline at end of file +We felt that reporting on the order date made more sense in reality, but, if you feel differently, please reach out and create a Feature Request. To align with the Shopify method yourself, this would most likely involve aggregating `transactions` data (relying on the `kind` column to determine sales vs returns) instead of `orders`. + +## Creating Empty Tables for Refunds, Order Line Refunds, and Order Adjustments + +Source tables related to `refunds`, `order_line_refunds`, and `order_adjustments` are created in the Shopify schema dyanmically. For example, if your shop has not incurred any refunds, you will not have a `refund` table yet until you do refund an order. + +Thus, the source package will create empty (1 row of all `NULL` fields) staging models if these source tables do not exist in your Shopify schema yet, and the transform package will work seamlessly with these empty models. Once `refund`, `order_line_refund`, or `order_adjustment` exists in your schema, the source and transform packages will automatically reference the new populated table(s). ([example](https://github.com/fivetran/dbt_shopify_source/blob/main/models/tmp/stg_shopify__refund_tmp.sql)). + +> In previous versions of the package, you had to manually enable or disable transforms of `refund`, `order_line_refund`, or `order_adjustment` through variables. Because this required you to monitor your Shopify account/schema and update the variable(s) accordingly, we decided to pursue a more automated solution. + +## Keeping Deleted Entities + +todo - not filtering out _fivetran_deleted in staging models. when joining these tables together in the transform package, bring in _fivetran_deleted as is__deleted \ No newline at end of file diff --git a/README.md b/README.md index 83d8c82..7274fce 100644 --- a/README.md +++ b/README.md @@ -66,17 +66,7 @@ vars: shopify_union_databases: ['shopify_usa','shopify_canada'] # use this if the data is in different databases/projects but uses the same schema name ``` -## Step 4: Disable models for non-existent sources -This package was designed with the intention that users have all relevant Shopify tables being synced by Fivetran. However, if you are a Shopify user that does not operate on `returns` or `adjustments` then you will not have the related source tables. As such, you may use the below variable configurations to disable the respective downstream models. All variables are `true` by default. Only add the below configuration to your root `dbt_project.yml` if you are wishing to disable the models: - -```yml -# dbt_project.yml - -vars: - shopify__using_order_adjustment: false # true by default - shopify__using_order_line_refund: false # true by default - shopify__using_refund: false # true by default -``` +## Step 4: TODO - timezone converting, maybe should be optional ## (Optional) Step 5: Additional configurations
Expand for configurations diff --git a/dbt_project.yml b/dbt_project.yml index f703bba..9a556b9 100644 --- a/dbt_project.yml +++ b/dbt_project.yml @@ -8,11 +8,11 @@ models: +materialized: table intermediate: +materialized: ephemeral - shopify__customer_email_rollup: + int_shopify__customer_email_rollup: +materialized: view # so we can use the dbt_utils.star macro - shopify__inventory_level__aggregates: + int_shopify__inventory_level__aggregates: +materialized: table # just for testing. remove later - shopify__products__with_aggregates: + int_shopify__products_with_aggregates: +materialized: table # just for testing. remove later vars: shopify: diff --git a/models/intermediate/shopify__customer_email_rollup.sql b/models/intermediate/int_shopify__customer_email_rollup.sql similarity index 100% rename from models/intermediate/shopify__customer_email_rollup.sql rename to models/intermediate/int_shopify__customer_email_rollup.sql diff --git a/models/intermediate/shopify__emails__order_aggregates.sql b/models/intermediate/int_shopify__emails__order_aggregates.sql similarity index 100% rename from models/intermediate/shopify__emails__order_aggregates.sql rename to models/intermediate/int_shopify__emails__order_aggregates.sql diff --git a/models/intermediate/shopify__inventory_level__aggregates.sql b/models/intermediate/int_shopify__inventory_level__aggregates.sql similarity index 96% rename from models/intermediate/shopify__inventory_level__aggregates.sql rename to models/intermediate/int_shopify__inventory_level__aggregates.sql index 82e6c66..f879ffc 100644 --- a/models/intermediate/shopify__inventory_level__aggregates.sql +++ b/models/intermediate/int_shopify__inventory_level__aggregates.sql @@ -40,7 +40,7 @@ joined as ( order_lines.order_line_id, order_lines.variant_id, order_lines.source_relation, - coalesce(fulfillment.location_id, orders.location_id) as location_id, + fulfillment.location_id, -- location id is stored in fulfillment rather than order orders.order_id, orders.customer_id, lower(orders.email) as email, diff --git a/models/intermediate/shopify__products__with_aggregates.sql b/models/intermediate/int_shopify__products_with_aggregates.sql similarity index 100% rename from models/intermediate/shopify__products__with_aggregates.sql rename to models/intermediate/int_shopify__products_with_aggregates.sql diff --git a/models/intermediate/intermediate.yml b/models/intermediate/intermediate.yml index 1d2ffaa..401b3b5 100644 --- a/models/intermediate/intermediate.yml +++ b/models/intermediate/intermediate.yml @@ -14,19 +14,19 @@ models: - order_id - source_relation - name: shopify__orders__order_refunds - - name: shopify__emails__order_aggregates + - name: int_shopify__emails__order_aggregates tests: - dbt_utils.unique_combination_of_columns: combination_of_columns: - email - source_relation - - name: shopify__customer_email_rollup + - name: int_shopify__customer_email_rollup tests: - dbt_utils.unique_combination_of_columns: combination_of_columns: - email - source_relation - - name: shopify__inventory_level__aggregates + - name: int_hopify__inventory_level__aggregates tests: - dbt_utils.unique_combination_of_columns: combination_of_columns: diff --git a/models/shopify__customer_emails.sql b/models/shopify__customer_emails.sql index 1e9be41..1b70210 100644 --- a/models/shopify__customer_emails.sql +++ b/models/shopify__customer_emails.sql @@ -1,13 +1,13 @@ with customer_emails as ( select - {{ dbt_utils.star(from=ref('shopify__customer_email_rollup'), except=["orders_count", "total_spent"]) }} - from {{ ref('shopify__customer_email_rollup') }} + {{ dbt_utils.star(from=ref('int_shopify__customer_email_rollup'), except=["orders_count", "total_spent"]) }} + from {{ ref('int_shopify__customer_email_rollup') }} ), orders as ( select * - from {{ ref('shopify__emails__order_aggregates' )}} + from {{ ref('int_shopify__emails__order_aggregates' )}} ), joined as ( diff --git a/models/shopify__inventory_levels.sql b/models/shopify__inventory_levels.sql index 910e093..489e4ec 100644 --- a/models/shopify__inventory_levels.sql +++ b/models/shopify__inventory_levels.sql @@ -31,7 +31,7 @@ product as ( inventory_level_aggregated as ( select * - from {{ ref('shopify__inventory_level__aggregates') }} + from {{ ref('int_shopify__inventory_level__aggregates') }} ), joined_info as ( diff --git a/models/shopify__order_lines.sql b/models/shopify__order_lines.sql index 8e22b0a..1b1f8de 100644 --- a/models/shopify__order_lines.sql +++ b/models/shopify__order_lines.sql @@ -8,7 +8,6 @@ with order_lines as ( select * from {{ var('shopify_product_variant') }} -{% if fivetran_utils.enabled_vars(vars=["shopify__using_order_line_refund", "shopify__using_refund"]) %} ), refunds as ( select * @@ -23,19 +22,16 @@ with order_lines as ( sum(coalesce(subtotal, 0)) as subtotal from refunds group by 1,2 -{% endif %} ), joined as ( select order_lines.*, - {% if fivetran_utils.enabled_vars(vars=["shopify__using_order_line_refund", "shopify__using_refund"]) %} coalesce(refunds_aggregated.quantity,0) as refunded_quantity, coalesce(refunds_aggregated.subtotal,0) as refunded_subtotal, order_lines.quantity - coalesce(refunds_aggregated.quantity,0) as quantity_net_refunds, order_lines.pre_tax_price - coalesce(refunds_aggregated.subtotal,0) as subtotal_net_refunds, - {% endif %} product_variants.created_timestamp as variant_created_at, product_variants.updated_timestamp as variant_updated_at, @@ -60,14 +56,12 @@ with order_lines as ( product_variants.option_3 as variant_option_3, product_variants.tax_code as variant_tax_code {# , - use inventoryitem or order line itself + use inventoryitem or order line itself -- add later TODO product_variants.is_requiring_shipping as variant_is_requiring_shipping #} from order_lines - {% if fivetran_utils.enabled_vars(vars=["shopify__using_order_line_refund", "shopify__using_refund"]) %} left join refunds_aggregated on refunds_aggregated.order_line_id = order_lines.order_line_id and refunds_aggregated.source_relation = order_lines.source_relation - {% endif %} left join product_variants on product_variants.variant_id = order_lines.variant_id and product_variants.source_relation = order_lines.source_relation diff --git a/models/shopify__orders.sql b/models/shopify__orders.sql index 6e80b81..599e31c 100644 --- a/models/shopify__orders.sql +++ b/models/shopify__orders.sql @@ -8,7 +8,6 @@ with orders as ( select * from {{ ref('shopify__orders__order_line_aggregates') }} -{% if var('shopify__using_order_adjustment', true) %} ), order_adjustments as ( select * @@ -22,7 +21,6 @@ with orders as ( sum(tax_amount) as order_adjustment_tax_amount from order_adjustments group by 1,2 -{% endif %} ), refunds as ( @@ -44,18 +42,14 @@ with orders as ( orders.*, coalesce(cast({{ fivetran_utils.json_parse("total_shipping_price_set",["shop_money","amount"]) }} as {{ dbt.type_float() }}) ,0) as shipping_cost, - {% if var('shopify__using_order_adjustment', true) %} order_adjustments_aggregates.order_adjustment_amount, order_adjustments_aggregates.order_adjustment_tax_amount, - {% endif %} refund_aggregates.refund_subtotal, refund_aggregates.refund_total_tax, (orders.total_price - {% if var('shopify__using_order_adjustment', true) %} + coalesce(order_adjustments_aggregates.order_adjustment_amount,0) + coalesce(order_adjustments_aggregates.order_adjustment_tax_amount,0) - {% endif %} - coalesce(refund_aggregates.refund_subtotal,0) - coalesce(refund_aggregates.refund_total_tax,0)) as order_adjusted_total, order_lines.line_item_count from orders @@ -66,11 +60,9 @@ with orders as ( left join refund_aggregates on orders.order_id = refund_aggregates.order_id and orders.source_relation = refund_aggregates.source_relation - {% if var('shopify__using_order_adjustment', true) %} left join order_adjustments_aggregates on orders.order_id = order_adjustments_aggregates.order_id and orders.source_relation = order_adjustments_aggregates.source_relation - {% endif %} ), windows as ( diff --git a/models/shopify__products.sql b/models/shopify__products.sql index 567a51b..21a8c81 100644 --- a/models/shopify__products.sql +++ b/models/shopify__products.sql @@ -1,7 +1,7 @@ with products as ( select * - from {{ ref('shopify__products__with_aggregates') }} + from {{ ref('int_shopify__products_with_aggregates') }} ), order_lines as (