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

intuit: Create generic get and post http helpers #949

Merged
merged 11 commits into from
Jan 30, 2025

Conversation

hunterachieng
Copy link
Contributor

@hunterachieng hunterachieng commented Jan 28, 2025

Summary

Implement a new adaptor: intuit with generic get and post http helpers

Fixes #827

AI Usage

Please disclose how you've used AI in this work (it's cool, we just want to
know!):

  • Code generation (copilot but not intellisense)
  • Learning or fact checking
  • Strategy / design
  • Optimisation / refactoring
  • Translation / spellchecking / doc gen
  • Other
  • I have not used AI

You can read more details in our
Responsible AI Policy

Review Checklist

Before merging, the reviewer should check the following items:

  • Does the PR do what it claims to do?
  • If this is a new adaptor, added the adaptor on marketing website ?
  • If this PR includes breaking changes, do we need to update any jobs in
    production? Is it safe to release?
  • Are there any unit tests?
  • Is there a changeset associated with this PR? Should there be? Note that
    dev only changes don't need a changeset.
  • Have you ticked a box under AI Usage?

packages/intuit/src/Utils.js Outdated Show resolved Hide resolved
packages/intuit/test/Adaptor.test.js Outdated Show resolved Hide resolved
packages/intuit/test/Adaptor.test.js Show resolved Hide resolved
packages/intuit/src/http.js Outdated Show resolved Hide resolved
packages/intuit/src/Utils.js Outdated Show resolved Hide resolved
@josephjclark
Copy link
Collaborator

@hunterachieng if you need to update the template please do it in a separate PR :)

And when you're ready for another review, please mark the PR as Ready

@hunterachieng hunterachieng marked this pull request as ready for review January 29, 2025 15:26
@hunterachieng
Copy link
Contributor Author

@josephjclark We do not need to update the template as post is written correctly

Copy link
Collaborator

@josephjclark josephjclark left a comment

Choose a reason for hiding this comment

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

Just a couple of small things!

packages/intuit/README.md Show resolved Hide resolved
"baseUrl": {
"title": "Base URL",
"type": "string",
"description": "The Quickbooks(intuit) base URL",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we default this? Can users even us different domains?

If our implementation is on the main intuit.com baseUrl, we should default it and leave this as optional

packages/intuit/src/Utils.js Show resolved Hide resolved
packages/intuit/src/Utils.js Outdated Show resolved Hide resolved
Signed-off-by: Hunter Achieng <[email protected]>
@@ -0,0 +1,6 @@
---
'@openfn/language-intuit': major
Copy link
Collaborator

Choose a reason for hiding this comment

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

changeset isn't needed for new adaptors - please delete this file and then we can merge

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Signed-off-by: Hunter Achieng <[email protected]>
@josephjclark josephjclark merged commit f033e36 into main Jan 30, 2025
1 check passed
@josephjclark josephjclark deleted the feature/intuit-adaptor-827 branch January 30, 2025 11:24
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.

New "Intuit" adaptor for Quickbooks Online
2 participants