search

pnp / cli-microsoft365

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

Add commands for SharePoint Embedded #5731

Open Adam-it opened 6 months ago

Adam-it commented 6 months ago

Aim

The aim of this issue is to track and add follow-up issues for commands which would allow to manage a SharePoint Embedded (SPE) container.

What would be the benefit?

Each Container Type is owned by one Application; and each Application can own only one Container Type. The owner application may grant permission to other apps to modify containers of the owner app container type. So let's say someone developed their own app that uses SPE containers but would want an additional application (like a console helper app) to generally have an overview of all containers and it's contents of some specific container type. So CLI for M365 could be this additional helper application that when granted by the owner application of a container type, would allow (admins or developers) to list, and manage containers of that type.

https://learn.microsoft.com/en-us/sharepoint/dev/embedded/concepts/app-concepts/app-architecture

Additional role for CLI could be the managements side. Like managing container type permissions.

Commands I would add

As SharePoint Embedded is a kinda a separate thing in SharePoint I would add it as a new group. So similar like for SharePoint Online we have spo I would introduce spe fro SharePoint Embedded.

The commands should be based on MS Graph API support and SharePoint REST API v2.1 (aka: VROOM). More info on this in the following docs:

... besides the above I am planning to add commands for managing container, container permissions, container property, container column, container files etc.... this is still work in progress

Other things to consider

We could research if it is possible to check if the SharePoint Embedded is enabled on the user tenant and add a ensure command for that

Tip

During development this might be helpful to browse and play around

waldekmastykarz commented 6 months ago

Nice proposal. Even though the admin part of SPE isn't exposed on Graph, it's using CSOM which is a perfect API for us to use as well. Let's try to start with the end to end story so that folks can use CLI from the very first touch with SPE to completing some scenarios, like you outlined.

Adam-it commented 6 months ago

Nice proposal. Even though the admin part of SPE isn't exposed on Graph, it's using CSOM which is a perfect API for us to use as well. Let's try to start with the end to end story so that folks can use CLI from the very first touch with SPE to completing some scenarios, like you outlined.

Good comment. I will research this as well and check how we may obtain that. I guess then we should add a command like m365 spe containertype ensure right?

waldekmastykarz commented 6 months ago

Good comment. I will research this as well and check how we may obtain that. I guess then we should add a command like m365 spe containertype ensure right?

Looking at the documentation, there's a cmdlet named New-SPOContainerType, which seems to create containers. That would be an equivalent to our add rather than ensure. What scenario can we think of where ensure would be more desirable? Are we expecting a usage pattern where you don't know if the container is there or not and want to ensure its final state?

waldekmastykarz commented 6 months ago

Also, should containertype be its own namespace, or should we aim for two separate words container type, so that all container commands fall under the common container namespace?

Thinking of the proposed command names some more: what if we simplified the name to m365 spe file list dropping the container part? By analogy, we've got spo file get, rather than spo documentlibrary file get. Thoughts?

Adam-it commented 6 months ago

Also, should containertype be its own namespace, or should we aim for two separate words container type, so that all container commands fall under the common container namespace?

Thinking of the proposed command names some more: what if we simplified the name to m365 spe file list dropping the container part? By analogy, we've got spo file get, rather than spo documentlibrary file get. Thoughts?

I don't have any strong opinion about the first on. For me both containertype or container type would do the trick. I think we will anyway have one or two commands for this area.

As for the files I guess it makes total sense to make it shorter. You may not have any files outside of the container anyway so there is no need to mention it

Jwaegebaert commented 5 months ago

That would be some nice additional commands to have. I think it makes sense to shorten container file to file, although it could be confusing that you would have to include an option like --containerId it is indeed similar to the approach we have for SPO libraries. Regarding containertype, I'm also more keen to use separate words like container type.

milanholemans commented 5 months ago

Since it's called SharePoint embedded, doesn't it make sense to include it in the spo group? Maybe something like spo embedded?

Adam-it commented 5 months ago

Since it's called SharePoint embedded, doesn't it make sense to include it in the spo group? Maybe something like spo embedded?

I disagree mainly since:

Adam-it commented 5 months ago

That would be some nice additional commands to have. I think it makes sense to shorten container file to file, although it could be confusing that you would have to include an option like --containerId it is indeed similar to the approach we have for SPO libraries

it depends 😉. For spo file commands we usually have to pass somehow the folder (either by name, id or url), which make total sense since all files are in 'some' folder. SPE is container-oriented so 'everything' you do starts and stops in a container. If someone is used tho this concept it may seem very natural that all spe commands require that we somehow pass which container we are talking about.

milanholemans commented 5 months ago

I haven't dug into it yet. What I noticed is that the API calls are relatively SharePoint-ish. You have to use the /drives endpoint in graph to target your container. This endpoint returns regular SharePoint doc libs as well. Also if it's not related to SharePoint Online, why did they include it into the SharePoint Online Management Shell module? This module is built to configure your SharePoint Online subscription. As mentioned before, haven't dug into it yet. Just dropping some ideas.

Adam-it commented 5 months ago

@milanholemans those are some good observations 👍.

I haven't dug into it yet.

I totally get it. It's really hard to go pass by all the Copilot AI stuff to reach this (SPE) place 😅

What I noticed is that the API calls are relatively SharePoint-ish. You have to use the /drives endpoint in graph to target your container. This endpoint contains regular SharePoint doc links as well.

it's SharePoint-ish not SharePoint Online-ish right 😜. Well If I am not mistaken the /drives is not only used to get data from SharePoint but also OneDrive right? so I would say it is not SharePoint-ish but Folder-ish (and it makes total sense as a container is a kinda of a folder) but it may only be my wrong feeling about it 🙂

Also if it's not related to SharePoint Online, why did they include it into the SharePoint Online Management Shell module? This module is built to configure your SharePoint Online subscription. As mentioned before, haven't dug into it yet. Just dropping some ideas.

Currently, SPE is a public preview so the setup experience is kinda 'harsh' and I believe might still change in the future. Since SPE feature is under SPO admin site (similar like Stream App launcher tile 😜) it kinda make sense that Microsoft.Online.SharePoint.PowerShell module is used to manage that since this is the module that allows you to manage (or should allow 😉) everything you may (or may not) find under SharePoint Online admin site. It makes total sense to extend a SP module you already have (meaning SPO), with a couple of commands rather than creating a new one for some small new feature. Although Microsoft did it this way I would say we may do it 'better' 🙂

Also @milanholemans please consider my other (top of mind) arguments I had in previous post like longer naming or how I relate it to spfx 👍.

@pnp/cli-for-microsoft-365-maintainers what is your opinion on that? Should we go spe or spo embedded?

Adam-it commented 5 months ago

@waldekmastykarz, @Jwaegebaert I updated the container file commands to just file and added the m365 spe container type add command (@waldekmastykarz I agree the ensure at least now does not make sense). I am still researching how we may reuse the CSOM used by the New-SPOContainerType for that

waldekmastykarz commented 5 months ago

I suggest we go with spe instead of spo embedded. Let's try to go with something that feels intuitive to our users. Product names tend to change, so let's not put too much emphasis on it, and the APIs that we use aren't always self explanatory either.

Adam-it commented 5 months ago

@waldekmastykarz ok so I checked and we may indeed add a new container type for some AAD (Entra ID) registered application using CSOM and passing the following parameters:

