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.

Sign up for GitHub

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

Jump to bottom

Upgrade to V2 API #9

Closed
jordan8037310 opened this issue Sep 30, 2017 · 7 comments
Closed

Upgrade to V2 API #9

jordan8037310 opened this issue Sep 30, 2017 · 7 comments

Comments

@jordan8037310
Copy link
Contributor

jordan8037310 commented Sep 30, 2017 •
edited
Loading

V1 API from Harvest is now deprecated: https://www.getharvest.com/blog/2017/09/introducing-harvest-api-v2/

It will be supported until January 2019. Will use this ticket to track effort to upgrade.
@nickfogler
Copy link

Any thoughts about upgrading to v2? I'd really like to see the Invoice Line Items included (https://help.getharvest.com/api-v2/invoices-api/invoices/invoices/#the-invoice-line-item-object).

@suzybates
Copy link

I would also like to see the "fixed fee" fields on the Project and the Project Role data working.

StevenMHernandez pushed a commit to fostermadeco/tap-harvest that referenced this issue Jun 28, 2018
begin transition from harvest api v1 to v2
6e763aa 
* update authentication method required for new v2 of the api
* add additional required config variables for tap usage
* update access_token request to comply with v2 of api
* update schemas to reflect data returned from the v2 api
* add additional schemas to reflect additional data points available

@see https://help.getharvest.com/api-v2/
@see singer-io#9
@StevenMHernandez StevenMHernandez mentioned this issue Jun 28, 2018
Merged
dmosorast pushed a commit that referenced this issue Aug 14, 2018
@StevenMHernandez @dmosorast
Upgrade to Api v2 (#20)
ca87f8b 
* begin transition from harvest api v1 to v2

* update authentication method required for new v2 of the api
* add additional required config variables for tap usage
* update access_token request to comply with v2 of api
* update schemas to reflect data returned from the v2 api
* add additional schemas to reflect additional data points available

@see https://help.getharvest.com/api-v2/
@see #9

* generify `sync_endpoint` so that it is the only method that loads data and loops through data, thus removing redundancies

* allow each parent object loaded from the `sync_endpoint` to recursively sync additional endpoints (related to the parent object) through a `for_each_handler`
* remove api requests from the individual `sync_$item` methods, instead prefer using the generic `sync_endpoint` method

* store the id of nested objects instead of the full nested object

* automatically paginate all "collection" results (all calls to the `sync_endpoint` method)

* allow alternative key_properties, updated documentation, minor fixes

* allow key_properties to be set to anything other than just `["id"]` for use with pivot table style data
* update config.json and state.json documentation in README.md
* add additional schemas and subsequent loading-code that was missed in the first pass
* additional minor fixes such as
  * incorrect schema attribute names
  * missing attributes
  * incorrect schema attribute type/formats
* minor formatting tweaks

* skip syncing any endpoints that are not enabled by the given user's company, otherwise the user receives a 403 response error

* load company metadata from harvest api
* ensure specific features are either enabled or disabled before beginning endpoint syncs

* update module to v2.0.0, minor tweaks

* define default key_properties directly in `load_and_write_schema` method signature
* update version
* fix other minor inconsistencies

* determine account_id automatically as opposed to requiring account_id from user config
@awarzon
Copy link

awarzon commented Aug 26, 2018

Thanks for this! I'm testing the latest release since it was pushed last week and ran into an error, see below. It appears that Harvest allows strings into the external_reference.group_id field and the Github/Harvest integration puts string data in.

2018-08-26 21:20:55,308Z    tap - CRITICAL Errors during transform
2018-08-26 21:20:55,308Z    tap - 	group_id: awme does not match {'type': ['integer', 'null']}
2018-08-26 21:20:55,308Z    tap - 	: {'id': '35', 'group_id': 'awme', 'service': 'github.com', 'permalink': 'https://github.com/<REMOVED>/issues/35', 'service_icon_url': 'https://proxy.harvestfiles.com/production_harvestapp_public/uploads/platform_icons/gitpro.ttaallkk.top.png?1349823336'} does not match {'properties': {'id': {'type': ['string', 'null']}, 'permalink': {'type': ['string', 'null']}, 'service_icon_url': {'type': ['string', 'null']}, 'task_id': {'type': ['null', 'integer']}, 'group_id': {'type': ['integer', 'null']}, 'service': {'type': ['string', 'null']}}, 'type': 'object'}

@dmosorast
Copy link
Contributor

@awarzon Thanks for posting! These kinds of unexpected data/schema issues are being shaken out right now, so this is very helpful to keep the testing moving forward.

I've changed the schema of group_id to be a string in light of this (#24), and will release it in version 2.0.4 shortly.

@tsmith512
Copy link

Related to #16 , time entry sync doesn't propagate deletions. I'm in the process of trying to write a little side cron job that will inspect the API response and delete rows out of our Stitch-populated tables that have been deleted in Harvest, but my approach seems brittle. I haven't read up much on the APIv2 yet, but is there a way deletions could be handled in this upgrade?

@awarzon
Copy link

awarzon commented Aug 31, 2018

Thanks @dmosorast !

@dmosorast
Copy link
Contributor

Closing this as V2 has been merged in #20

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
6 participants
@awarzon @nickfogler @tsmith512 @jordan8037310 @suzybates @dmosorast