New command: m365 file move

appieschot commented 1 year ago

Usage

m365 file move

Description

Moves a file to another location using the Microsoft Graph

Options

Option	Description
`-u, --webUrl <webUrl>`	The URL of the site where the file is located.
`-s, --sourceUrl <sourceUrl>`	Server-relative or absolute URL of the file.
`-t, --targetUrl <targetUrl>`	Server-relative or absolute URL of the location.
`--newName [newName]`	New name of the destination file.
`--nameConflictBehavior [nameConflictBehavior]`	Behavior when a document with the same name is already present at the destination. Possible values: `fail`, `replace`, `rename`. Default is `fail`.

Examples

Moves a file using the graph

m365 file move --webUrl https://contoso.sharepoint.com/sites/project --sourceUrl "/sites/project/Shared Documents/Document.pdf" --targetUrl "/sites/IT/Shared Documents"

Default properties

No response

Additional Info

We currently have a m365 file add command using the Microsoft Graph this should be part of that command structure
Should use the new Drive API
Validate if we already have appropriate permissions for this command

Two remarks;

If we agree on this spec I shall create one for the m365 file copy option as well (it will be the same structure)
We most likely want to have a m365 folder move and m365 file copy as well, the source could/would be the almost same. We could use an Alias for that and have our --sourceUrl reflect both files and folders.
We should consider support for the driveItem since that is something that will be returned by the m365 file list

@pnp/cli-for-microsoft-365-maintainers please chime in :D

waldekmastykarz commented 1 year ago

spec looks good
let's create one for file copy
if folder move and folder copy would work the same way as file move and file copy then having an alias would be sufficient

We should consider support for the driveItem since that is something that will be returned by the m365 file list

Could you please elaborate what you mean here? Adding the file list command or something else?

Jwaegebaert commented 1 year ago

Maybe it would be good to add a remark to the docs explaining the difference between the commands m365 file move and m365 spo file move. Now it can be quite confusing to see two different commands with quite similar examples. They both can move an item in SharePoint as shown in the examples.

milanholemans commented 1 year ago

I know we already have Graph commands that add a file to SharePoint for example, but I actually don't understand why we have 2 commands for this. I don't think the user cares whether we use SPO API or Graph API. Also when we use Graph API, we first have to use the SPO API to retrieve Drive ID & DriveItem ID. So to be honest I don't get why we are creating duplicate commands. 😊 Also looking at the usage, spo commands are used a lot more.

waldekmastykarz commented 1 year ago

In many cases, there's a discrepancy in functionality between SharePoint and Microsoft Graph APIs. In such cases, we often two alternatives so that users can choose themselves which way they want to go. I suggest we always default to Microsoft Graph APIs, because they're better documented, and fallback to SharePoint APIs if there's no alternative.

Jwaegebaert commented 1 year ago

Would the idea be then to combine both commands into one? If so, what will the command name be that we use? We could of course keep both and change one to an alias for the other.

Adam-it commented 1 year ago

maybe we should some info what we would recommend in the m365 spo file copy or maybe just info of the alternative command using graph 😉

waldekmastykarz commented 1 year ago

The main question is if both commands are the same when it comes to functionality, output and required permissions? If the answer is 'yes', then we could remove the spo variant, add an alias for now, mark the spo alias as deprecated, and then in the next major version remove the spo alias. But it all stands or falls on both APIs being identical.

appieschot commented 1 year ago

Could you please elaborate what you mean here? Adding the file list command or something else?

We should consider adding an optional driveItem (or driveitemId) property since the graph works with drive item ids (not only filenames / paths). It makes the command quicker if you know the id (something that is listed when calling the m365 file list command.

appieschot commented 1 year ago

AFAIK currently they are not the same, the reason we would need the graph version is for scenario's like retrieving the thumbnail or converting to PDF; something that is native to the graph so would be awesome to have in our use case 🔥

milanholemans commented 1 year ago

AFAIK currently they are not the same, the reason we would need the graph version is for scenario's like retrieving the thumbnail or converting to PDF; something that is native to the graph so would be awesome to have in our use case 🔥

The API requests are not the same indeed. But the outcome is the same right? Whether you move/copy a file via SP REST or Graph, the outcome should be the same. I understand thumbnails and PDF conversion are Graph related, but they can't they be done afterwards? You could use spo file move and when that's done you can use file convert pdf.

Adam-it commented 1 year ago

hey @pnp/cli-for-microsoft-365-maintainers did the discussion actually ended here? I think @milanholemans has a valid comment 🤔 maybe we could revisit, @waldekmastykarz, @appieschot, @Jwaegebaert what do you think?

appieschot commented 1 year ago

I still feel we need this command, and personally need the graph implementation version. I am not sure if the outcome is 100% the same result so am hesitant to update our spo file move and would opt for providing both.

Jwaegebaert commented 1 year ago

We want our commands to be easy to use. In my opinion, it would be worth exploring whether we can merge them together. I'm all for working more with Graph, but if we can simplify things by having one command being able to call both endpoints, that would be way easier. 😄

Saurabh7019 commented 4 months ago

Is the discussion still ongoing? If not, can I work on it?

milanholemans commented 4 months ago

Since we merged a new command file copy in this release, I think we chose to implement this indeed.

Saurabh7019 commented 4 months ago

I've just noticed that the DriveItems API only supports moving items within folders in the same drive, not across drives like between libraries. Would that be an issue?

Saurabh7019 commented 4 months ago

Hi @milanholemans, Before I start working on it, I just wanted to check if it would be acceptable to implement the command for moving items between folders within the same drive?

milanholemans commented 4 months ago

If this is an API restriction, then it is what it is I guess 🤷 I suggest that we add an admonition to the docs where we tell the user that it's only possible to move within the same library. @appieschot agreed?

Adam-it commented 4 months ago

a bit of a bummer right 😉. But I agree with @milanholemans that unfortunately we need to accept it or we may do a bit of a hack here and use what was implemented for the copy command to copy a file between drives and then use delete endpoint to remove the current file 🤔 @pnp/cli-for-microsoft-365-maintainers any comments on this?

martinlingstuyl commented 4 months ago

Sounds like a plan... it's a workaround indeed, but I can't find a reason why it should really be a problem..

Saurabh7019 commented 4 months ago

Copying DriveItem doesn't include the versions, whereas moving does.

Adam-it commented 4 months ago

Copying DriveItem doesn't include the versions, whereas moving does.

That's a good point 👍. Thanks for mentioning that. What if we try to tackle both things 🤔. if the command runs in the context of the same library then we are able to use the move endpoint and therefore we may move the file with version history. If the user will run the command to move the file between different drives then we under the hood to copy+delete combination to move the file and present a warning to the user that in this case version history is not included. We should also highlight it in the spec as well. @pnp/cli-for-microsoft-365-maintainers any comments? @martinlingstuyl, @milanholemans what's your take on that? This way we support multiple scenarios and are not restricted to only a single drive which is very limiting 🤔🙂

pnp / cli-microsoft365