pnp / cli-microsoft365

Manage Microsoft 365 and SharePoint Framework projects on any platform
https://aka.ms/cli-m365
MIT License
923 stars 325 forks source link

New command: planner roster plan list #4405

Closed milanholemans closed 1 year ago

milanholemans commented 1 year ago

Usage

m365 planner roster plan list [options]

Description

Get a all Roster plans for a specific user

Options

Option Description
--userId [userId] User's Azure AD ID. Specify either userId, userName but not both. Specify this option only when using application permissions.
--userName [userName] User's UPN (user principal name, e.g. johndoe@example.com). Specify either userId, userName but not both. Specify this option only when using application permissions.

Examples

List all Planner plans contained in a Roster where the current logged in user is member of

m365 planner roster plan list

List all Planner plans contained in a Roster where another user is member of

m365 planner roster plan list --userName john.doe@contoso.com

List all Planner plans contained in a Roster where another user is member of

m365 planner roster plan list --userId 6336c567-b318-4cfb-b911-b2d6be3db9c5

Default properties

No response

Additional Info

API Request: https://learn.microsoft.com/en-us/graph/api/planneruser-list-rosterplans?view=graph-rest-beta&tabs=http

In delegated mode, we should use request:

GET https://graph.microsoft.com/beta/me/planner/rosterPlans

When using application permissions, the API request should be:

GET https://graph.microsoft.com/beta/users/{usersId}/planner/rosterPlans

Let's also add to the docs:

Adam-it commented 1 year ago

@milanholemans maybe a stupid question but what do you think if we would just make it m365 planner roster list and just add details in the docs how this command works (when delegated returns only current user plans when app permissions need to specify the user)?

milanholemans commented 1 year ago

@milanholemans maybe a stupid question but what do you think if we would just make it m365 planner roster list and just add details in the docs how this command works (when delegated returns only current user plans when app permissions need to specify the user)?

Good question, I don't really know if we can use planner roster list for this, because currently there is no way to list all Rosters (which is a shame). It's quite related to #4461 where we also chose for the name aad user group list.

Adam-it commented 1 year ago

@milanholemans maybe a stupid question but what do you think if we would just make it m365 planner roster list and just add details in the docs how this command works (when delegated returns only current user plans when app permissions need to specify the user)?

Good question, I don't really know if we can use planner roster list for this, because currently there is no way to list all Rosters (which is a shame). It's quite related to #4461 where we also chose for the name aad user group list.

ok understood. lets proceed with what you specified then 👍

Adam-it commented 1 year ago

@milanholemans I see the needs peer review label? do you want me to check something more? or could we open this up?

@pnp/cli-for-microsoft-365-maintainers for sure any other comments are more than welcome 😊

milanholemans commented 1 year ago

If it's ok for you, then I guess we can open it up indeed, thx!

nanddeepn commented 1 year ago

Can I work on this?

milanholemans commented 1 year ago

Sure thing @nanddeepn!

nanddeepn commented 1 year ago

Hi @milanholemans, @Adam-it Do you know any easy steps to add plan to the roster?

Adam-it commented 1 year ago

@nanddeepn what I did in the past to check PR's for those commands, is I used the graph-explorer, logged in to my test tenant and used MS Graph endpoints to create a roster plan and other stuff needed to test https://learn.microsoft.com/en-us/graph/api/planner-post-rosters?view=graph-rest-beta&tabs=http

milanholemans commented 1 year ago

Don't use Graph explorer! Use CLI! 😬

You can use m365 planner roster add to create a roster and then use m365 planner plan add --rosterId <id> to attach a plan to it.

You can also create one via the UI if you leave the add to existing group field empty, it should create a roster: image

Adam-it commented 1 year ago

Right 👍 I forgot those are already merged 🤦‍♂️😉. I agree. If we may use CLI 👍

nanddeepn commented 1 year ago

Thank you very much @milanholemans and @Adam-it You both Rock! 🤩

milanholemans commented 1 year ago

@pnp/cli-for-microsoft-365-maintainers I'd like your opinion on this one. It's been awhile since I specced this issue, but right now my gut feeling says we should name it planner user roster plan list. Why? Because a user can have multiple rosters. If we name it roster user it feels like that we are targeting a user associated to a Planner Roster which is not the case. We are listing all roster plans for a specific user. Any opinions?

Adam-it commented 1 year ago

for the targeting user I guess we use currently the word member 🤔. TBH in both ways I kinda feel it could be ok but what you suggested indeed provides more clarity 👍. BTW the command seems really long 😅

martinlingstuyl commented 1 year ago

Hi guys, am I right in thinking that a user is assigned to a roster (instead of to a plan)? And that there currently exists one plan per roster?

If both are a yes, or even if only the first is a yes, I think I’d list on roster level:

planner roster list —userId <userId>

As to switching it around with the user sub command first: I’m not in favor of that. I think the Azure Ad example really is different. We’re in the context of the Aad command there, and a user is an Aad entity. However, a planner user is not really a thing. Let’s just stick with using options here.

milanholemans commented 1 year ago

Hi guys, am I right in thinking that a user is assigned to a roster (instead of to a plan)? And that there currently exists one plan per roster?

2 times correct.

