search

pnp / PnP-PowerShell

SharePoint PnP PowerShell CmdLets
https://pnp.github.io/powershell
Other
990 stars 663 forks source link

sourceUri is now generated using the file's folder URL instead of the file URL.(Issue 2780) #2796

Closed kadu-jr closed 3 years ago

kadu-jr commented 4 years ago

Type

Related Issues?

Fixes #2780.

What is in this Pull Request ?

The souceUri variable used in the WebUrlFromFolderUrlDirect method is now built using the URL of the folder containing the file. The goal is to never send the URL of the file, since this method will currently throw an exception if you send a the URL of a json file. Omitting the file name works just fine.

KoenZomers commented 3 years ago

Thanks for your contribution @kadu-jr and sorry for the long wait on this one. I just tested your PR. Although I cannot reproduce the issue you are describing, looking at the proposed source code change, I also see no harm in applying this change as we only use it to get the target folder anyway, so if it does make a difference in some scenarios, it should be fine. Marking this change ready for merge in the next release.

kadu-jr commented 3 years ago

Hi @KoenZomers. Thanks for looking into it. However, if I recall correctly, this fix would cause some errors on different cases(i.e. copying a folder), like we discussed in #2780 . I have to download the code to my new machine in order to test it, so it may take some time, but I can try to test it again this week and make sure that it's not breaking anything.

Benoleg commented 3 years ago

Hello @kadu-jr, @KoenZomers, I've just tried to use the Copy-PnPFile command (3.28.2012.0) to copy a file from a SharePoint site to another one. The command now copies the folder (and all its files) instead of copying only the file I've specified (due to this change if I understand correctly as only the string before the last / is kept). That means that the examples provided on Microsoft site don't work anymore. As a workaround, I've added a "/' at the end of the file name but I'm concerned that this may not work anymore if the behaviour is corrected in a future release... Sorry if it is not the right place to comment but that's the first time I do so...

kadu-jr commented 3 years ago

Hello @Benoleg ,

I was afraid this was the case - hence my previous comment. Unfortunately this repo is closed(no longer accepting pull requests). The new PnP library moving forward is this one. If you can reproduce the error with this new library(and I think you will be able to), create an issue over there. I will try my best to at least issue a new pull request reverting these changes.