search

itglue / powershellwrapper

This PowerShell module acts as a wrapper for the IT Glue API.
Apache License 2.0
120 stars 51 forks source link

Refactor all API calls into single function #160

Closed davidhaymond closed 1 year ago

davidhaymond commented 1 year ago

All requests have their query parameters (e.g. filter, sort) moved to the $query_params variable. This is passed to the -QueryParams parameter of Invoke-ITGlueRequest which adds it to the final request URI.

The $body variable has been removed from all exposed functions and moved to Invoke-ITGlueRequest.

Closes #151, closes #66. Thanks to @ecspresso for contributing the bulk of this patch in #157.

davidhaymond commented 1 year ago

@CalebAlbers @adrianwells @ecspresso I plan to do some testing before marking this ready for review, but feel free to look it over anyways.

Celerium commented 1 year ago

@davidhaymond nice work!, I've gone through about half of the GETs and at this time everything appears to be working as expected.

davidhaymond commented 1 year ago

Thanks for your help, @Celerium! Did you get a chance to make sure filtering and sorting works as expected? We'll also need to try a few create/update/delete operations. Without proper unit tests it's probably not practical to test everything. Once I get Linux and macOS support done and fix some of the higher priority bugs, I would like to work on expanding unit test coverage. That will make it a lot easier to catch issues when doing big refactors like this.

davidhaymond commented 1 year ago

Query params like sort and filter seem to be working, and I also successfully created a test configuration in IT Glue. I think this PR should be merged so we can move foward with other PRs and issues. We can do more testing before cutting a release.

@CalebAlbers Are you able to get this merged soon?

Celerium commented 1 year ago

Query params like sort and filter seem to be working, and I also successfully created a test configuration in IT Glue. I think this PR should be merged so we can move foward with other PRs and issues. We can do more testing before cutting a release.

@CalebAlbers Are you able to get this merged soon?

So far, the different filters for most of the GETs work as expected (tested about 3/4th of them). I was able to successfully create and edit flexible assets & passwords as well. No issues that I can find so far and I agree with @davidhaymond about merging this so we can move forward with other PRs.

adrianwells commented 1 year ago

@davidhaymond @Celerium Hi. Based on the comments here this looks basically ready to go, believe this should include a version number increase, 2.3.0?

davidhaymond commented 1 year ago

@davidhaymond @Celerium Hi. Based on the comments here this looks basically ready to go, believe this should include a version number increase, 2.3.0?

I would do the version bump in a separate commit just before releasing, since it would be great to patch the UTF-8 bug (#12) in this release as well. Once this is merged, I'll open a new PR for that.

adrianwells commented 1 year ago

@davidhaymond Thanks. OK. I will merge this PR so you can create another PR for #12.