[WIP] Rewrite/Extend all resources

[iranl]

Supersedes all earlier PR's by me. Implements all requested changes in those PR's. Also fixes some other open issues.

Unfortunately, mainly because everything is moved to traits, it is now impossible to separate the changes into different PR's for each resource.

Changes (non exhaustive list, applies to alle resources)

• All fields (including fields that are only used once) moved to traits in the Fields namespace. Re-use traits whenever possible (same exact element name and type in Twinfield API).
• Created Enums for every field with an exhaustive list
• Make a PhpTwinfield object of elements whenever possible (in the same way the office code field is turned into an Office object)
• Implement all DOM attributes from element fields (name, shortname, dimensiontype, locked, inherit etc,)
• Rewrote all ApiConnectors, Mappers and Documents to mimic Transactions
• Added resources (see below)

Component	get()	listAll()	send()	delete()	Mapper
Activities	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Articles	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Asset Methods	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Browse Data	:white_check_mark:				:white_check_mark:
Cash and Bank Books		:white_check_mark:			:white_check_mark:
Cost Centers	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Countries		:white_check_mark:			:white_check_mark:
Currencies	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Customers	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Dimension Groups	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Dimension Types	:white_check_mark:	:white_check_mark:	:white_check_mark:		:white_check_mark:
Electronic Bank Statements			:white_check_mark:
Fixed Assets	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
General Ledger Accounts	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Matching			:white_check_mark:		:white_check_mark:
Offices	:white_check_mark:	:white_check_mark:			:white_check_mark:
Paycodes		:white_check_mark:			:white_check_mark:
Projects	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Rates	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Sales Invoices	:white_check_mark:	:white_check_mark:	:white_check_mark:		:white_check_mark:
Sales Invoice Types		:white_check_mark:			:white_check_mark:
Suppliers	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
Transactions
Bank Transactions	:white_check_mark:		:white_check_mark:	:white_check_mark:	:white_check_mark:
Cash Transactions	:white_check_mark:		:white_check_mark:	:white_check_mark:	:white_check_mark:
Journal Transactions	:white_check_mark:		:white_check_mark:	:white_check_mark:	:white_check_mark:
Purchase Transactions	:white_check_mark:		:white_check_mark:	:white_check_mark:	:white_check_mark:
Sale Transactions	:white_check_mark:		:white_check_mark:	:white_check_mark:	:white_check_mark:
Users	:white_check_mark:	:white_check_mark:			:white_check_mark:
User Roles		:white_check_mark:			:white_check_mark:
VAT	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:	:white_check_mark:
VAT Groups		:white_check_mark:			:white_check_mark:
VAT Group Countries		:white_check_mark:			:white_check_mark:

Breaking Changes

• Basically breaks everything because instead of strings in a lot of cases PhpTwinfield Objects or Enums are now expected instead.

To Do

• Document breaking changes

[remkobrenters]

Promising PR which we could use in our current projects. Any expectations when is to be reviewed?

[iranl]

@remkobrenters As this PR is currently WIP i'm not expecting (although of course welcoming) a review at this time.

I will try to complete the last outstanding functionality to-do's in the coming days (Complete mapper for Users and Office and send() function for Users).

At that point I consider the PR feature complete and a preliminary review could be started at that time.

But a review will probably be a lot easier when I've completed the Unit/integration tests and documentation.

This will still take some time as i'm writing examples/docs and tests for 24(!) resources including examples of all API methods, all getters and all setters.

Op top of that there are breaking changes for 6 existing resources (Articles, Customers, Invoices, Bank Transactions, Suppliers and VAT) where replacement methods should be documented.

[remkobrenters]

Thanks for the fast reply. We will keep an eye on this and maybe we can help if the fork gets stuck at some point to wrap it up in a production-ready state.

[willemstuursma]

@remkobrenters I don't mind some help reviewing. Its turning into a pretty big change.

[RVxLab]

