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

#1021 - New-PnPTeamsTeam , ability to add multiple owners and members #1241

Merged
merged 35 commits into from
Jan 26, 2022

Conversation

gautamdsheth
Copy link
Collaborator

Type

  • Bug Fix
  • New Feature
  • Sample

Related Issues?

Fixes #1021 , #1022

What is in this Pull Request ?

Adds ability to add multiple owners and members to a Teams team.

@KoenZomers KoenZomers linked an issue Oct 13, 2021 that may be closed by this pull request
@KoenZomers KoenZomers added the in review PR or issue is currently being reviewed label Oct 13, 2021
@gautamdsheth
Copy link
Collaborator Author

Found a way to improve this PR. Will add some code tomorrow

src/Commands/Utilities/TeamsUtility.cs Outdated Show resolved Hide resolved
src/Commands/Utilities/TeamsUtility.cs Outdated Show resolved Hide resolved
@KoenZomers
Copy link
Collaborator

@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.

@gautamdsheth
Copy link
Collaborator Author

gautamdsheth commented Dec 21, 2021

@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 !

@KoenZomers
Copy link
Collaborator

KoenZomers commented Dec 21, 2021

@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:

image

Let me debug it (now I can finally debug again! 🥳) and see where it goes wrong.

@gautamdsheth
Copy link
Collaborator Author

@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.

@KoenZomers
Copy link
Collaborator

Pending PG validation if there's a maximum on the Graph API on how many users we can add in 1 call

@KoenZomers KoenZomers merged commit dda26eb into pnp:dev Jan 26, 2022
@KoenZomers KoenZomers removed the in review PR or issue is currently being reviewed label Jan 26, 2022
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.

[FEATURE] New-PnPTeamsTeam Allow Add Members [FEATURE] New-PnPTeam Allow Add Multiple Owners
2 participants