mattmcnabb / O365ServiceCommunications

A Powershell module for monitoringOffice 365 Service Health!
25 stars 6 forks source link

Get-SCTenantEvent Error #5

Closed PowershellNinja closed 7 years ago

PowershellNinja commented 7 years ago

When calling the command below, having established a valid SCSession, which works with Get-SCEvent:

Get-SCTenantEvent -EventTypes Incident -PastDays 1 -SCSession $o365Session -Domains "mycustomer.onmicrosoft.com"

Then I get the following Error Message:

Select-Object : Property "Events" cannot be found.
At C:\Program Files\WindowsPowerShell\Modules\O365ServiceCommunications\1.4\functions\Get-SCTenantEvent.ps1:56 char:31
+ ...     Invoke-RestMethod @Splat | Select-Object -ExpandProperty Events |
+                                    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    + CategoryInfo          : InvalidArgument: (@{EventInfoType...ystem.Object[]}:PSObject) [Select-Object], PSArgumen
   tException
    + FullyQualifiedErrorId : ExpandPropertyNotFound,Microsoft.PowerShell.Commands.SelectObjectCommand
mattmcnabb commented 7 years ago

@PowershellNinja to be honest, I've never gotten a chance to actually test this module from a partner perspective. It would be great to try to tackle this and get it working properly. I haven't reviewed this project in a while, but if I can get some commands gathered that will help troubleshoot this, can you help by trying to execute the commands and then sending through the output? Sanitized, of course.

Thanks!

PowershellNinja commented 7 years ago

Hi Matt

Sorry I didn't answer your question before, as I am really interested in this functionality (and had some spare time), I went ahead and analyzed the problem.

It seems on one hand, the API Call for Get-SCTenantServiceInfo had a wrong JSON Body Property: the official Microsoft Docu states it should be "companyDomains" while in fact it has to be "tenantDomains". On the other hand, Get-SCTenantEvent did not correctly handle the return objects as they differ from the ones returned by Get-SCEvent. I tested the module and both functions now work for me, but could you please test my changes too? (Pull Request #6 )

Thanks!

mattmcnabb commented 7 years ago

No biggie - I'm glad to get the PR! unfortunately I probably won't be able to review this today. I'll get back to you tomorrow sometime after I've checked out why the tests are failing.

Unfortunately I can't actually test the commands as I don't have a partner admin account. That's kind of been a blocker for me getting these commands finished. If it's working for you and I can work out the Pester tests, this should be good to release.

Thanks!

mattmcnabb commented 7 years ago

ok, I lied - I figured out the problem with the tests and will merge PR #6 assuming you pull in the changes first.

Also, I don't mind too much, but there are some changes in here that aren't strictly related to the issue at hand, such as whitespace changes (spaces to tabs, no newline at EOF, expansion of aliases, etc.). Not a huge deal but in the future I'd prefer hygiene items like that in their own PR.

Thanks!

PowershellNinja commented 7 years ago

Hi Matt

You are absolutely right, I will keep that in mind for the future. Bad habits :/ And: Sorry for breaking your tests, I didn't check if there were any.

I repulled master and pushed it to my repo, it should now be in the PR #6.

Would you like me to make Tests for the two Tenant Functions? I could create them in a separate PR. And if you want, I could provide you with some sample data responses from the Tenant Functions so you don't have to work on air in the future.

Thanks, Raphael

mattmcnabb commented 7 years ago

No worries. if you'd like to add some tests, feel free. The tests included now are very basic and it probably needs some additional validation. And yeah, if you can provide some sample data that would be really useful - maybe event post it right here in the issues feed as long as it's sanitized, or a link to a gist.

It's nice to finally get this fixed right before the deprecation of the API ;) Eventually I'll have to get around to a v2.0 version that supports the newer API.

I merged PR #6 and will add some release notes and deploy.

PowershellNinja commented 7 years ago

Hi Matt

Here are the links to the two gists: https://gist.github.com/PowershellNinja/54b62346ea417e50518b0887439625fc#file-gettenanteventresponse_sanitized-json https://gist.github.com/PowershellNinja/8c35e00040bdc8ccb05135fd8462f836#file-gettenantserviceresponse_sanitized-json

I went through them 2 times, so I guess they should be clean. If you still should discover something, I would be glad for a note ;-)

Thanks, Raphael

mattmcnabb commented 7 years ago

@PowershellNinja

Thanks! I'll check this out and see if I can build some tests around the data.