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
New command: viva engage community get. Closes #5754 #5920
base: main
Are you sure you want to change the base?
Conversation
Thanks @Saurabh7019! We'll try to review it ASAP. |
Hi @milanholemans , The PR is failing due to the error below. Have you encountered this issue before? /home/runner/work/cli-microsoft365/cli-microsoft365/src/m365/viva/commands/engage/engage-community-get.ts |
Hi, @Saurabh7019 this is because we are using a dictionary to enforce a correct naming of commands in CLI. cli-microsoft365/.eslintrc.cjs Lines 3 to 11 in 443bfd8
In this case, I think we should add the word |
Hi @Saurabh7019, could you rebase with the latest main to resolve the merge conflict, please? |
Done! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice start @Saurabh7019, made a few comments while reviewing, could you have a look at them, please?
|
||
This command is based on an API that is currently in preview and is subject to change once the API reached general availability. | ||
|
||
In order to use this command, you need to grant the Azure AD application used by the CLI for Microsoft 365 the permission to the Viva Engage API. To do this, execute the `cli consent --service VivaEngage` command. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since this is a Graph command, we don't have to show this warning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I may not have fully understood this. I have removed the "cli consent" part since it was not included in the issue spec. Please let me know if any other changes are needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @Saurabh7019, yes the consent part was not needed. For other viva engage
commands we use the Yammer API which requires this consent. Since this command is built on Graph, we don't have to show this warning here.
Closes #5754