<Request AddExpandoFieldTypeSuffix="true" SchemaVersion="15.0.0.0" LibraryVersion="16.0.0.0" ApplicationName="SharePoint Online PowerShell (16.0.24322.0)"
    xmlns="http://schemas.microsoft.com/sharepoint/clientquery/2009">
    <Actions>
        <ObjectPath Id="4" ObjectPathId="3" />
        <Method Name="NewSPOContainerType" Id="5" ObjectPathId="3">
            <Parameters>
                <Parameter TypeId="{5466648e-c306-441b-9df4-c09deef25cb1}">
                    <Property Name="AzureSubscriptionId" Type="Guid">{00000000-0000-0000-0000-000000000000}</Property>
                    <Property Name="ContainerTypeId" Type="Guid">{00000000-0000-0000-0000-000000000000}</Property>
                    <Property Name="CreationDate" Type="Null" />
                    <Property Name="DisplayName" Type="String">testTrail</Property>
                    <Property Name="ExpiryDate" Type="Null" />
                    <Property Name="IsBillingProfileRequired" Type="Boolean">false</Property>
                    <Property Name="OwningAppId" Type="Guid">{1b3b8660-9a44-4a7c-9c02-657f3ff5d5ac}</Property>
                    <Property Name="OwningTenantId" Type="Guid">{00000000-0000-0000-0000-000000000000}</Property>
                    <Property Name="Region" Type="Null" />
                    <Property Name="ResourceGroup" Type="Null" />
                    <Property Name="SPContainerTypeBillingClassification" Type="Enum">1</Property>
                </Parameter>
            </Parameters>
        </Method>
    </Actions>
    <ObjectPaths>
        <Constructor Id="3" TypeId="{268004ae-ef6b-4e9b-8425-127220d84719}" />
    </ObjectPaths>
</Request>

This is awesome 🤩 as this is the starting point of every SPE based solution. It all starts with a App registration (for this we already have a command) and the contrainer type assinged to it (this we may now add 😍🙂)

Adam-it commented 5 months ago

@pnp/cli-for-microsoft-365-maintainers any additional feedback before I start specking the commands out?

waldekmastykarz commented 5 months ago

Not from me. Let's go for it 🚀

Adam-it commented 5 months ago

@pnp/cli-for-microsoft-365-maintainers So apparently I was a bit wrong. We may not associate many apps to a container type but rather Container Type is a property stamped on every Container instance. Each Container Type is owned by one Application; and each Application can own only one Container Type. Then if needed the owner application may grant permission to other apps to modify containers of that container type. For me it is not yet clear how this grant work but a good use case would be for the CLI not to be the owner app (of course it may, but what would be the use case) but rather an additional helper console app to browse containers of a type. I need to research this a bit more but for now we may for sure start working on, add, get, list, remove commands for container types that will base in CSOM. I will start specking those commands ASAP

andrewconnell commented 5 months ago

FWIW, to save time on research, check out the postman collection the SPE team has shared: https://www.voitanos.io/blog/sharepoint-embedded-create-apps/#register-container-type-in-consumer-tenant

waldekmastykarz commented 5 months ago

Looks good @Adam-it, let's do it!

andrewconnell commented 3 months ago

Man, I wish I had some spare cycles to contribute to this... I will in the future, but until then, here's my contribution based on work I've been doing with the SPE engineering group and some gaps from what you can only do with SPO POSH or REST:

A lot of these are listed as research items... where are you tracking those as I can provide a TON of detail.

Also, this statement in the OP isn't correct:

The commands should base on MS Graph API support. More info on this in the following docs: https://learn.microsoft.com/en-us/sharepoint/dev/embedded/mslearn/m01-06-unit

It should be:

The commands should be based on MS Graph API support and SharePoint REST API v2.1 (aka: VROOM). More info on this in the following docs:
- [SPE docs](https://learn.microsoft.com/en-us/sharepoint/dev/embedded/overview)
- [MS Leearn module](https://learn.microsoft.com/en-us/training/modules/sharepoint-embedded-create-app/6-read-write-files-sharepoint-embedded)
waldekmastykarz commented 3 months ago

@andrewconnell thanks for chiming in, that's a ton of great info!

m365 spe container activate - all Containers created in "inactive" state... if still that way after 1-2 days, they're purged automatically; either upload content to it (which auto activates it) or manually activate it

Should spe container activate be a separate command or would spe container set --state active also work? For the latter, we could accommodate other possible changes, without introducing a separate command for just activating. Thoughts?

m365 spe container delete list

Should this be spe container deleted list which shows containers in the recycle bin?

andrewconnell commented 3 months ago

@waldekmastykarz said:

Should spe container activate be a separate command or would spe container set --state active also work? For the latter, we could accommodate other possible changes, without introducing a separate command for just activating. Thoughts?

If it's an either/or, I'd have it be a separate command because there could be a scenario where you create it but want to activate it hours later. But... from the workloads I've seen, it's usually something you do immediately after creation (like creation = 2-step process: create + activate.

So... having it as a separate command would be ideal, but also as an argument in the creation would also be a nice bonus.

Should this be spe container deleted list that shows containers in the recycle bin?

yeah, that makes sense.

I'd love to help with this... just need to find time & dig into what's involved in creating commands. It's frustrating to be forced into using SPO POSH cmdlets for some things you simply can't do easily with REST. I've been able to do almost everything with REST commands & Postman, but CLI would be so much easier.

Adam-it commented 2 months ago

@andrewconnell thanks for the awesome input to this issue. I totally feel your struggles about the 'finding the time' as I had similar issues myself for the last month, mainly focusing on preparing demos and SPFx toolkit. But in April I want to increase my priority on SPE support in CLI. I will start by going over your comments and changing the initial desc. 👍

Adam-it commented 2 months ago

@andrewconnell thanks for your awesome input in this issue. Your Rock 🤩

Some comments from my side till now (I am still and will be working on that to start pushing this forward)

As for:

  • suggest renaming all the commands related to "Container Types" to a single object - to me, container & container type implies the type is a kind or flavor of a container which isn't true. they are completely different things:

    • m365 spe container type * => m365 spe containerType

very good comment. Done ✅. I updated the commands to containertype as we use only lowercase in our commands.

As for:

The commands should be based on MS Graph API support and SharePoint REST API v2.1 (aka: VROOM). More info on this in the following docs:

Indeed 🙂. Thanks for the update. I see the docs were a bit rebuilt since I started working on this 😉

As for:

  • Suggest removing the following commands... all file management has zero difference with DocLibrary/OneDrive CRUD operations. Don't do special commands just for SPE:

    • m365 spe file list
    • m365 spe file add
    • m365 spe file remove

I disagree. Mainly because:

@andrewconnell what do you think about my arguments 👆. @pnp/cli-for-microsoft-365-maintainers what do you think?

As for:

  • suggest adding:

    • m365 spe containerType permission add - this is related to the register command you already have. registration is just adding an app+permissions to the Container Type's permissions collection... you can also add guest apps this same way. the concept of "registering" is just adding the first permission in your tenant.
    • m365 spe containerType permission remove
    • m365 spe container activate - all Containers created in "inactive" state... if still that way after 1-2 days, they're purged automatically; either upload content to it (which auto activates it) or manually activate it
    • m365 spe container remove - send to recycle bin
    • m365 spe container restore - restore from recycle bin
    • m365 spe container purge - remove sends to recycle bin for <=93d, after that it's perm deleted. you can force the removal from the recycle bin with MS Graph
    • m365 spe container delete list
    • m365 spe container permission add
    • m365 spe container permission update
    • m365 spe container permission remove
    • m365 spe container permission list
    • m365 spe container property add
    • m365 spe container property remove
    • m365 spe container property list
    • m365 spe container column add
    • m365 spe container column remove
    • m365 spe container column set
    • m365 spe container column list

A lot of these are listed as research items... where are you tracking those as I can provide a TON of detail.

yes this is a very good list. For now I updated the issue description only to containertype commands and will first start adding specs for those but I will be slowly adding more and more commands in this issue to track all SPE features and the list you provided is a goldmine 🤩. Thanks

andrewconnell commented 2 months ago

@Adam-it said, WRT the commands for performing file CRUD ops...

what do you think about my arguments

I understand your points. I don't have an opinion.

martinlingstuyl commented 2 months ago

I think I'd personally like the spe group to be just about CRUD on spe, not about CRUD on spe content, like files.

So using m365 file add etc for containers as well would be fantastic from my point of view. We could create a consolidated list of commands for working with SharePoint entities through the modern endpoints that are available. I agree with you that commands aren't meant to be API wrappers though. I would see this as a gradual refactoring to a more modern approach. We'll need to eventually do that I guess and not get bogged down because of the different options that are needed with Graph commands VS classic REST /CSOM commands.

But that would mean that all those commands would need two extra command options (containerId and containerName) and maybe it might get more complicated to see how it's meant to be used. Which would speak to your idea @Adam-it...

So... just a couple of thoughts.

Adam-it commented 2 months ago

I think I'd personally like the spe group to be just about CRUD on spe, not about CRUD on spe content, like files.

So using m365 file add etc for containers as well would be fantastic from my point of view. We could create a consolidated list of commands for working with SharePoint entities through the modern endpoints that are available. I agree with you that commands aren't meant to be API wrappers though. I would see this as a gradual refactoring to a more modern approach. We'll need to eventually do that I guess and not get bogged down because of the different options that are needed with Graph commands VS classic REST /CSOM commands.

But that would mean that all those commands would need two extra command options (containerId and containerName) and maybe it might get more complicated to see how it's meant to be used. Which would speak to your idea @Adam-it...

So... just a couple of thoughts.

How I described it is that we would still have the m365 file add command and the main logic there. Here we would just additionally expose this command under spe are giving more user-friendly options which are just specific to SPE containers

waldekmastykarz commented 2 months ago

Having all SPE-related commands under one group simplifies their discovery. When you're looking for SPE stuff, you need to look at just one place, without having to understand how it's implemented in the API or in the CLI. That said, we can solve this with an alias and adding containerId and containerName options. If the majority of the command logic is the same, then it would be easier for us to add to what we've got already instead of adding a new command which is pretty much exactly the same.

Adam-it commented 2 months ago

Having all SPE-related commands under one group simplifies their discovery. When you're looking for SPE stuff, you need to look at just one place, without having to understand how it's implemented in the API or in the CLI. That said, we can solve this with an alias and adding containerId and containerName options. If the majority of the command logic is the same, then it would be easier for us to add to what we've got already instead of adding a new command which is pretty much exactly the same.

thanks @waldekmastykarz for your response. Thats exactly what I had in mind. So reuse the commands we have and expose then in spe area as an alias with additional options that are spe specific.

andrewconnell commented 2 months ago

@waldekmastykarz said:

... we can solve this with an alias and adding containerId and containerName options ...

What's the thinking behind using containerName? If you want to target a specific container, you'd use the ID, not the name... right? I don't believe names are necessarily unique.

Adam-it commented 2 months ago

@waldekmastykarz said:

... we can solve this with an alias and adding containerId and containerName options ...

What's the thinking behind using containerName? If you want to target a specific container, you'd use the ID, not the name... right? I don't believe names are necessarily unique.

Doesn't have to be. If CLI finds more than one container with the same name then CLI will usually show a prompt to select the correct one based on ID. Working with names not guids is more user-friendly don't you think? This is how we (humans) talk and memorize things, giving them names. Its hard to remember a set of numbers.

Adam-it commented 2 months ago

@andrewconnell did you ever used the Set-SPOContainerType command?

andrewconnell commented 2 months ago

Yes, when defining / changing billing details.

Adam-it commented 2 weeks ago

@MathijsVerbeeck did by any chance you had some time to look into using REST calls for the list, get, add, set SPE container types commands? I saw some already have PR's openned based on my research so based on using CSOM. Till now I did not have much luck with the REST approach at least for this part