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

TDL-19422 add new streams #34

Merged
merged 73 commits into from Feb 16, 2023
Merged

TDL-19422 add new streams #34

merged 73 commits into from Feb 16, 2023

Conversation

kethan1122
Copy link
Contributor

@kethan1122 kethan1122 commented Jan 31, 2023

Description of change

This PR adds three new streams for tap-helpscout:

  1. teams Doc
  2. team_users Doc
  3. happiness_ratings Doc

Manual QA steps

  • Generated test data for streams.
  • Ran discovery mode to make sure metadata file has all the new tables.
  • Ran sync mode to extract the data

Risks

  • Low, Couldn't test pagination tests due to lack of test data

Rollback steps

  • revert this branch

Copy link

@dsprayberry dsprayberry left a comment

Choose a reason for hiding this comment

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

Note I'm not approving this PR until the refactor is merged into master and this PR is against master.

README.md Outdated Show resolved Hide resolved
Comment on lines 8 to 9
"conversation_id": {
"type": ["null", "integer"]

Choose a reason for hiding this comment

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

This is a key_property; is it actually possible for it to be null? If not, we should remove the null type.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm going to remove null because I'm 100% confident that these fields can never null from source API response.
I observed for most of the integrations we included null for PKs, I feel this is just to avoid unwanted loading errors if by mistake any field comes as null from source API

Examples:

Comment on lines 21 to 22
"rating_customer_id": {
"type": ["null", "integer"]

Choose a reason for hiding this comment

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

This is a key_property; is it actually possible for it to be null? If not, we should remove the null type.

Comment on lines 30 to 31
"rating_created_at": {
"type": ["null", "string"],

Choose a reason for hiding this comment

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

This is a key_property; is it actually possible for it to be null? If not, we should remove the null type.

tap_helpscout/streams/happiness_ratings.py Outdated Show resolved Hide resolved
tap_helpscout/schemas/happiness_ratings.json Outdated Show resolved Hide resolved
tap_helpscout/transform.py Outdated Show resolved Hide resolved
tap_helpscout/transform.py Outdated Show resolved Hide resolved
tap_helpscout/transform.py Show resolved Hide resolved
tap_helpscout/transform.py Show resolved Hide resolved
@kethan1122 kethan1122 changed the base branch from TDL-21651-code-refactoring to master February 7, 2023 07:58
@kethan1122 kethan1122 merged commit 93e2c8f into master Feb 16, 2023
@kethan1122 kethan1122 deleted the TDL-19422-add-new-streams branch February 16, 2023 17:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants