cli-microsoft365
cli-microsoft365 copied to clipboard
Enhancement: Use the Microsoft Graph Types package in our commands
Based on the discussion #2427 we would like to introduce a new package @microsoft/microsoft-graph-types to our project to have strongly typed results when calling Microsoft Graph.
This should reduce the likelihood to introduce bugs based on typing errors.
The aim of this issue is to go through all commands and replace the "custom" types with the "official" types provided by the package.
For the beta endpoints we should consider the "@microsoft/microsoft-graph-types-beta" package.
Here are the commands that need to be updated:
aad -> O365Group
- [ ] o365group-get
- [ ] o365group-list
- [ ] o365group-set
- [ ] o365group-add
- [ ] o365group-recyclebinitem-clear
- [ ] o365group-recyclebinitem-list
- [ ] o365group-recyclebinitem-set
aad -> user
- [ ] o365group-user-list
- [ ] o365group-user-set
- [ ] user-list
@pnp/cli-for-microsoft-365-maintainers do we still see here some benefits or should we close this issue due to lack of interest?
I'd be interested in keeping this open, maybe we should consider doing some investigation to create a list of commands that currently use custom types that can be replaced with the official types?
Once we have a list of commands, this will help us better understand the scope of this change.
Instead of going through lists I could just start searching a command and refactor it with types.
I could give it a shot
All yours @plamber 👍
We use the package already so it would be a matter of identifying custom types and replacing them with a reference to the type from the package. Even if we use beta APIs, we should see if we need access to beta properties or if we can use the information from the main package to avoid having to reference two of them.
Had a look into our commands.
- Almost all team commands implement a dedicated interface in our cli. These can be replaced with the graph types
- Something similar happens for the users and groups endpoints
- Same under rstieclassification
There are also other places but these are the first ones that I found by performing a simple scan.
Should we create dedicated issues for each namespace or can we create multiple pull requests and associate them with this issue?
Cheers
Ideally, let's create a checklist of all affected commands and tests that need updating so that we can track progress and know when we're done. Once we have it, we could create multiple PRs linked to the same issue.
aad -> O365Group
- [ ] o365group-get
- [ ] o365group-list
- [ ] o365group-set
- [ ] o365group-add
- [ ] o365group-recyclebinitem-clear
- [ ] o365group-recyclebinitem-list
- [ ] o365group-recyclebinitem-set
aad -> user
- [ ] o365group-user-list
- [ ] o365group-user-set
- [ ] user-list
I moved the list of commands to the issue for readability
Looking at the project, there are other custom types beyond just Group and User that we've added so let's complete the list of affected commands. #2902 only completes this issue partially.
What shall we do with this one @plamber, @waldekmastykarz, move it back to help wanted?
Yes, good catch @martinlingstuyl! Let's remove custom interfaces where possible.
Ideally, let's create a checklist of all affected commands and tests that need updating so that we can track progress and know when we're done. Once we have it, we could create multiple PRs linked to the same issue.
Should we turn this into an epic
and convert each checklist item into an issue for each command, this way we keep one PR to one issue?
I don't think we should do it per-command, because if you remove the interface, it will break multiple commands. Instead, we should make the list based on the interfaces that we use, so that each PR will contain the removed interface and one or more adjusted commands.
I could give this a go :-)
Great to hear @MathijsVerbeeck! All yours!