Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Folder picker does not works if outside of current site collection #1379

Closed
stevebeauge opened this issue Dec 6, 2022 · 7 comments · Fixed by #1709
Closed

Folder picker does not works if outside of current site collection #1379

stevebeauge opened this issue Dec 6, 2022 · 7 comments · Fixed by #1709

Comments

@stevebeauge
Copy link
Contributor

Category

[ ] Enhancement

[X] Bug

[ ] Question

Version

Please specify what version of the library you are using: [ 3.12.0 ]

Expected / Desired Behavior / Question

The folder picker should allow setting the rootFolder to any location the current has permission to access.

Observed Behavior

If the rootFolder is set to a folder in another site collection, the picker is empty and the new folder action fails :
image

When the "Select folder" icon is click the dev console emits :

image

The failing network request is:

https://myTenant.sharepoint.com/sites/sitecoll1/_api/web/getFolderByServerRelativePath(decodedUrl='%2Fsites%2Fsitecoll2%2FDocuments%20partages')/folders?$select=Name,ServerRelativeUrl&$orderby=Name%20asc

And its result is:

{
    "odata.error": {
        "code": "-2147024809, System.ArgumentException",
        "message": {
            "lang": "en-US",
            "value": "Server relative urls must start with SPWeb.ServerRelativeUrl"
        }
    }
}

My Analysis

Looking at the failing request, we can see the getFolderByServerRelativePath method is called onto the current page's web. Not the web targeted by the root folder, which is required.

Steps to Reproduce

Create a webpart which use FolderPicker and point to a folder outside the current site collection.

My repro code (I can push the full project if needed):

import { FolderPicker, IFolder } from '@pnp/spfx-controls-react';
import * as React from 'react';

import { BaseComponentContext } from '@microsoft/sp-component-base';
import { SPHttpClient } from '@microsoft/sp-http';
import { ISite, SitePicker } from '@pnp/spfx-controls-react/lib/SitePicker';
import { Stack } from 'office-ui-fabric-react';

export type FolderPickerTestProps = {
  context: BaseComponentContext
}

type LibraryInfo = {
  webUrl: string;
  name: string;
}

// Ask the graph to provide the default library for a site
const getDefaultLibrary = async (context: BaseComponentContext, siteAbsoluteUrl: string): Promise<LibraryInfo> => {
  const apiUrl = `${siteAbsoluteUrl}/_api/v2.0/drive?$select=webUrl,name`;

  const response = await context.spHttpClient.get(apiUrl, SPHttpClient.configurations.v1);

  if (!response.ok) {
    throw new Error(`[${response.status}] Error fetching default library: ${await response.text()}`);
  }

  return await response.json();
}

const FolderPickerTest: React.FC<FolderPickerTestProps> = ({ context }) => {

  const [defaultLibrary, setDefaultLibrary] = React.useState<LibraryInfo | undefined>(undefined);
  const [selectedFolder, setSelectedFolder] = React.useState<IFolder | undefined>(undefined);

  // After a site is selected, get its default library
  const onSiteSelect = (site: ISite | undefined): void => {
    if (!site?.url) { setDefaultLibrary(undefined); return; }

    getDefaultLibrary(context, site.url)
      .then(setDefaultLibrary)
      .catch((error) => alert);
  }

  const data = {    defaultLibrary,    selectedFolder,  };

  return (
    <Stack>
      <SitePicker context={context} onChange={selectedSites => onSiteSelect(selectedSites[0])} multiSelect={false} />
      {defaultLibrary && (
        <FolderPicker context={context}
          label={''}
          rootFolder={{
            Name: defaultLibrary.name,
            ServerRelativeUrl: decodeURIComponent(new URL(defaultLibrary.webUrl).pathname), // Get the server relative url, with spaces decoded
          }}
          onSelect={folder => {
            setSelectedFolder({ ...folder });
          }}
          canCreateFolders
        />
      )}
      <h3>Data:</h3>
      <pre>
        <code>{JSON.stringify(data, null, 2)}</code>
      </pre>
    </Stack>
  );

}

export { FolderPickerTest };
@stevebeauge
Copy link
Contributor Author

Ok, actually it works as expected if siteAbsoluteUrl is also set to the correct site.

        <FolderPicker context={context}
          label={''}
          siteAbsoluteUrl={defaultLibrary.siteAbsoluteUrl}
          rootFolder={{
            Name: defaultLibrary.name,
            ServerRelativeUrl: decodeURIComponent(new URL(defaultLibrary.webUrl).pathname), // Get the server relative url, with spaces decoded
          }}
          onSelect={folder => {
            setSelectedFolder({ ...folder });
          }}
          canCreateFolders
        />

However, I think there's a hole in the documentation of the component which doesn't mention this prop.

An earlier check could also be beneficial (by checkinf if rootFolder.ServerRelativeUrl starts with siteAbsoluteUrl for example).

@michaelmaillot
Copy link
Collaborator

Hi @stevebeauge,

I agree about the missing prop in the control's doc.

Regarding the earlier check mentioned above, the thing is that the FolderPicker control is using the FolderExplorer one under the hood, which works with "current web" context by default and only outside when using the siteAbsoluteUrl prop is used.

From my perspective, the rootFolder.ServerRelativeUrl prop should not contain an absolute URL (as it's relative and because of usage of getFolderByServerRelativePath API behind the scene). But documentation should be more clear to indicate that this one targets current web context by default if siteAbsoluteUrl is not used.

@stevebeauge
Copy link
Contributor Author

Dealing with different kinds of url is sometimes painful. I wrote a utility package to use in my projects, that allows me to rationalize URL forms.

Here are the relevant parts, if it can help:

export const toServerRelativeUrl = (url: string): string => {
  const fullUrl = new URL(url, document.location.href);
  return decodeURIComponent(fullUrl.pathname); // decodeURIComponent is needed to decoded urls like %20 for spaces
};

export const toAbsoluteUrl = (url: string): string => {
  const fullUrl = new URL(url, document.location.href);
  return fullUrl.toString();
};

export const toWebRelativeUrl = (webUrl: string, objectUrl: string): string => {
  const webUrlObj = new URL(webUrl, document.location.href);
  const objectUrlObj = new URL(objectUrl, webUrlObj);

  if (
    !objectUrlObj
      .toString()
      .toLocaleLowerCase()
      .startsWith(webUrlObj.toString().toLocaleLowerCase())
  ) {
    throw new Error("Object url is not located inside the specified web");
  }

  const objServerRelativeUrl = decodeURIComponent(objectUrlObj.pathname);
  const webServerRelativeUrl = decodeURIComponent(webUrlObj.pathname);
  return trimStart(objServerRelativeUrl.replace(webServerRelativeUrl, ""), "/");
};

/**
 * Trims a string from the beginning.
 * @param input The input string.
 * @param c The characters to trim (default is whitespace).
 * @param ignoreCase Whether to ignore case when trimming (default is true).
 * @returns The input string with leading characters removed.
 */
export const trimStart = (input: string, c = '\\s', ignoreCase = true): string => {
    // Use a regular expression to match the specified characters at the beginning of the string,
    // and remove them. The `ignoreCase` option controls whether the match is case-insensitive.
    return input.replace(new RegExp(`^[${c}]+`, ignoreCase ? 'gi' : 'g'), '');
};

The trick is to let the standard URL type handle the different forms of URL as input and produce the desired form.

This way, whenever I need to add sharepoint URL as props of my component, I don't have to force the user to think about which kind of URL is required.

ex:

describe("urlUtils", () => {
  test("toWebRelativeUrl absolute + absolute", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "https://contoso.sharepoint.com/sites/site1",
        "https://contoso.sharepoint.com/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl absolute + absolute / leading slash", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "https://contoso.sharepoint.com/sites/site1/",
        "https://contoso.sharepoint.com/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl relative + absolute", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "/sites/site1",
        "https://contoso.sharepoint.com/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl relative + absolute / leading slash", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "/sites/site1/",
        "https://contoso.sharepoint.com/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl absolute + relative", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "https://contoso.sharepoint.com/sites/site1",
        "/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl absolute + relative / leading slash", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "https://contoso.sharepoint.com/sites/site1/",
        "/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl relative + relative", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "/sites/site1",
        "/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl relative + relative / leading slash", () => {
    assert.equal(
      urlUtils.toWebRelativeUrl(
        "/sites/site1/",
        "/sites/site1/Shared documents",
      ),
      "Shared documents",
    );
  });
  test("toWebRelativeUrl absolute + absolute / different site", () => {
    expect(() =>
      urlUtils.toWebRelativeUrl(
        "https://contoso.sharepoint.com/sites/site1",
        "https://contoso.sharepoint.com/sites/site2/Shared documents",
      ),
    ).toThrowError();
  });
});

Feel free to reuse it.

@joelfmrodrigues
Copy link
Collaborator

Ok, actually it works as expected if siteAbsoluteUrl is also set to the correct site.

        <FolderPicker context={context}
          label={''}
          siteAbsoluteUrl={defaultLibrary.siteAbsoluteUrl}
          rootFolder={{
            Name: defaultLibrary.name,
            ServerRelativeUrl: decodeURIComponent(new URL(defaultLibrary.webUrl).pathname), // Get the server relative url, with spaces decoded
          }}
          onSelect={folder => {
            setSelectedFolder({ ...folder });
          }}
          canCreateFolders
        />

However, I think there's a hole in the documentation of the component which doesn't mention this prop.

An earlier check could also be beneficial (by checkinf if rootFolder.ServerRelativeUrl starts with siteAbsoluteUrl for example).

Definitely missing in documentation, thanks @stevebeauge for spotting it. Would you be interested in submitting a PR?

@stevebeauge
Copy link
Contributor Author

?

You mean a PR for improving the docs or a PR to add the url utility ? Or both ?

@joelfmrodrigues
Copy link
Collaborator

nful. I wrote a utility package to use in my projects, that allows me to rationalize URL forms.

Here are the relevant parts, if it can help:

@stevebeauge I was referring to the docs for siteAbsoluteUrl .
Regarding the improvement mentioned by @michaelmaillot , this is valid but maybe we should create a separate issue for it to keep discussions short and focused. @michaelmaillot can you create a new issue please if not created yet?

@michaelmaillot
Copy link
Collaborator

Done: #1658.

@joelfmrodrigues joelfmrodrigues added the status:fixed-next-drop Issue will be fixed in upcoming release. label Sep 26, 2023
@joelfmrodrigues joelfmrodrigues added this to the 3.16.0 milestone Sep 26, 2023
@AJIXuMuK AJIXuMuK mentioned this issue Nov 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants