-
Notifications
You must be signed in to change notification settings - Fork 103
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
Quickbooks plugin major new improvements and features by timbze and marcbou #230
base: master
Are you sure you want to change the base?
Quickbooks plugin major new improvements and features by timbze and marcbou #230
Conversation
- Implemented automatic Quickbook Income Account number lookup based on Account Name configured by the user. New configuration parameter qbIncomeAccountName replaces qbIncomeAccountNumber. - Implemented clarkraymond's excellent idea of using the type from the UCRM Invoice object (service, product, surcharge, other etc) as the item name for QBO instead of creating thousands of unnecessary items. The QBO item name is generated using a new configuration parameter, "itemNameFormat" which can be typically set to something like "YourWisp %s" or "UCRM %s" to end up with items named "UCRM service", "UCRM product" etc.. - Added preliminary support for Canadian GST (Federal) and Provincial (Quebec) taxes. TaxCodes in the US version of QBO are defined globally for the invoice, whereas in International versions they must be looked up and specified for each item. - Added qbApiDelay with usleep() before each API call to avoid triggering Intuit's throttling limits for APIs. See https://developer.intuit.com/app/developer/qbo/docs/develop/rest-api-features#limits-and-throttles - Improved logging throughout - Added the possibility of cleanly stopping the plugin during operation (or preventing certain phases from running) by creating "stop files" under data/ directory. The files are "stop" (stops everything), "stopclients" (stops clients being exported), "stopinvoices" (stops invoices being exported), and "stoppayments" (stops payments being exported). This is very handy during development/testing too. - Fixed incorrect UCRMID pattern for client matching occasionally causing incorrect/multiple matches: SELECT * FROM Customer WHERE DisplayName LIKE \'%%UCRMID-%d%%\ should be much more robust with LIKE \'%% (UCRMID-%d)%%\ - Changed QBO invoice numbering scheme so that it is based on UCRM Invoice number, a "/" separator and then the internal UCRM invoice ID. This makes it much easier to find/track UCRM invoices in QBO, separately sort them etc..
- Improved payment processing - Log warning for refunds (which are not yet supported)
b5dfb57
to
1ddeef1
Compare
1ddeef1
to
8207c87
Compare
8207c87
to
0f7c93c
Compare
0f7c93c
to
73196d1
Compare
Do you know when this update would go live in UCRM? |
|
Has been in production for close to a month now and working great. One additional improvement needed that I've made an issue for. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hello @timbze, thank you for the PR!
I don't have much time now, so I just skimmed through the code and commented on some minor issues I've noticed.
I'll review the code properly when I've got some time and we'll do some testing, but so far it looks good.
private function getItems($items, int $qbIncomeAccountId, bool $negateQty, DataService $dataService, PluginData $pluginData): array { | ||
$lines = []; | ||
foreach ($items as $item) { | ||
$qbItem = $this->itemCache[$item['label']]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will throw an undefined index error if the item doesn't exist in cache, you can use null coalesce operator to handle it
$qbItem = $this->itemCache[$item['label']]; | |
$qbItem = $this->itemCache[$item['label']] ?? false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the code without issue. I thought what I found with PHP (didn't have experience in PHP before) was that checking if (! $qbItem)
like I did in L668 returns false if it's null or if it's false. I thought this type of check was used constantly for null throughout the file.
But I could still change it if you want me to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, not an error, just a notice, I'm too used to our base code converting even notices to errors. :) However even if it works, it's still bad practice.
You can check this example code over at https://3v4l.org/I0uOJ
<?php
$itemCache = [];
$qbItem = $itemCache['foo'];
if (! $qbItem) {
var_dump('foo works, but shows notice');
}
$qbItem = $itemCache['bar'] ?? false;
if (! $qbItem) {
var_dump('bar works and no notice');
}
$qbItem = array_key_exists('foobar', $itemCache) ? $itemCache['foobar'] : false;
if (! $qbItem) {
var_dump('foobar works and no notice');
}
$qbItem = isset($itemCache['foo2']) ? $itemCache['foo2'] : false;
if (! $qbItem) {
var_dump('foo2 works and no notice, but hides falsey values too, see foo3');
}
$itemCache = ['foo3' => 0];
$qbItem = isset($itemCache['foo3']) ? $itemCache['foo3'] : false;
if (! $qbItem) {
var_dump('foo3 has no notice, but hides falsey values');
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
plugins/quickbooks-online/README.md
Outdated
- Remove entity from QB when the related entity is deleted in UCRM. | ||
|
||
## Changelog | ||
### 1.1.5 (2021-12-01) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably should be "2.0.0", same as in the manifest.
### 1.1.5 (2021-12-01) | |
### 2.0.0 (2021-12-01) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
return [$lineArray, $additionalUnapplied, $totalApplied]; | ||
} | ||
|
||
private $itemCache = []; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please put class variables to the beginning of file, don't declare new variables in the middle of methods. If you want to have variables near the methods using them, it's usually a good idea to extract the variables and the method to a new class, it even makes the code more readable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
I'm very close to getting this to work but I'm having trouble finding the income account id in QuickBooks online. Where do you find the income account id? |
The |
Thank you for the reply. I guess I'm just confused about what to put for either the income account id or the income account name. Is it the company name from quickbooks? Or the name of bank account where the payments are going to? Sorry for the dumb questions I'm just trying to automate sending invoices from UISP CRM to quickbooks online. I appreciate your help. |
Did you follow these instructions? It looks like the app has not been properly authorized with QB yet |
Are your app keys from the "Production Settings" section? I see you've set your plugin to use "Production" (according to first screenshot). You'll need to set that to "Development" if you took the development settings keys. |
I have now updated dependencies and made sure this works with UISP 2.3.57 (lastest as of now) |
This was started off of #117, but many improvements were made to it, and additional features.
If there are specific parts of this that should be separate pull requests I can try to separate them sometime.
This seems to be working, but definitely needs some more testing. I may find minor bugs to fix yet over the coming weeks.
Here are the main features in this PR. They mostly come from these issues:
Export Credit Memos. Set the
Date to start for Credit Memo export
value in the plugin config so that older credits that you have already entered will not export.Don't generate new items all the time. Re-use the same items.
Make it possible to use
Income account name
instead ofIncome account ID
in plugin configExport phone # and email address for new clients.
Cleaner payment exports. Only export 1 transaction per payment.
Export payment method, and optionally "Deposit to" account, to QBO along with the payment
Logs get cleaned out so that only 10,000 rows are kept after file grows to over 1 Mb. This improves plugin interaction speed.
Add better error handling so that network hiccups don't cause individual transactions to fail that easily.
Make it possible to set logging level dynamically from plugin config
update the information.version in manifest.json. Increased major version because credit memos are now exported and the user should add a date from when credit memos should export before running the plugin.
in the
plugins/
directory, runphp ../pack-plugin.php $YOUR_PLUGIN
, where $YOUR_PLUGIN is the directory name of your plugin; this creates the installable packagecomposer.json
, this will check it and install the dependencies - if this reports as "outdated", please fix by runningcomposer update
in your plugin's directorycommit the changes made during these steps
in the project root directory, run
php validate.php
php generate-json.php > plugins.json
, but it still saysThe "plugins.json" file is not up to date. Run
php generate-json.php > plugins.jsonto update it.
in the project root directory, run
bash php-cs-check.sh
to check for valid PHP codePHP needs to be a minimum version of PHP 5.6.0 and maximum version of PHP 7.3.*.
. I don't know if this is something I can or should fix