cli-microsoft365: Script samples contain an invalid condition check
Most samples start off by doing a m365 status check to see if they are still logged in to their Microsoft 365 environment. Which looks something like this.
$m365Status = m365 status
if ($m365Status -eq "Logged out") {
m365 login
}
The if condition here will always be false because we aren’t converting the output of m365 status to an object.
This isn’t a new issue because it also occured within the repo script samples and was recently fixed aswell. https://github.com/pnp/script-samples/issues/298
The way we can resolve this, is by updating the status command to output as text. Like the following:
$m365Status = m365 status -o text
if ($m365Status -eq "Logged out") {
m365 login
}
I would suggest we apply this to the following script samples:
- analyze-users-haveibeenpwnd
- delete-m365-groups-and-sharepoint-sites
- replace-membership-of-selected-groups
- replace-owner-with-a-different-one
- flow-runs-day-summary
- add-multiple-tasks
- add-multiple-lists-in-multiple-sites
- copy-files-to-another-library
- list-all-checked-out-files (all 3 samples)
- list-all-files-specific-name
- list-all-list-folders-itemcount
- list-large-files-within-a-site
- list-site-collection-lists
- list-site-collection-owners
- list-site-externalusers
- replace-site-collection-admin
- setup-example-site
- upload-local-files-and-folder
- add-bulk-users-teams
- create-team-and-add-members-and-owners
- export-all-channels-teams
- install-personal-app
- list-all-tabs-teams
- list-all-teammembers-teams
- list-teams-owners-and-members
- remove-personal-app
- share-socialchampions
About this issue
- Original URL
- State: closed
- Created 2 years ago
- Reactions: 2
- Comments: 24 (23 by maintainers)
Will do 😄
Please do @Jwaegebaert. Let’s use
--output textfor now.The new issue and interesting PoSh stuff aside, I think the
output textway is indeed the best way forward for now.Could you update the specs @Jwaegebaert?
Some bg: the reason
matchdoes not work when logged in, is because the returned json is not really a string in PowerShell. It’s an array of strings. Each line being a string.Let’s continue this discussion in the other issue.
at least we can fix the current issue using the setup with
--output text@martinlingstuyl, specs have been updated.
and like
m365 status --validateLogin --output Textwould return true butm365 status --validateLogin --output Jsonwould return{ validateLogin: true }😅… only joking 😉. Interesting idea 👍 I’m all in. Lets check in on a separate issue 🤔BTW I forgot to add: Thanks @Jwaegebaert for you awesome support in the scrip samples!!! You totally rock 🤩🤩🤩. I think script samples is always the best way to show true potential of a tool and it’s functionality and it should be our priority to be sure they work just like that 👍👍.
Sorry for being late to the party 😉. Why can’t we just add
--output text🤔? Isn’t it like the safest possible option 🤔? I mean what if in the future we will again change the approach to make not JSON default output mode but… hmm xml 😁… I know, not gonna happen 😉. But I think adding the output mode would keep us in the save side for future. That’s what happened now 😉It is an issue, the if block doesn’t execute as expected in my tests because its empty (annoying its not false) but it doesn’t break the pattern? @Jwaegebaert are you seeing something different?
I noticed that too. Don’t know why that’s happening.
That’s true. That’s also a possibility. I think that was something @Adam-it also suggested during the discussion in the script sample repo. https://github.com/pnp/script-samples/issues/298#issuecomment-1146884949
The idea of using the
-matchoperator was something @pkbullock came up with. So, I think there are several ways we can solve this condition.😄Wow, good catch. Totally didn’t notice the upper O. 😅