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

✨ Source Chargebee: add new fields in streams #32098 #32100

Closed
wants to merge 8 commits into from

Conversation

KimPlv
Copy link
Contributor

@KimPlv KimPlv commented Nov 2, 2023

Issue : #32097

What

Add new fields:

addon:

  • channel
  • object
  • type

coupons:

  • object

credit_notes

  • base_currency_code
  • business_entity_id
  • channel
  • exchange_rate
  • is_digital
  • is_vat_moss_registered
  • object

customers:

  • business_entity_id
  • channel
  • object

event:

  • object

invoices:

  • base_currency_code

channel

  • object

item:

  • external_name

orders:

  • base_currency_code
  • business_entity_id
  • exchange_rate
  • object

payment_source:

  • object

plan:

  • channel
  • charge_model
  • object

promotional_credits:

  • object

subscriptions:

  • business_entity_id
  • channel

transactions:

  • base_currency_code
  • business_entity_id
  • error_text
  • object
  • payment_method_details
  • reference_number

How

update schema folders

Copy link

vercel bot commented Nov 2, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment
Name Status Preview Comments Updated (UTC)
airbyte-docs ⬜️ Ignored (Inspect) Visit Preview Dec 19, 2023 9:48am

Copy link
Contributor

github-actions bot commented Nov 2, 2023

Before Merging a Connector Pull Request

Wow! What a great pull request you have here! 🎉

To merge this PR, ensure the following has been done/considered for each connector added or updated:

  • PR name follows PR naming conventions
  • Breaking changes are considered. If a Breaking Change is being introduced, ensure an Airbyte engineer has created a Breaking Change Plan.
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • You've updated the connector's metadata.yaml file any other relevant changes, including a breakingChanges entry for major version bumps. See metadata.yaml docs
  • Secrets in the connector's spec are annotated with airbyte_secret
  • All documentation files are up to date. (README.md, bootstrap.md, docs.md, etc...)
  • Changelog updated in docs/integrations/<source or destination>/<name>.md with an entry for the new version. See changelog example
  • Migration guide updated in docs/integrations/<source or destination>/<name>-migrations.md with an entry for the new version, if the version is a breaking change. See migration guide example
  • If set, you've ensured the icon is present in the platform-internal repo. (Docs)

If the checklist is complete, but the CI check is failing,

  1. Check for hidden checklists in your PR description

  2. Toggle the github label checklist-action-run on/off to re-run the checklist CI.

@octavia-squidington-iii octavia-squidington-iii added the area/documentation Improvements or additions to documentation label Nov 2, 2023
@KimPlv KimPlv marked this pull request as draft November 2, 2023 16:28
@KimPlv KimPlv changed the title ✨ Source Chargebee: add new field in Invoice stream #32098 ✨ Source Chargebee: add new fields in streams #32098 Nov 2, 2023
@KimPlv KimPlv marked this pull request as ready for review November 2, 2023 18:01
@KimPlv KimPlv force-pushed the chargebee_add_new_field branch from dde5cb1 to b5bd770 Compare November 2, 2023 18:03
@marcosmarxm marcosmarxm added the gl label Nov 7, 2023
Copy link
Member

@marcosmarxm marcosmarxm left a comment

Choose a reason for hiding this comment

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

Can you bump the metadata, dockerfile and doc changelog version?

@marcosmarxm
Copy link
Member

Thanks for the contribution @KimPlv is it possible to give me access to your branch? I need to send some updates for expected records.

@KimPlv
Copy link
Contributor Author

KimPlv commented Nov 7, 2023

Can you bump the metadata, dockerfile and doc changelog version?

Update for the DockerImageTag in metadata.yaml
Changelog version in doc already increased
No Dockerfile

@KimPlv
Copy link
Contributor Author

KimPlv commented Nov 7, 2023

Thanks for the contribution @KimPlv is it possible to give me access to your branch? I need to send some updates for expected records.

You should be able to access our repo, it's https://github.com/sendinblue/airbyte/tree/chargebee_add_new_field

@lideke
Copy link
Contributor

lideke commented Nov 17, 2023

Hi @marcosmarxm,
We are facing more issues with chargebee where some fields are missing.
Right now i'm switching to our build but it will be great if you can accept the PR.
Thanks

@lideke lideke force-pushed the chargebee_add_new_field branch 2 times, most recently from b9eeeeb to 53f07c3 Compare November 17, 2023 21:36
@marcosmarxm
Copy link
Member

Hello @KimPlv sorry the delay to get this merged. This week the codebase is freeze because of holidays. I'll run tests and return to you, thanks for the patience.

@lideke
Copy link
Contributor

lideke commented Nov 23, 2023

Hello @KimPlv sorry the delay to get this merged. This week the codebase is freeze because of holidays. I'll run tests and return to you, thanks for the patience.

Hi @marcosmarxm, Kim she on vacation, i'll follow the PR
Just to let you know we are already using in our side on production for missing fields in chargebee streams
Thanks you to take time to test and validate the PR

@pnilan
Copy link
Contributor

pnilan commented Dec 14, 2023

@lideke @KimPlv I can't seem to find any record of is_vat_moss_registered in the Chargebee API documentation for credits_notes. Also does not show up in the sample responses. Can you verify this field for the credits_notes stream? Thanks.

"null"
]
},
"is_vat_moss_registered": {
Copy link
Contributor

@pnilan pnilan Dec 14, 2023

Choose a reason for hiding this comment

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

Please verify this field and its datatype.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi @pnilan, we can collect data for this field, but it depends on the record, in some cases we have it and in others not. That's why i've removed the field from the credit note schema, and added "additionalProperties" to true to include it anyway. It's good for you like this ?

Copy link
Contributor

Choose a reason for hiding this comment

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

@KimPlv Thanks for the confirmation. I confirmed with Chargebee support that this specific field is only available for accounts that are configured for EU taxes, which is why it wasn't populating on my end. I think we can go ahead and include it in the schema and add a $comment property to note this.

I will update this on my end and verify the tests are running properly and then we should be able to merge the PR. I will have to merge this as a separate PR though for our CI pipeline to run properly. I'll let you know when I have the PR submitted.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pnilan fine ! Thanks for the information. I've made the change for this field

@KimPlv KimPlv force-pushed the chargebee_add_new_field branch from 53f07c3 to 57d9171 Compare December 18, 2023 10:09
@KimPlv KimPlv deleted the chargebee_add_new_field branch January 23, 2024 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

6 participants