Bug report: Cannot get term by name if it is a child of another term

[martinlingstuyl]

Description

If I try to get a term by name, it will not find it if the term is a child or grandchild in a hierarchy:

The following will throw an error:

m365 spo term get --termSetName "some-set" --termGroupName "some-group" --name "some-childterm-name"

Error: Specified argument was out of the range of valid values.
Parameter name: index

However, the term does exist and it can be retrieved by id.
The same operation with a top level term will succeed.

Steps to reproduce

Create a taxonomy hierarchy and try to retrieve the top-level and child-level terms by name.

Expected results

It finds the term

Actual results

It does not

Diagnostics

No response

CLI for Microsoft 365 version

6.1 (next)

nodejs version

16

Operating system (environment)

Windows

Shell

PowerShell

cli doctor

No response

Additional Info

No response

[MathijsVerbeeck]

I'd like to have a look at this.

[milanholemans]

Great to hear! All yours!

[MathijsVerbeeck]

@martinlingstuyl @milanholemans

I've managed to find the solution for the issue. I have however one more question. Currently, when we don't find a term either by id or by name, we just use return;. Shouldn't it be more useful that we return something like
The specific term '${args.options.id || args.options.name}' could not be found.

[MathijsVerbeeck]

Another question: What is multiple terms get found? Should we log them all?

[milanholemans]

Good questions @MathijsVerbeeck, in my opinion:

• We should indeed log an error if a term was not found
• We should log an error if multiple terms with the same name were found. In that case people should define an ID instead of a name.

Correct me if I'm wrong @pnp/cli-for-microsoft-365-maintainers

[Adam-it]

Good questions @MathijsVerbeeck, in my opinion:

• We should indeed log an error if a term was not found
• We should log an error if multiple terms with the same name were found. In that case people should define an ID instead of a name.

Nothing to correct 😉. I agree with the approach 👍.
Thanks @MathijsVerbeeck for your awesome work 🤩

[martinlingstuyl]

Hi @MathijsVerbeeck,

Awesome! I'm curious to see how you solved it!

I agree with you that we should show a message when no term is found. Feel free to add that in this PR.

Also: when multiple terms are found, a disambiguation prompt should be shown, where the user may specify what term he meant.

We use disambiguation prompts in multiple places in the source code, you can check one of those for inspiration. The most helpful scenario would be that you see the id and the term path of the term, so the user can decide easiest what term he meant. He may then copy the id and reuse the term get command with the id option.

[MathijsVerbeeck]

I agree with you that we should show a message when no term is found. Feel free to add that in this PR.

Quite easily. Currently, we were just simply using the CSOM operation termset.Terms.GetByName(), however, I implemented it now using LabelMatchOperation and then using termset.GetTerms(lmi) but then in TypeScript with the annoying XML as request body 😄

I agree with you that we should show a message when no term is found. Feel free to add that in this PR.

This has been done in the PR.

Also: when multiple terms are found, a disambiguation prompt should be shown, where the user may specify what term he meant.

I will implement this after the first review. I believe that there will be some comments 😄

[MathijsVerbeeck]

Also: when multiple terms are found, a disambiguation prompt should be shown, where the user may specify what term he meant.

We use disambiguation prompts in multiple places in the source code, you can check one of those for inspiration. The most helpful scenario would be that you see the id and the term path of the term, so the user can decide easiest what term he meant. He may then copy the id and reuse the term get command with the id option.

I just had a look at this, but I'm not sure what we should show to the user. I could show Id (logically), but then I'm uncertain what to show exactly. Do you have any suggestions? Implementing with only Id would look like this:

A possibility could be the pathOfTerm. I think that this makes the most sense, however, this property currently does not exist on the Term interface, so has to be added.

[martinlingstuyl]

Hi @MathijsVerbeeck, if the data for the term path is available, that would be what you want to see, combined with the id, so you can select that and execute the command again.

If that means updating the interface, then that's okay.

[MathijsVerbeeck]

@martinlingstuyl I've changed the output when multiple terms were found. It will now look like this:

[martinlingstuyl]

Very nice @MathijsVerbeeck!