New command: teams channel member remove. Closes #3175#3240
New command: teams channel member remove. Closes #3175#3240Jwaegebaert wants to merge 2 commits intopnp:mainfrom
teams channel member remove. Closes #3175#3240Conversation
|
Awesome! We'll review it shortly |
|
How can I test this command @Jwaegebaert? When I try to run it, I'm getting an error: |
|
Hey @waldekmastykarz, if you get the error |
|
Thanks! I missed that detail in the command's description. An extra check would be awesome. To avoid holding up the PR, let's do it in a separate issue as an enhancement. Could you please create an issue so that we can track it? Thank you for your help! |
waldekmastykarz
left a comment
waldekmastykarz
left a comment
There was a problem hiding this comment.
Works like a charm! Merged with a few small adjustments
|
|
||
| ```sh | ||
| m365 teams channel member remove --teamName "Team Name" --channelName "Channel Name" --userId 00000000-0000-0000-0000-000000000000 | ||
| ``` No newline at end of file |
There was a problem hiding this comment.
Let's add an empty line at the end of the file
| cb(); | ||
| } | ||
| else { | ||
| this |
There was a problem hiding this comment.
Let's wrap this logic in a function so that we can keep it in one place without repeating.
| } | ||
|
|
||
| public validate(args: CommandArgs): boolean | string { | ||
| if (args.options.teamId && args.options.teamName) { |
There was a problem hiding this comment.
We can simplify these checks by using option sets. Check out: https://github.com/waldekmastykarz/cli-microsoft365/blob/5b821fead07e125b9c8aef0bc8a848eca7874d36/src/m365/aad/commands/app/app-add.ts#L82-L86
| assert.strictEqual((alias && alias.indexOf(commands.CONVERSATIONMEMBER_REMOVE) !== -1), true); | ||
| }); | ||
|
|
||
| it('fails validation if required options are not passed', (done) => { |
There was a problem hiding this comment.
We check for the presence of required options outside of the command so we don't need to have a test for it in the command
|
Merged manually. Thank you! 👏 |
2 participants
Closes #3175