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

Added internalName option for spo field commands #6379

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ktskumar
Copy link
Contributor

Closes #6249

In this PR, i have added internalName option to below commands

  • spo field get
  • spo field set
  • spo field remove

@milanholemans
Copy link
Contributor

Thank you, we'll try to review it ASAP!

@MathijsVerbeeck MathijsVerbeeck self-assigned this Nov 13, 2024
src/m365/spo/commands/field/field-get.spec.ts Outdated Show resolved Hide resolved
src/m365/spo/commands/field/field-get.spec.ts Outdated Show resolved Hide resolved
src/m365/spo/commands/field/field-get.spec.ts Outdated Show resolved Hide resolved
src/m365/spo/commands/field/field-get.ts Outdated Show resolved Hide resolved
src/m365/spo/commands/field/field-remove.spec.ts Outdated Show resolved Hide resolved
src/m365/spo/commands/field/field-set.spec.ts Outdated Show resolved Hide resolved
@MathijsVerbeeck MathijsVerbeeck marked this pull request as draft November 29, 2024 15:22
@ktskumar ktskumar marked this pull request as ready for review December 21, 2024 04:43
@martinlingstuyl martinlingstuyl self-assigned this Feb 1, 2025
Copy link
Contributor

@martinlingstuyl martinlingstuyl left a comment

Choose a reason for hiding this comment

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

Some last comments

@@ -191,7 +196,8 @@ class SpoFieldRemoveCommand extends SpoCommand {
}
else {
try {
await removeField(listRestUrl, args.options.id, args.options.title);
const columnName: string | undefined = args.options.title ? args.options.title : args.options.internalName;
Copy link
Contributor

Choose a reason for hiding this comment

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

The removeField function now has a title parameter, which can contain the title or the internalName, that does not make sense. Let's just forward the entire options object into the removeField function

@@ -158,7 +163,7 @@ class SpoFieldSetCommand extends SpoCommand {
// retrieve column CSOM object id
const fieldQuery: string = args.options.id ?
`<Method Id="663" ParentId="7" Name="GetById"><Parameters><Parameter Type="Guid">${formatting.escapeXml(args.options.id)}</Parameter></Parameters></Method>` :
`<Method Id="663" ParentId="7" Name="GetByInternalNameOrTitle"><Parameters><Parameter Type="String">${formatting.escapeXml(args.options.name || args.options.title)}</Parameter></Parameters></Method>`;
`<Method Id="663" ParentId="7" Name="GetByInternalNameOrTitle"><Parameters><Parameter Type="String">${formatting.escapeXml(args.options.name || args.options.title || args.options.internalName)}</Parameter></Parameters></Method>`;
Copy link
Contributor

Choose a reason for hiding this comment

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

There's an odd args.options.name here, thats not actually part of the options, lets remove it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add internalName option for spo field commands
4 participants