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

Fix exchange rate issue with automated invoicing in qbo #1325

Merged

Conversation

renintw
Copy link
Contributor

@renintw renintw commented May 23, 2024

In the 2017 discussion mentioned in the issue, it can be seen that at that time, even if you didn't provide an exchange rate when calling the QBO API, QBO would automatically fill in the exchange rate for you. So I tested in this direction in the first place but then found that if you don't include the ExchangeRate in the API call now, QBO will automatically set the value to 1: "CurrencyRef":{"value":"EUR","name":"Euro"},"ExchangeRate":1.

After searching, I found discussions indicating that the QBO invoice API does not support automatically providing the ExchangeRate (perhaps they changed the rules at some point after 2017). You have to get the value yourself and include it in the payload. So here I used the get exchange rate API also from QBO (the sample on the page doesn't work, but the API actually works..).

In addition, the local QBO connection is also fixed. It turned out that the relevant SDK was missing, and installing it solved the issue.

Fixes #1250

Screenshots

Exchange rate now correctly displays

Invoice manually created in QBO Invoice sent from WordCamp
EUR Screenshot 2024-05-23 at 20 44 34 Screenshot 2024-05-23 at 20 44 17
JPY Screenshot 2024-05-23 at 20 57 58 Screenshot 2024-05-23 at 20 57 43

How to test the changes in this Pull Request:

  1. composer update.
  2. Follow https://github.com/WordPress/wordcamp.org/tree/production/public_html/wp-content/plugins/wordcamp-qbo#wordcamp-qbo to set up QBO locally.
  3. Add a new sponsor invoice here in /wp-admin/post-new.php?post_type=wcb_sponsor_invoice.
    • If the community dropdown does not display any country options, click on "Delete Cache" and then refresh.
  4. Approve the invoice you added in /wp-admin/network/admin.php?page=sponsor-invoices-dashboard&section=submitted
  5. Go to https://app.sandbox.qbo.intuit.com/app/invoices and enter the invoice editing screen.
  6. Check if exchanges are correct. (you can test with multiple currencies)

@renintw renintw self-assigned this May 23, 2024
@renintw renintw requested a review from pkevan May 23, 2024 12:19
@renintw renintw requested a review from pkevan May 24, 2024 14:38
),
)
);
Logger\log( 'remote_request', compact( 'currency_code', 'response' ) );
Copy link
Contributor

Choose a reason for hiding this comment

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

unsure if this is debugging code left in or needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This code appears in other parts as well. You can see logs on the terminal and can also check them using wp wc-misc format-log wp-content/debug.log as specified in the QBO Readme.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It appears that it can also let you see the production logs. (ref)

Copy link
Contributor

Choose a reason for hiding this comment

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

ok - but it's kind of pointless logging every time we do a lookup isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't feel strongly about it; just following the practice here. Perhaps there was a discussion that every API call should have a logger and what benefits it would bring? I'm not sure. However, removing it here makes sense to me since it isn't a critical call. What do you think?

@renintw renintw merged commit 24557ae into production May 27, 2024
3 checks passed
@renintw renintw deleted the fix/Exchange-Rate-Issue-with-Automated-Invoicing-in-QBO branch May 27, 2024 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Exchange Rate Issue with Automated Invoicing in QBO
2 participants