I really like your idea 💡only downside for this is that the response are not roster objects, but Planner plan objects. So using planner roster list I'd expect rosters as output which is not the case.

martinlingstuyl commented 1 year ago

To clarify:

We should remove the user subcommand is what I’m thinking.

Whether we implement it as planner roster plan list or planner roster list can be debated though… the user assignment is on roster level, which would speak for option 2, however, a roster is not really anything aside from a container to connect a plan to users. (Right?) so it would not be a very helpful command. That speaks for option 1, but technically speaking I dislike that…

martinlingstuyl commented 1 year ago

I'd expect rosters as output which is not the case.

We could change the specs to change the expected output :)

martinlingstuyl commented 1 year ago

Because there’s such a close symmetry between a roster and it’s plan, we could also opt for planner roster list —userId <userId> —includePlanDetails

By default it would just respond with the roster details, but using the flag would add the plan details as well.

We could even add this flag to other roster commands as well. I think it might be helpful to remediate the odd 1 on 1 relation between a plan and a roster.

(I remember we had 1 on 1 tables in SQL server databases which always caused this same kind of confusion)

milanholemans commented 1 year ago

We already have a planner roster get command, so if we name it planner roster list these 2 commands should have the same type of output, which is not the case. planner roster plan list might work tho.

milanholemans commented 1 year ago

Because there’s such a close symmetry between a roster and it’s plan, we could also opt for planner roster list —userId <userId> —includePlanDetails

By default it would just respond with the roster details, but using the flag would add the plan details as well.

There is currently no way to list all rosters, so planner roster list doesn't exist.

martinlingstuyl commented 1 year ago

We already have a planner roster get command, so if we name it planner roster list these 2 commands should have the same type of output, which is not the case. planner roster plan list might work tho.

Which is why I'm suggesting the flag... but you probably wrote this while I was typing :)

martinlingstuyl commented 1 year ago

There is currently no way to list all rosters, so planner roster list doesn't exist.

Which is the reason I'm suggesting a mandatory user option here. (Required option set) It's perfectly fine to implement the roster list like this and force people to add a user id or name. If a more general not user specific roster list api will become available we can make the user optionSet not required. But as long as there is none, you'll just have to pass a user option.

milanholemans commented 1 year ago

We already have a planner roster get command, so if we name it planner roster list these 2 commands should have the same type of output, which is not the case. planner roster plan list might work tho.

Which is why I'm suggesting the flag... but you probably wrote this while I was typing :)

I don't really understand the example with the flag. A flag is optional, what happens if you don't provide it?

martinlingstuyl commented 1 year ago

I don't really understand the example with the flag. A flag is optional, what happens if you don't provide it?

My idea was to return roster details only in that scenario. But my bad, I'm only now looking better at the mentioned endpoints in the spec and it seems it will return the plan info with the roster container. So my idea will not fly unfortunately. We're indeed hampered by the available endpoints currently. (And by the 1 on 1 relation between a roster and plan)

planner roster plan list makes no sense as well though, as you'd think a roster can have multiple plans.

martinlingstuyl commented 1 year ago

I'm not sure now. All our options seem to have downsides. The only think that's clear in my head is that we should avoid a user subcommand

milanholemans commented 1 year ago

In PnP.PowerShell this is called Get-PnPPlannerRosterPlan -User "johndoe@contoso.onmicrosoft.com" https://pnp.github.io/powershell/cmdlets/Get-PnPPlannerRosterPlan.html

So they tend to planner roster plan list as well.

martinlingstuyl commented 1 year ago

Thinking about it some more: We could also drop the word roster here. Effectively we're talking about plans of a user, so we could add the functionality to the planner plan list command.

The only challenge there is that you'd want the users group plans as well, but there is no command to list all plans from all groups a user is a member of. (We do have the /me/planner/recentPlans endpoint however)

Would it make sense to think in this direction? Aka: add a userId optionSet to that command and have it call multiple endpoints if available.

martinlingstuyl commented 1 year ago

In PnP.PowerShell this is called Get-PnPPlannerRosterPlan -User "johndoe@contoso.onmicrosoft.com" https://pnp.github.io/powershell/cmdlets/Get-PnPPlannerRosterPlan.html

So they tend to planner roster plan list as well.

Yeah, that's still a possibility as well, though with them the commandlet might function as a Get and List command, which is convenient for them. With us, the subcommand structure gives the sense that a roster can have multiple plans.

milanholemans commented 1 year ago

Thinking about it some more: We could also drop the word roster here. Effectively we're talking about plans of a user, so we could add the functionality to the planner plan list command.

The only challenge there is that you'd want the users group plans as well, but there is no command to list all plans from all groups a user is a member of. (We do have the /me/planner/recentPlans endpoint however)

Would it make sense to think in this direction? Aka: add a userId optionSet to that command and have it call multiple endpoints if available.

Like you mentioned, it looks impossible to make this work.

martinlingstuyl commented 1 year ago

Some things have drawbacks all around :)

Would planner roster plan list --userId <userId> have the least drawbacks? I'm now leaning to that one...

nanddeepn commented 1 year ago

planner roster plan list will be a good name, as this command helps to get roster plans for user.

milanholemans commented 1 year ago

Seems like planner roster plan list has the most votes, let's pick that one. Thank you for the discussion folks!