[Bug] Bank Transaction table missing

fivetran / dbt_xero

Data models for Fivetran's Xero connector built using dbt.

https://fivetran.github.io/dbt_xero/#!/overview

Apache License 2.0

10 stars 20 forks source link

[Bug] Bank Transaction table missing #26

Closed sjmunoz closed 2 years ago

sjmunoz commented 2 years ago

Is there an existing issue for this?

[X] I have searched the existing issues

Describe the issue

One of our 3 Xero instances doesn't have the bank_transaction table and most models are skipped because of this.

Relevant error log or model output

No response

Expected behavior

If the table doesn't exist, users should be able to not include it in the models being ran through a variable, just like the credit_note

dbt Project configurations

We just use the standard configuration for this package.

models: xero: +tags: 'default_transformation_xero' +schema: "dataset" xero_source: +tags: 'default_transformation_xero' +schema: "dataset"

Package versions

package: fivetran/xero version: 0.4.0

What database are you using dbt with?

bigquery

dbt Version

version: 1.0.1

Additional Context

No response

Are you willing to open a PR to help address this issue?

[X] Yes.
[ ] Yes, but I will need assistance and will schedule time during our office hours for guidance
[ ] No.

fivetran-joemarkiewicz commented 2 years ago

Hi @santi95 thanks so much for opening this issue.

I can see how not having the bank_transaction table would cause the downstream models to fail within your project. Resolving this would be pretty straight forward where we would want to add a variable that allows users to disable the model if their connector is not actively syncing the table.

I noticed you mentioned you would be willing to open a PR for to address this issue. If you would like some guidance, you can see how we did a similar update to disable the credit_note models within this dbt_xero_source PR and the dbt_xero PR.

Let me know if you are still comfortable contributing. Otherwise, I can add it as a task for my team in a future sprint.

sjmunoz commented 2 years ago

Hey @fivetran-joemarkiewicz thank you for the quick reply.

I already coded the changes, my only question would be about repo synchronization, because this change requires both changes in the dbt_xero and the dbt_xero_source. How would it be better to submit both PRs? I was thinking on linking the dbt_xero_source changes on the main one, would this be ok?

Creating the PR soon!

fivetran-joemarkiewicz commented 2 years ago

@santi95 this is great thanks so much for opening this PR 💯!

The way you have done this is perfect with the two PRs and the reference in this issue. I will review these for you tomorrow! The only thing I will ask off the bat is:

[ ] Make sure to update the README to include this new variable for both the source/modeling packages.
[ ] Increase the index of the package version in the dbt_project.yml files
[ ] Add your contribution to the CHANGELOG.md files for both the source/modeling packages

Those are what I was able to see at a cursory glance. Everything else I will review in more detail tomorrow 😄

sjmunoz commented 2 years ago

@fivetran-joemarkiewicz Just uploaded the requested changes! Thanks for everything

fivetran-joemarkiewicz commented 2 years ago

Hey @santi95 thanks again so much for your contributions! 🙌

I just cut the release for the dbt_xero v0.4.1 and you should see the release be live on the dbt hub at the top of the hour. Thanks again for your collaboration and please feel free to open another issue or feature request in the future if you encounter any issues or would like more flexibility to the package. 😄