@iranl Does the finder wsdl always return its value as strings to your knowledge?

If so, this is currently an issue when requesting invoices. The invoicenumber property is expected to be a nullable int as per the your InvoiceNumberField Trait setInvoiceNumber signature.

See https://github.com/iranl/twinfield/blob/extend-all/src/Fields/Invoice/InvoiceNumberField.php

Also, I found that in the FinderService you're called $this->Search, which resolves to SoapClient::__call, which is deprecated. SoapClient::__soapCall should be used instead.

[iranl]

@RVxLab The finder returns its items as an ArrayOfString, so yes I do believe its values are returned as strings.

Maybe I'm not yet familiar enough with nullable type casting but I don't believe casting/juggling a numeric string value (which is the only possible value that the finder can return for this field, per the API documentation) to (nullable) int leads to anything other than an int with the correct value. This behavior was confirmed in my testing so far.

Can you elaborate in which real world situation you believe this will cause an issue?

As to your second suggestion -> I added the following method to BaseService (will add this soon to the PR), came across this as the easiest way to switch from __call to __soapcall when working with WSDL:

public function __call($func, $args)
{
    return $this->__soapCall($func, $args);
}

[RVxLab]

@iranl With the current implementation of the API, calling InvoiceApiConnector::listAll() will fetch results from both the new Invoice module and the old one. The old Invoice's invoiceNumber property is by my observations always a numeric string, which can be cast without any problems assuming strict mode is not enabled.

However, with Invoices from the new module, the invoices can be non-numeric, in which case a notice with the message A non well formed numeric value encountered will be thrown. Which in itself would not be an issue, except when using with a framework like Laravel, which converts all notices and warnings to errors.

While this can be worked around by using error_reporting(E_ALL ^ E_NOTICE), it's not ideal.

[iranl]

@RVxLab Was not aware that Twinfield start returning results from the new modules and was also not aware that the new one allows non-numeric invoice numbers (contradictio in terminis). I just followed the API documentation in this one which also states it should be an integer.

Considering the above I changed the InvoiceNumber to a string instead of an int.

With the latest commits I also rewrote Transactions (almost completely) and BrowseData/Matching/ElectronicBankStatement (a bit) to match the rest of the components.

[willemstuursma]

To be honest @iranl, the amount of changes is a bit overwhelming for me (653 files changed).

Looking at all the comments, it looks like its all moving into the right direction. But it will be hard for me to check it all.

What would be some good ways to proceed?

[iranl]

@willemstuursma Agree that it got a little out of hand. You kind of called it upon yourself when you requested me to do it right (e.g. traits/enums/typehints) though ;).

How about we break it down into seperate PR chuncks as follows:

NOTE: FIRST PART OF THIS LIST HAS NO IMPACT ON ANY EXISTING FUNCTIONALITY

• Add additions to current base files (e.g. BaseApiConnector, BaseDocuments, BaseMapper)

• Other small modifications that have no effect on existing functionality

• Fields (account for half of all changes files, but are in large parts just copies of existing functions from the individual objects and can be devided into groups that are almost identical)

• • Add String field traits
• • Add Int field traits
• • Add Bool field traits
• • Add Float field traits
• • Add Date/DateTime field traits
• • Add Money field traits
• • Add Enum field traits
• • Add PHPTwinfield object field traits (such as office/customer fields)

• Add new Enums and modify existing Enums where necessary

• Add new resources one by one (whenever possible)

NOTE: BREAKING CHANGES STARTING HERE

• Modify existing resources one by one (whenever possible) + modifiying tests where needed

• Add examples

• Add documentation (e.g. readme, breaking)

• Remove all unnecessary functions and files that are left over

Should get it down to 10-20 file changes per PR, the exception being the traits, but those are known functions and mostly copies of each other

[willemstuursma]

@iranl That seems like a sensible plan.

I would greatly appreciate if you can create the PRs for it.

Lets keep this PR open, so we can slowly see the diff getting smaller.