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

New model for a purchase order reporting structure #24

Closed
wants to merge 12 commits into from
Closed

New model for a purchase order reporting structure #24

wants to merge 12 commits into from

Conversation

DJFresh66
Copy link

Are you a current Fivetran customer?
Doug Jeffery, Modulous

What change(s) does this PR introduce?
Adds required model for a purchase order reporting structure (follows the invoice model 'pattern')

Does this PR introduce a breaking change?

  • Yes (please provide breaking change details below.)
  • No - New models only, existing models are untouched

Is this PR in response to a previously created Issue

How did you test the PR changes?

  • CircleCi
  • Other Using Fivetran transformations against our live Xero synch, manual tests in SQL to validate the volume and contents of the reporting table versus the source tables and the raw data. Also created a detailed download CSV and summary CSV which was QA'd against Xero by my internal customer (our CFO)

Select which warehouse(s) were used to test the PR

  • BigQuery
  • Redshift
  • Snowflake
  • Postgres
  • Databricks
  • Other (provide details below)

Provide an emoji that best describes your current mood
😎
Feedback

We are so excited you decided to contribute to the Fivetran community dbt package! We continue to work to improve the packages and would greatly appreciate your feedback on our existing dbt packages or what you'd like to see next.

@fivetran-joemarkiewicz fivetran-joemarkiewicz added the enhancement New feature or request label Jan 25, 2022
Copy link
Contributor

@fivetran-joemarkiewicz fivetran-joemarkiewicz left a comment

Choose a reason for hiding this comment

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

@DJFresh66 the updates you are suggesting in this PR make sense and look great to me! Before we can merge this into the next release branch, I have a quick request from you, and a few notices as to what I will change before release.

One request for you to add

  • For our CircleCi testing we require a dummy csv table for the new source models you are adding. Would you be able to add two dummy csv tables to the integration_tests/seeds location. Here is a good example of what we are looking for.

Changes I will make once the seed data is added

  • Apply a variable for users to enable the purchase order models. I checked and it seems only 40% of Xero users have these models. Therefore, we will want to disable them by default and allow users to enable them if they would like.
  • Keep the versioning to be within dbt v1. Since the latest version of the package is compatible with dbt v1, we do not want to decrease the dbt range.

@DJFresh66
Copy link
Author

Hi @fivetran-joemarkiewicz sorry for the delay getting back to you, I should be able to add the csvs as requested this week, cheers

@Project-Rouge Project-Rouge closed this by deleting the head repository Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants