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

Added Class BulkTransactionLine #354

Merged
merged 1 commit into from
Apr 24, 2019

Conversation

florowebdevelopment
Copy link

We thought of two solutions:

  1. Overwrite the url property of the TransactionLine class, however its not only a 'bulk' prefix the url is also slightly different 'financialtransaction/TransactionLines' => 'bulk/Financial/TransactionLines'.
  2. Create a new class BulkTransactionLine

Please note we did not add an Entity Test because the parent class is Picqer\Financials\Exact\TransactionLine instead of Picqer\Financials\Exact\Model.

@markjongkind
Copy link
Contributor

markjongkind commented Mar 27, 2019

The deprecated endpoint financialtransaction/Transactions has 26 properties, while the new bulk/Financial/TransactionLines endpoint has 68 properties. Also the Primary Key is different (EntryID vs ID). So I think option 2 (creating a new class) is the best solution, without using TransactionLine as the parent class.

Edit: nevermind.

@davidvandertuijn
Copy link
Contributor

davidvandertuijn commented Mar 27, 2019

@markjongkind

without using TransactionLine as the parent class.

You compare Transactions with TransactionLines, while BulkTransactionLines is the same as TransactionLines, except for the url property.

https://start.exactonline.nl/docs/HlpRestAPIResourcesDetails.aspx?name=FinancialTransactionTransactionLines
https://start.exactonline.nl/docs/HlpRestAPIResourcesDetails.aspx?name=BulkFinancialTransactionLines

Because of the deprecated endpoint Transactions, i decided to use BulkTransactionLines instead of TransactionLines because of the increased Page Size of 1000 instead of 60.

I hope this Pull Request can go live before 01-04-2019.

@markjongkind
Copy link
Contributor

Oh boy, my bad. I'm going to shut up now and let you talk. Nice work with this PR!

@Lauren076
Copy link

Waiting for this one to get approved...

@stephangroen stephangroen merged commit 14c052b into picqer:master Apr 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants