Skip to content

Updates navigation node commands navigation/menuState API. Closes #7617 - #7625

Merged
milanholemans merged 5 commits into
pnp:mainfrom
Adam-it:7617-updates-all-navigation-node-commands
Oct 18, 2026
Merged

milanholemans merged 5 commits into
pnp:mainfrom
Adam-it:7617-updates-all-navigation-node-commands

Conversation

@Adam-it

@Adam-it Adam-it commented Oct 7, 2026

Copy link
Copy Markdown
Member

Closes #7617

@Adam-it Adam-it changed the title Updates navigation node commands navigation/menuState API. Closes #7617 WIP: Updates navigation node commands navigation/menuState API. Closes #7617 Oct 7, 2026
@Adam-it Adam-it added the pr-major PR for the next major release label Oct 7, 2026
@Adam-it Adam-it changed the title WIP: Updates navigation node commands navigation/menuState API. Closes #7617 Updates navigation node commands navigation/menuState API. Closes #7617 Oct 10, 2026
@Adam-it
Adam-it marked this pull request as ready for review October 10, 2026 23:31
@Adam-it Adam-it added the pr-priority Process this PR asap label Oct 10, 2026

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

The remove command remains on the old API, and set-command interactive validation lacks required option-set metadata.

2 open findings
What changed in this PR

Migrates SharePoint navigation node add, get, and set commands to the MenuState API and aligns their schemas, output, tests, and documentation.

Changes:

  • Adds shared recursive MenuState node lookup.
  • Migrates commands to MenuState and Zod validation.
  • Updates tests and documentation for the new API shape.
File Description
src/​utils/​spo.ts Adds MenuState node lookup helpers.
src/​utils/​spo.spec.ts Tests recursive node lookup.
NavigationNode.ts Updates MenuState node types.
navigation-node-add.ts Migrates node creation to MenuState.
navigation-node-add.spec.ts Tests new add behavior.
navigation-node-get.ts Retrieves nodes through MenuState.
navigation-node-get.spec.ts Tests location-specific retrieval.
navigation-node-set.ts Updates nodes through MenuState.
navigation-node-set.spec.ts Tests updated properties and validation.
navigation-node-add.mdx Documents revised options and output.
navigation-node-get.mdx Documents MenuState output.
navigation-node-set.mdx Documents location and revised updates.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/utils/spo.ts Resolved

@milanholemans milanholemans left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks almost good to go, got one remark, also Copilot found a valid remark.

Comment on lines +70 to 76
if (title !== undefined) {
node.Title = title;
}

const requestBody: any = {
Title: args.options.title,
IsExternal: args.options.isExternal,
Url: url
};

if (args.options.audienceIds !== undefined) {
requestBody.AudienceIds = args.options.audienceIds === '' ? [] : args.options.audienceIds.split(',');
if (url !== undefined) {
node.SimpleUrl = url;
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of putting it into conditions, can't we just assign it directly to the object? If the option is undefined, it will not be included in the request body anyway.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe no. What we do is retrieve the full node from getMenuStateNodeByKey, then update what changed ans send it back using saveMenuState. If we set it to undefined and send this back I think it will clear the URL value of the menu option

@milanholemans milanholemans Oct 18, 2026 •

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I was mistaken; I thought it was the add command.

@milanholemans
milanholemans marked this pull request as draft October 16, 2026 22:00
@Adam-it
Adam-it marked this pull request as ready for review October 18, 2026 00:50
@Adam-it

Adam-it commented Oct 18, 2026

Copy link
Copy Markdown
Member Author

@milanholemans can you recheck

@milanholemans
milanholemans merged commit dbf388d into pnp:main Oct 18, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-major PR for the next major release pr-merged pr-priority Process this PR asap

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Align all navigation node commands to use _api/navigation/ManuState

3 participants