pnp / pnpjs

Fluent JavaScript API for SharePoint and Microsoft Graph REST APIs
https://pnp.github.io/pnpjs/
Other
753 stars 305 forks source link

getAllChildrenAsTree does not include an option to retrieve properties. #3083

Closed JelleCeulemans closed 2 weeks ago

JelleCeulemans commented 1 month ago

What version of PnPjs library you are using

4.x

Minor Version Number

4.2.0

Target environment

SharePoint Framework

Additional environment details

SPFx 1.19.0

Question/Request

The documentation explains that since version 2.6.0, it has been possible to retrieve properties from terms when using getAllChildrenAsOrderedTree. However, in Pnp 4.2.0, this function has been renamed to getAllChildrenAsTree, and it no longer supports selecting graph properties or using the retrieveProperties parameter.

https://pnp.github.io/pnpjs/graph/taxonomy/#getallchildrenasorderedtree

const childTree = await set.getAllChildrenAsOrderedTree({ retrieveProperties: true });

image

JelleCeulemans commented 1 month ago

@patrick-rodgers

Could this be related to your commit from January 30th? https://github.com/pnp/pnpjs/commit/1b4175971ce27088afdbb83b708ccaaa55455e16

juliemturner commented 1 month ago

I'm sorry, is your question, can it? My feeling would be that we wouldn't change that function to include properties because of the overhead of the call. But you certainly can copy that logic and try to extend the request for children using the select OData query property to include the terms properties. If you find success, we'll happily accept a PR for an additional method, but I think we won't want to change the existing one.

patrick-rodgers commented 1 month ago

You mean the commit that added the method? We changed to the new Graph API for taxonomy that has different capabilities and behavior - so I don't think at the time we could bring the properties back in the same way.

JelleCeulemans commented 1 month ago

@patrick-rodgers Yes, I am aware that since version 4, the taxonomy part has moved to the graph approach, and by default, custom properties are not included in the call's result. To retrieve these, you need to use the select OData functionality.

https://graph.microsoft.com/beta/termStore/sets/{setId}/children

https://graph.microsoft.com/beta/termStore/sets/{setId}/children?$select=id,createdDateTime,lastModifiedDateTime,labels,descriptions,properties

image

@juliemturner I also forked the solution and attempted to get it running through the settings file, but I haven't succeeded yet.

juliemturner commented 1 month ago

@JelleCeulemans My comment was more around just creating a method in your solution that mimic's the same thing using that source code as a starting point. It's a helper function in the library so you don't need to build it in the library.

JelleCeulemans commented 1 month ago

@juliemturner @patrick-rodgers

I am now using the following piece of code as a workaround, which is based on the current existing function in pnp:

export async function getAllChildrenAsTree(termSet: ITermSet, includeProperties?: boolean): Promise<IOrderedTermInfo[]> {
  const visitor = async (source: ITerm | ITermSet | undefined, parent: IOrderedTermInfo[]): Promise<void> => {
    if (source) {
      const children: IChildren = source.children;

      if (includeProperties) {
        children.select('createdDateTime', 'id', 'descriptions', 'labels', 'properties', 'lastModifiedDateTime');
      }

      const childTerms: ITermStoreType.Term[] = await children();

      for (const child of childTerms) {
        const orderedTerm: Partial<IOrderedTermInfo> = {
          children: [] as any,
          defaultLabel: child.labels?.find((l) => l.isDefault)?.name ?? undefined,
          ...child
        };

        parent.push(orderedTerm as IOrderedTermInfo);

        await visitor(child.id ? termSet.getTermById(child.id) : undefined, orderedTerm.children as IOrderedTermInfo[]);
      }
    }
  };

  const tree: IOrderedTermInfo[] = [];

  await visitor(termSet, tree);

  return tree;
}
bcameron1231 commented 1 month ago

This looks like a solid approach and makes sense to me. If you'd like to submit a PR that would be awesome, otherwise we'll get to it when we find some time :)

JelleCeulemans commented 1 month ago

@bcameron1231 Thank you for the feedback! I'd be glad to submit a PR, but I ran into some issues setting up the PnPjs solution on my first attempt. I'll give it another try and keep you posted.

github-actions[bot] commented 2 weeks ago

This issue is locked for inactivity or age. If you have a related issue please open a new issue and reference this one. Closed issues are not tracked.