Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

fix(quickbooks): prompt user for a realmId connection config parameter #2999

Conversation

hassan254-prog
Copy link
Contributor

Describe your changes

  • Prompt user for the realmId in quickbooks.

Issue ticket number and link

Checklist before requesting a review (skip if just adding/editing APIs & templates)

  • I added tests, otherwise the reason is:
  • I added observability, otherwise the reason is:
  • I added analytics, otherwise the reason is:

Copy link

linear bot commented Nov 18, 2024

@hassan254-prog hassan254-prog self-assigned this Nov 18, 2024
@hassan254-prog hassan254-prog requested a review from a team November 18, 2024 08:00
@hassan254-prog hassan254-prog force-pushed the kelvinwari/ext-244-quickbooks-realmid-isnt-always-provided-in-the-authorization branch from c69d0c5 to 09f7664 Compare November 18, 2024 08:18

quickbooks-sandbox:
alias: quickbooks
display_name: Quickbooks (sandbox)
proxy:
connection_config:
realmId: ${connectionConfig.realmId}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think it's a follow-up question from the aws PR I didn't get, How are we supposed to use it if it's not in a URL or headers?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For the AWS-IAM, since we didn't support the AWS V4 signature authentication mode, we needed a way to prompt the user for the region. This region is then used to generate the signature within the script. For QuickBooks, the realmId is required to build the proxy base_url. However, after discussing with Khaliq, we decided it was best not to alter the base_url as doing so might break compatibility for other users.

Copy link
Collaborator

Choose a reason for hiding this comment

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

alright 👌🏻

@hassan254-prog hassan254-prog enabled auto-merge (squash) November 18, 2024 10:04
@hassan254-prog hassan254-prog merged commit e2d4ed3 into master Nov 18, 2024
20 checks passed
@hassan254-prog hassan254-prog deleted the kelvinwari/ext-244-quickbooks-realmid-isnt-always-provided-in-the-authorization branch November 18, 2024 10:14
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants