SharePoint / PnP-JS-Core

Code moved to https://github.com/pnp/pnpjs. This repository is archived.
Other
380 stars 231 forks source link

GetAllProperties errors on site page in Application customizer #756

Closed JWiersema closed 6 years ago

JWiersema commented 6 years ago

Thank you for reporting an issue, suggesting an enhancement, or asking a question. We appreciate your feedback - to help the team understand your needs please complete the below template to ensure we have the details to help. Thanks!

Please check out the Developer Guide to see if your question is already addressed there. This will help us ensure our documentation covers the most frequent questions.

Category

[ ] Enhancement

[ X] Bug

[ ] Question

Version

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

If you are not using the latest release, please update and see if the issue is resolved before submitting an issue.

Expected / Desired Behavior / Question

Getting all properties of the current web

Observed Behavior

404 error because of wrong api url. GET https://mytenant.sharepoint.com/sites/styletransfer/**SitePages**/_api/web/allproperties 404 () This happens in an application customizer on the homepage of a communication site. I'm serving the customizer from localhost with the debug URL.

Steps to Reproduce

Do something like this in an application customizer and run it on a SitePage. sp.web.allProperties.get().then(w => { if (w[this.propertynameAutoPlay]) { console.log(w[this.propertynameAutoPlay]); this.properties.autoPlay = w[this.propertynameAutoPlay]; }

NB: Same bug exists in sp-pnp-js

koltyakov commented 6 years ago

Hi @JWiersema,

Are you following SPFx context configuration tips. Looks like you missed setup required with SPFx:

pnp.setup({
  spfxContext: this.context
});
JWiersema commented 6 years ago

I have that in place for @pnp but it doesnt help. Didnt have it in place for sp-pnp-js.

JWiersema commented 6 years ago

I have done this: Created an Application customizer project. Installed @ pnp Added the setup in OnInit:

return super.onInit().then(_ => {
      sp.setup({
        spfxContext: this.context
      });
    });

Used PNP like this: sp.site.rootWeb.lists.getByTitle('ScreenreaderSettings').items.select("screenreader_apiUrl","screenreader_autoPlay","screenreader_selectors").top(1).get().then((items: any[]) => { console.log(items);

This doesnt work as the below URL is called, which is incorrect. https://mytenant.sharepoint.com/sites/_api/site/rootweb/lists/getByTitle('ScreenreaderSettings')/items?$select=screenreader_apiUrl,screenreader_autoPlay,screenreader_selectors&$top=1

This is the correct link: https://mytenant.sharepoint.com/sites/**SPFxWebparts3**/_api/site/rootweb/lists/getByTitle('ScreenreaderSettings')/items?$select=screenreader_apiUrl,screenreader_autoPlay,screenreader_selectors&$top=1

So I have a similar error on a different site. The URL is not determined correctly. If I'm doing something wrong please let me know.

KEMiCZA commented 6 years ago

Could it be that you need to set the headers?

sp.setup({
  sp: {
    headers: { 'Accept': 'application/json;odata=verbose' }
  },
  spfxContext: this.context
});
JWiersema commented 6 years ago

How does that help if the REST api URL is incorrect? spfx_screenreader_pnp_error

KEMiCZA commented 6 years ago

@JWiersema you have a point. I used to have this issue a lot, seemed worth to try it.

koltyakov commented 6 years ago

Is it the latest SPFx version (1.4.1), you're using? The key word to try here on our end I guess is 'Application customizer'.

JWiersema commented 6 years ago

Yes, 1.4.1. I think it's an error specific to application customizers, as I have used sp-pnp-js for an SPFx webpart previously without any problems.

JWiersema commented 6 years ago

I'm getting the error on these URLs btw: https://jwiersem.sharepoint.com/sites/SPFxWebparts3 (here it rips off the last part SPFxWebParts3, which it shouldnt) and https://jwiersem.sharepoint.com/sites/styletransfer/SitePages/Home.aspx (Here it only removes Home.aspx and not SitePages)

KEMiCZA commented 6 years ago

@JWiersema I've tested this last week with SPFx version 1.4.1. and it worked correctly. So I wasn't able to reproduce your error. Can you share more of your code?

jwiersem commented 6 years ago

Sorry, I can't because I already replaced PnP with a sphttpclient call. Did you create an application customizer?

KEMiCZA commented 6 years ago

@jwiersem Yes, I did

export default class HwCustomizerApplicationCustomizer
  extends BaseApplicationCustomizer<IHwCustomizerApplicationCustomizerProperties> {

  @override
  public onInit(): Promise<void> {

    sp.setup({
      spfxContext: this.context
    });

    sp.site.rootWeb.lists.getByTitle('Documenten')
      .items.select("FileLeafRef").top(1).get().then((items: any[]) => {
        console.error(items[0]);
        Dialog.alert(`${items[0].FileLeafRef}`);
      });

    Log.info(LOG_SOURCE, `Initialized ${strings.Title}`);

    let message: string = this.properties.testMessage;
    if (!message) {
      message = '(No properties were provided.)';
    }

    return Promise.resolve();
  }
}

When I execute this in a subsite inside a custom sitepage, it works correctly. I even created another subsite and still no problem.

jwiersem commented 6 years ago

Here is my current code: https://github.com/jwiersem/sharepointscreenreader/ Line 60 of https://github.com/jwiersem/sharepointscreenreader/blob/master/Code/spfxscreenreader/src/extensions/spfxScreenreader/SpfxScreenreaderApplicationCustomizer.ts used to have the pnp call for the web properties.

jwiersem commented 6 years ago

Did you try it on the homepage of the site? That was where I got the error. Also, my call was for the property bag of the web, not for a list.

KEMiCZA commented 6 years ago

This code works on the following URL's:

using the following URL metadata, because I'm running the scripts from localhost: ?loadSPFX=true&debugManifestsFile=https://localhost:4321/temp/manifests.js&customActions={"555da961-a47b-4710-a36c-00eefe940c33":{"location":"ClientSideExtension.ApplicationCustomizer","properties":{"Top":"Top%20area%20of%20the%20page","Bottom":"Bottom%20area%20in%20the%20page"}}} code:

  @override
  public onInit(): Promise<void> {

    sp.setup({
      spfxContext: this.context
    });

    sp.web.allProperties.get().then(w => {
      console.info((w as any)["odata.id"]);
    });

    return Promise.resolve();
  }

Result in console log for subsite: image

Result in console log for rootsite: image

This code does NOT work on both URL's (removed sp.setup call):

  @override
  public onInit(): Promise<void> {

    sp.web.allProperties.get().then(w => {
      console.info((w as any)["odata.id"]);
    });

    return Promise.resolve();
  }

output in console: image netutil.ts:62 GET https://m365x924991.sharepoint.com/sites/test1/SitePages/_api/web/allproperties 404 (Not Found)

So I'm not sure where it went wrong for you. It must be the call to sp.setup that failed or was not correctly configured, or maybe even misplaced?

jwiersem commented 6 years ago

Ok, well then you have found the cause I guess. I did have it in place in OnInit though. I changed one other thing in OnInit and that is to make it async. Unless I'm mistaken, in your code you can not depend on the call to have returned if you add code in the render method that depends on it, correct..?

jwiersem commented 6 years ago

I just retried it on the project I linked (which runs fine). Only changed these things:

ran npm i @pnp/logging @pnp/common @pnp/odata @pnp/sp @pnp/nodejs

added import { sp } from "@pnp/sp";

changed return of onInit return super.onInit().then(_ => { // other init code may be present sp.setup({ spfxContext: this.context }); }); called in _renderPlaceHolders sp.web.allProperties.get().then(w => { if (w[this.propertynameAutoPlay]) { console.log(w[this.propertynameAutoPlay]); // this.properties.autoPlay = w[this.propertynameAutoPlay]; }} ); Still 404 errors because of a call to https://jwiersem.sharepoint.com/sites/_api/web/allproperties

KEMiCZA commented 6 years ago

@jwiersem try it like this:

  @override
  public async onInit(): Promise<void> {

    sp.setup({
      spfxContext: this.context
    });

    // Added to handle possible changes on the existence of placeholders.
    this.context.placeholderProvider.changedEvent.add(this, this._renderPlaceHolders);

    // Call render method for generating the HTML elements.
    this._renderPlaceHolders();

    return super.onInit().then(_ => {
      console.log("OnInit ran.");
    });

    // return Promise.resolve();
  }

This does not work for me either:

  @override
  public async onInit(): Promise<void> {

    // Added to handle possible changes on the existence of placeholders.
    this.context.placeholderProvider.changedEvent.add(this, this._renderPlaceHolders);

    // Call render method for generating the HTML elements.
    this._renderPlaceHolders();

    return super.onInit().then(_ => {
      sp.setup({
        spfxContext: this.context
      });
      console.log("OnInit ran.");
    });

    // return Promise.resolve();
  }

Update: This works as well for me:

  @override
  public async onInit(): Promise<void> {
    return super.onInit().then(_ => {
      sp.setup({
        spfxContext: this.context
      });

      // Added to handle possible changes on the existence of placeholders.
      this.context.placeholderProvider.changedEvent.add(this, this._renderPlaceHolders);

      // Call render method for generating the HTML elements.
      this._renderPlaceHolders();
      console.log("OnInit ran.");
    });
  }
jwiersem commented 6 years ago

I guess the documentation needs to be given a little update then :-) https://pnp.github.io/pnp/getting-started.html

EDIT: Or I guess I'm reading it wrong.. It's not about replacing the return object, it's about moving all code in the then method.

patrick-rodgers commented 6 years ago

@JWiersema - Catching up on things today - so it looks like the issues was you were running the code that relies on setup before setup was called. You can take two approaches - either that @KEMiCZA took with placing the setup call first - or placing all code, in this case _renderPlaceHolders, that relies on setup executing in the then.

Going to close this as answered, please reopen if you have further questions. Thanks!

And thanks @KEMiCZA for the help.