search

Anteris-Dev / autotask-client

An HTTP client for the Autotask API built with PHP.
MIT License
22 stars 15 forks source link

Interface for Anteris\Autotask\Client #27

Closed Rodeveer closed 3 years ago

Rodeveer commented 3 years ago

Is it possible that we can interface the Anteris\Autotask\Client and its sub Services? I've counted the services quickly; its 181 services you've created so if we could interface them too. I mean, would be lovely to see some SOLID in there so its actually testable without mocking! 👍

When people (like me) want to test their code with maybe a fake/stub/dummy people can't since they áctually need to return a Anteris\Autotask\Client; Which calls alot of http requests and is thus untestable within PHPunit.

The repo which is a wrapper for the new AutoTask Restfull API is great nonetheless!

Thanks for answering!

aidan-casey commented 3 years ago

Hey @Rodeveer! Welcome and thanks for the suggestions!

If I am understanding the request correctly, we are actually looking into this for the next major version of this package. Our general thinking is that instead of a building an isolated Guzzle instance within the wrapper, it will accept a PSR-7 client interface. This way you would be able to use whatever PSR-7 compliant client you like (including a mock client for tests).

The other benefit to this would be the flexibility to add to the configuration options on the HTTP requests.

We do run a series of basic tests against a live version of the API on push to this repository. This is better than nothing, but I definitely agree there is room to improve!

Rodeveer commented 3 years ago

Hey @aidan-casey. Thanks for your reply! Thats actually even better! Give people the possibility to choose what client they want to choose. I guess you would implement a default tho.

When do you think you'll be releasing this? (Guess this is just a side fun project you do in between?)

Running against a live api is something I would not recommend. In case the connection is dead; Got accidently the wrong credentials or anything that would influence the live data is imo a bad case. But I assume, better then nothing. Now atleast you know your code works hehe!

If there's anything I could maybe help you with...

aidan-casey commented 3 years ago

@Rodeveer - Exactly. We would probably not implement a default client in interest of keeping unnecessary requirements out of the package for its users. Instead we would utilize Composer's suggest feature and recommend Guzzle Http. While this does not seem as developer friendly at first, it keeps extra bloat out of their software.

We are shooting for mid-month in November. Our company maintains this package for the Autotask community, but when it comes to a major release version, our goal is to pack all the necessary changes in so that users do not need to keep implementing breaking changes into their projects. For this reason, we are being careful to leave no stone unturned with this update.

Regarding tests, these are important tests known as integration tests. They test the software as a group to ensure it works appropriately and they are best practice. Already these tests have notified us twice of breaking changes made to the Autotask API. Along with the next version, we are looking to further the integration tests we have in place and implement some solid unit tests alongside them.

Contributions are always welcome. I recommend you check out the generator if you are interested in helping us out. You can always ping me directly and we can talk through some helpful ways you could contribute.

Rodeveer commented 3 years ago

Thanks! I'll waitfor the new release. Thanks for the information!