Skip to content
This repository has been archived by the owner on Jan 19, 2021. It is now read-only.

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

Merged
merged 4 commits into from
Dec 1, 2020

Conversation

kadu-jr
Copy link

@kadu-jr kadu-jr commented Jul 16, 2020

Type

  • Bug Fix
  • New Feature
  • Sample

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 KoenZomers added the status:checked and ready for merge ✔ PR has been checked and is ready for merge label Nov 16, 2020
@KoenZomers KoenZomers removed their assignment Nov 16, 2020
@KoenZomers
Copy link
Collaborator

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.

KoenZomers added a commit to KoenZomers/pnppowershell that referenced this pull request Nov 16, 2020
@kadu-jr
Copy link
Author

kadu-jr commented Nov 16, 2020

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.

@erwinvanhunen erwinvanhunen merged commit 8acb2ac into pnp:dev Dec 1, 2020
@Benoleg
Copy link

Benoleg commented Jan 8, 2021

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
Copy link
Author

kadu-jr commented Jan 8, 2021

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.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
status:checked and ready for merge ✔ PR has been checked and is ready for merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants