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

Adds support for syncing deal pipelines #50

Closed
wants to merge 4 commits into
base: master
from

Conversation

Projects
None yet
3 participants
@jaeger401
Contributor

jaeger401 commented Sep 29, 2017

Please let me know if anything need to be adjusted in this -- deal pipelines don't have any timestamps on them, so they don't appear to have a precedent among the other data currently supported by this tap. I took a shot at what I think would be the best approach. Thanks!

@cmerrick

This comment has been minimized.

Contributor

cmerrick commented Sep 29, 2017

Hi @jaeger401, thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes.

@cmerrick

This comment has been minimized.

Contributor

cmerrick commented Sep 29, 2017

You did it @jaeger401!

Thank you for signing the Singer Contribution License Agreement.

@cmerrick cmerrick removed the cla-missing label Sep 29, 2017

@psantacl

This comment has been minimized.

Contributor

psantacl commented Oct 3, 2017

It looks like the deal_pipeline schema you provide is malformed at the declaration of the stages array. A json schem validator I found online produced the following notes:

[ {
  "level" : "warning",
  "schema" : {
    "loadingURI" : "#",
    "pointer" : "/properties/stages/items"
  },
  "domain" : "syntax",
  "message" : "the following keywords are unknown and will be ignored: [active, closedWon, displayOrder, label, probability, stageId]",
  "ignored" : [ "active", "closedWon", "displayOrder", "label", "probability", "stageId" ]
} ]
@psantacl

This comment has been minimized.

Contributor

psantacl commented Oct 3, 2017

Please run this new code along with the target in the dry-run mode to verify the schema emits the data you are looking for. If you need help running the target alongside the tap, please ask around in slack.

jaeger401 added some commits Sep 28, 2017

Appends undocumented `staticDefault` field to schema
This field is being returned in results from the `/deals/v1/pipelines`
endpoint, even though it is not documented in the API:

https://developers.hubspot.com/docs/methods/deal-pipelines/overview

This commit adds it as a nullable field.
@jaeger401

This comment has been minimized.

Contributor

jaeger401 commented Oct 20, 2017

I have corrected the schema (complicated by the fact that the first online validator I used found no issues), added an undocumented field, and corrected a typo. Piping this tap into target-csv produces results that appear to be sane. Care to have a second look, @psantacl?

@psantacl

This comment has been minimized.

Contributor

psantacl commented Oct 31, 2017

merged and released

@psantacl psantacl closed this Oct 31, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment