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

Datto REST API - SaaS Bulk Seat Change #3

Merged
merged 11 commits into from
Jan 20, 2024
Merged

Datto REST API - SaaS Bulk Seat Change #3

merged 11 commits into from
Jan 20, 2024

Conversation

cksapp
Copy link
Contributor

@cksapp cksapp commented Jan 16, 2024

Pull Request

Datto SaaS Protection - Bulk Seat Change REST API Function

Description

Fixes SaaS Bulk Seat Assignment Function added in 5cabe17 to support API endpoint.

Resolves: #2 [Bug]: SaaS Bulk Seat Update Endpoint should be SET with PUT call.

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • General code cleanup (non-breaking change which improves readability)

Checklist

  • I have reviewed the Contributing guide
  • I am pulling to the dev branch
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

How Has This Been Tested?

Validated against expected behavior from API calls via Postman, changes were reflected in the Datto SaaS Protection service.

  • Postman
  • Before
    2024-01-04_360
  • Postman API call
    2024-01-04_357
  • Result
    2024-01-04_359

  • PowerShell Wrapper Module
  • Before
    2024-01-15_416-1
  • PowerShell Function
    2024-01-15_412
  • Result
    2024-01-15_414

cksapp added 10 commits January 7, 2024 16:38
Changed Get to Set for Datto SaaS Bulk Seat Assignment and also added http PUT
Renamed Get-DattoBulkSeatAssignment to Set-DattoBulkSeatAssignment to match function name and better match PowerShell approved verb-noun for actions performed
Change SaaS bulk seat assignment to include required parameters, added examples, and structured body request to pass in API call to endpoint.
Added parameters for bulk SaaS seats to excluded parameters in DattoQueryString.

Changed Invoke-DattoRequest to include valid PUT type, $body expected to be hashtable and passed as hashtable, removed wrapper in body parameter.
Appears that the quotes in the hashtable for variables were causing the issue for the SaaS Bulk Seat Assignment PUT, removed quotes and response from endpoint received and validated seat change went live. Also added ability to input multiple remoteId in an array
Does not appear to be consistent, for wrapper we can just return errors and allow module user to decide further logic on handling to possibly limit number of input parameters.
Change function name a bit further from `Set-DattoBulkSeatAssignment` to `Set-DattoBulkSeatChange` to better align with the API endpoint being called `https://api.datto.com/v1/saas/{saasCustomerId}/{externalSubscriptionId}/bulkSeatChange`

Also updates pester test from `Get` to `Set`, and `DattoBulkSeatAssignment` to `DattoBulkSeatChange`.
Change function name a bit further from `Set-DattoBulkSeatAssignment` to `Set-DattoBulkSeatChange` to better align with the API endpoint being called `https://api.datto.com/v1/saas/{saasCustomerId}/{externalSubscriptionId}/bulkSeatChange`

Also updates pester test from `Get` to `Set`, and `DattoBulkSeatAssignment` to `DattoBulkSeatChange`.

Also updates doc site from `Get` to `Set`, and `DattoBulkSeatAssignment` to `DattoBulkSeatChange`.
Change function name a bit further from `Set-DattoBulkSeatAssignment` to `Set-DattoBulkSeatChange` to better align with the API endpoint being called `https://api.datto.com/v1/saas/{saasCustomerId}/{externalSubscriptionId}/bulkSeatChange`

This should get just about everywhere the function is mentioned and updates the docs to match the correct method for the function.
@cksapp cksapp changed the base branch from development to main January 16, 2024 01:13
Best practice for Set-, add cmdlet binding for SupportsShouldProcess, -whatif and -confirm parameters

Update README.md
@Celerium
Copy link
Owner

Celerium commented Jan 20, 2024

@cksapp, Thank you for the awesome contrubutions! Mostly everything looks good here so I am going to merge and make some tweaks to a few items.

#2

@Celerium Celerium merged commit 4b28a24 into Celerium:main Jan 20, 2024
5 checks passed
@cksapp
Copy link
Contributor Author

cksapp commented Jan 20, 2024

Thanks @Celerium! I actually meant to also push a minor commit in 12f5790 as part of this, not anything major so otherwise the function works just as intended. Mainly updating the messaging and examples, not sure if the PR can be amended after being closed or it would require another pull request.

Otherwise feel free to simply copy the changes from there, and feel free to make any changes overall as you see fit for the module.
Glad to contribute 😄

@Celerium
Copy link
Owner

No worries, ill add those updates as well. I also found some other changes that Datto made to their API that ill be adding as well.

For the actionType where did you find the values of 'License', 'Pause', 'Unlicense'? I cant seem to find them in their API docs

@cksapp
Copy link
Contributor Author

cksapp commented Jan 22, 2024

No worries, ill add those updates as well. I also found some other changes that Datto made to their API that ill be adding as well.

Okay great, there were a few additional things I noticed in the docs that may have been missed or need to be updated so might try to work on another small PR to update those when I can.
I did notice the new additions to seats API as well but hadn't had a chance to review further.


For the actionType where did you find the values of 'License', 'Pause', 'Unlicense'? I cant seem to find them in their API docs

Regarding the actionType, I have to agree the documentation for the API is fairly sparse to say the least. Mostly determining the actionType was based on trial and error, as well as just a bit of insider knowledge.
(I currently work within Datto's SaaS Protection product and scoured for information internally based on the changes from their dev team as no-one else was familiar). 😉

There does not appear to be any other mentions of additional actionTypes and testing with an actionType of something like Archive does not appear to accept any PUT and simply returns,

{
    "code": "error.internal",
    "message": "An internal server error occurred"
}

On a side note to this, I do strongly believe we should make the note that both actionType, and seatType are case-sensitive and otherwise appear to require PascalCase as the call will return an error if not respected.

I presume this is due to the *NIX backend systems being used. I am not sure if we could/should enforce the case-sensitivity for the input, or simply return the error and make the warning in the docs here.
We probably could, but as an API wrapper I feel its best to let most of the logic be handled by the module user

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.

[Bug]: SaaS Bulk Seat Update Endpoint should be SET with PUT call.
2 participants