Skip to content

Commit

Permalink
joe feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
fivetran-jamie committed Jan 6, 2023
1 parent 5b05c67 commit 56bc8c6
Show file tree
Hide file tree
Showing 14 changed files with 31 additions and 43 deletions.
6 changes: 2 additions & 4 deletions .buildkite/scripts/run_models.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
dbt run --vars '{shopify_timezone: "America/New_York"}' --target "$db" --full-refresh
dbt test --target "$db"
16 changes: 15 additions & 1 deletion DECISIONLOG.md
Original file line number Diff line number Diff line change
@@ -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`.
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_<foreign key table>_deleted
12 changes: 1 addition & 11 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
<details><summary>Expand for configurations</summary>
Expand Down
6 changes: 3 additions & 3 deletions dbt_project.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
6 changes: 3 additions & 3 deletions models/intermediate/intermediate.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
6 changes: 3 additions & 3 deletions models/shopify__customer_emails.sql
Original file line number Diff line number Diff line change
@@ -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 (

Expand Down
2 changes: 1 addition & 1 deletion models/shopify__inventory_levels.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down
8 changes: 1 addition & 7 deletions models/shopify__order_lines.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand All @@ -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,
Expand All @@ -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
Expand Down
8 changes: 0 additions & 8 deletions models/shopify__orders.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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 *
Expand All @@ -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 (

Expand All @@ -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
Expand All @@ -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 (

Expand Down
2 changes: 1 addition & 1 deletion models/shopify__products.sql
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
with products as (

select *
from {{ ref('shopify__products__with_aggregates') }}
from {{ ref('int_shopify__products_with_aggregates') }}
),

order_lines as (
Expand Down

0 comments on commit 56bc8c6

Please sign in to comment.