-
Notifications
You must be signed in to change notification settings - Fork 359
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
#1021 - New-PnPTeamsTeam , ability to add multiple owners and members #1241
Conversation
Found a way to improve this PR. Will add some code tomorrow |
@gautamdsheth I have rebuilt the logic behind this one. Not sure how you got it to work the way it was. In my test cases it looks to work fine, but as this is likely an often used cmdlet, please check this code again as well. I've removed quite some pieces of what I believed was redundant code. Also removed the /me Graph call. Not sure if that could wreck backwards compatibility with this cmdlet. |
@KoenZomers - tested it, fixed the chunk add logic because Graph only allows adding 20 users at a time. Much cleaner code now, thanks a lot. We can merge it ! |
@gautamdsheth We're not there yet. With a fresh view I noticed that Owner and Owners are not mandatory. And I believe in the current situation that would mean that the current user executing the cmdlet would become the owner. In this updated code it throws an exception in that case: Let me debug it (now I can finally debug again! 🥳) and see where it goes wrong. |
@KoenZomers - can you give it a try now ? Have fixed the issue that you faced. If the case if app-only context, we will create a group without an owner. |
2d48172
to
e50ded9
Compare
…hell into feature/1021
Pending PG validation if there's a maximum on the Graph API on how many users we can add in 1 call |
…hell into feature/1021
Type
Related Issues?
Fixes #1021 , #1022
What is in this Pull Request ?
Adds ability to add multiple owners and members to a Teams team.