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

Quickbooks plugin major new improvements and features by timbze and marcbou #230

Open
wants to merge 47 commits into
base: master
Choose a base branch
from

Conversation

timbze
Copy link

@timbze timbze commented Dec 1, 2021

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 of Income account ID in plugin config

  • Export 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, run php ../pack-plugin.php $YOUR_PLUGIN, where $YOUR_PLUGIN is the directory name of your plugin; this creates the installable package

    • if you have a composer.json, this will check it and install the dependencies - if this reports as "outdated", please fix by running composer update in your plugin's directory
    • normally, this should finish without errors or warnings
  • commit the changes made during these steps

  • in the project root directory, run php validate.php

    • I run php generate-json.php > plugins.json, but it still says The "plugins.json" file is not up to date. Run php generate-json.php > plugins.json to update it.
  • in the project root directory, run bash php-cs-check.sh to check for valid PHP code

    • I get PHP 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

Marc Boucher and others added 30 commits November 3, 2021 05:40
- 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)
@timbze timbze force-pushed the new_features_timbze_and_marcbou branch from b5dfb57 to 1ddeef1 Compare December 2, 2021 14:16
@timbze timbze force-pushed the new_features_timbze_and_marcbou branch from 1ddeef1 to 8207c87 Compare December 2, 2021 14:22
@timbze timbze force-pushed the new_features_timbze_and_marcbou branch from 8207c87 to 0f7c93c Compare December 2, 2021 14:42
@mshaw101
Copy link

Do you know when this update would go live in UCRM?

@timbze
Copy link
Author

timbze commented Jan 11, 2022

Do you know when this update would go live in UCRM?

@keksa @drlikm @janprochazkacz

@timbze
Copy link
Author

timbze commented Jan 11, 2022

This seems to be working, but definitely needs some more testing. I may find minor bugs to fix yet over the coming weeks.

Has been in production for close to a month now and working great. One additional improvement needed that I've made an issue for.

Copy link
Collaborator

@keksa keksa left a 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']];
Copy link
Collaborator

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

Suggested change
$qbItem = $this->itemCache[$item['label']];
$qbItem = $this->itemCache[$item['label']] ?? false;

Copy link
Author

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

Copy link
Collaborator

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');
}

Copy link
Author

Choose a reason for hiding this comment

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

Done

- Remove entity from QB when the related entity is deleted in UCRM.

## Changelog
### 1.1.5 (2021-12-01)
Copy link
Collaborator

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.

Suggested change
### 1.1.5 (2021-12-01)
### 2.0.0 (2021-12-01)

Copy link
Author

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 = [];
Copy link
Collaborator

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.

Copy link
Author

Choose a reason for hiding this comment

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

Done

@JonnyTrexler
Copy link

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?

@timbze
Copy link
Author

timbze commented Jul 7, 2022

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 id can not be seen in QB online as far as I know. While you set up the plugin it prints all the accounts with ID in the log. In this PR I have also made it possible to simply use the account name instead of ID.

@JonnyTrexler
Copy link

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.

@timbze
Copy link
Author

timbze commented Jul 7, 2022

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.

If you use the "Income account name" you would need to use "Design income" in this screenshot (from chart of accounts)
image

@timbze timbze requested a review from keksa July 7, 2022 17:12
@JonnyTrexler
Copy link

image
This is what happens whenever I execute the plugin. I am currently testing in a QuickBooks sandbox

@timbze
Copy link
Author

timbze commented Jul 7, 2022

This is what happens whenever I execute the plugin. I am currently testing in a QuickBooks sandbox

Did you follow these instructions? It looks like the app has not been properly authorized with QB yet

@JonnyTrexler
Copy link

image
image
I followed the instructions. This is the error that happens when it tries to export

@timbze
Copy link
Author

timbze commented Jul 7, 2022

I followed the instructions. This is the error that happens when it tries to export

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.

@JonnyTrexler
Copy link

image
I managed to get some of the invoices to export, but it only did 6 of around 200. Whenever I execute the plugin the cli process starts but then immediately ends. I've tried changing the start dates around but no luck. Any advice?

@timbze
Copy link
Author

timbze commented Dec 4, 2023

I have now updated dependencies and made sure this works with UISP 2.3.57 (lastest as of now)

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.

4 participants