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 Salesloft: Added new streams (call data records, call dispositions, call sentiments, custom fields, meetings, searches) #27505

Merged
merged 28 commits into from Jul 11, 2023

Conversation

markortleb-dice
Copy link
Contributor

@markortleb-dice markortleb-dice commented Jun 20, 2023

What

This pull request will add the following streams to the Salesloft Source. This is the issue this work is related to: #27504.

Here are the new streams:

  • Call Data Records
  • Call Dispositions
  • Call Sentiments
  • Custom Fields
  • Meetings
  • Searches

How

These new Salesloft Source streams were added in the line with the same pattern that all previous Salesloft Source streams
have. The only stream that was a bit different was Searches, which has a POST request rather than GET (which all other
Salesloft Source streams have).

🚨 User Impact 🚨

There are no breaking changes in this 1.2.0 release. In this change, we are only adding net-new Salesloft Source streams.
We made no modifications to existing streams.

Pre-merge Actions

Updating a connector

Community member or Airbyter

  • Grant edit access to maintainers (instructions)
  • Unit & integration tests added

Airbyter

If this is a community PR, the Airbyte engineer reviewing this PR is responsible for the below items.

  • Create a non-forked branch based on this PR and test the below items on it
  • Build is successful
  • If new credentials are required for use in CI, add them to GSM. Instructions.

@CLAassistant
Copy link

CLAassistant commented Jun 20, 2023

CLA assistant check
All committers have signed the CLA.

@octavia-squidington-iii octavia-squidington-iii added area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/salesloft labels Jun 20, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Jun 20, 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 and you've followed all steps in the Breaking Changes Checklist
  • Connector version has been incremented in the Dockerfile and metadata.yaml according to our Semantic Versioning for Connectors guidelines
  • 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
  • The connector tests are passing in CI
  • You've updated the connector's metadata.yaml file (new!)
  • 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.

@markortleb-dice markortleb-dice changed the title Source Salesloft: Add new streams (call data records, call dispositions, call sentiments, custom fields, meetings, searches) #27504 Source Salesloft: Added new streams (call data records, call dispositions, call sentiments, custom fields, meetings, searches) Jun 20, 2023
Copy link
Contributor

@sajarin sajarin left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Left some comments:

@marcosmarxm marcosmarxm added the team/tse Technical Support Engineers label Jun 27, 2023
Copy link
Contributor

@sajarin sajarin left a comment

Choose a reason for hiding this comment

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

A couple of more formatting things:

Run the ./gradlew --no-daemon :airbyte-integrations:connectors:source-salesloft:airbytePythonFormat command to format the files and commit them

Bump the number of streams in the test_streams file (expected streams should be 29 now instead of 23)

@markortleb-dice
Copy link
Contributor Author

A couple of more formatting things:

Run the ./gradlew --no-daemon :airbyte-integrations:connectors:source-salesloft:airbytePythonFormat command to format the files and commit them

Bump the number of streams in the test_streams file (expected streams should be 29 now instead of 23)

Thank you for the feedback @sajarin . As requested, I was able to run that Gradle command which automatically formatted the Salesloft files, and I also changed the expected number of streams from 23 to 29.

@marcosmarxm
Copy link
Member

@markortleb-dice you must sign the CLA.

@markortleb-dice
Copy link
Contributor Author

@markortleb-dice you must sign the CLA.

CLA is now signed!

@sh4sh sh4sh mentioned this pull request Jul 5, 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.

There are some fixes need to be made in this contribution @markortleb-dice.

…om Fields to full refresh. Use created_at instead of updated_at for timestamp field for Meetings.
@markortleb-dice
Copy link
Contributor Author

There are some fixes need to be made in this contribution @markortleb-dice.

Thank you for the feedback @marcosmarxm . I have made the changes as requested:

  • Call Dispositions, Call Sentiments, and Custom Fields streams have been converted to non-incremental streams.
  • The created_at_field was changed to created_at rather than updated_at for the Meetings stream. I also converted the Meetings stream to non-incremental, since it does not seem to have a good field to use as a cursor when requesting from the API, such as updated_at. Here is a link to the docs for Meetings just fyi: https://developers.salesloft.com/docs/api/meetings-index.

@archangelic archangelic dismissed stale reviews from marcosmarxm and sajarin July 11, 2023 20:42

Stale review

@archangelic archangelic merged commit 8ff7e58 into airbytehq:master Jul 11, 2023
38 of 41 checks passed
efimmatytsin pushed a commit to scentbird/airbyte that referenced this pull request Jul 27, 2023
…ions, call sentiments, custom fields, meetings, searches) (airbytehq#27505)

* adding new Salesloft streams

* change the version to 1.1.1

* increment the patch version

* fixing typo in meetings

* fix another typo

* making corrections requested by reviewer

* increment version in salesloft metadata to 1.2.0

* reformating files

* fix expected records

* Source Salesloft: change Call Dispositions, Call Sentiments, and Custom Fields to full refresh. Use created_at instead of updated_at for timestamp field for Meetings.

* changing meetings cursor field to created_at

* Source Salesloft: Convert meetings to a non-incremental stream, since there is no viable cursor field option.

* fix schemas and update tests

---------

Co-authored-by: sh4sh <6833405+sh4sh@users.noreply.github.com>
Co-authored-by: Mal Hancock <mallory@archangelic.space>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/connectors Connector related issues area/documentation Improvements or additions to documentation community connectors/source/salesloft team/tse Technical Support Engineers
Projects
Development

Successfully merging this pull request may close these issues.

None yet

7 participants