Fix commands where we're unable to set options to empty values because of wrong conditions

[martinlingstuyl]

For certain commands, setting an option to an empty value will not take care of resetting a value in MS365. This is caused by badly written conditions.

take the following example from spo list set

private mapRequestBody(options: Options): any {
    const requestBody: any = {};

    if (options.newTitle) {
      requestBody.Title = options.newTitle;
    }

    if (options.description) {
      requestBody.Description = options.description;
    }

   //...
}

This function builds a request object to post to SharePoint. The problem is that using an empty string for description will cause the runtime to skip the if-statement.

That's because an empty string is considered a falsy value in JavaScript, when used in a condition. So if options.description is an empty string, if (options.description) { } will be evaluated to false.

The right check in this case should be an explicit undefined check:

if (options.description !== undefined) { }

or

if (typeof options.description !== "undefined") { }

Originally posted by @martinlingstuyl in https://github.com/pnp/cli-microsoft365/issues/3718#issuecomment-1260830957

Commands

This impacts primarily set commands, where a user would set something to empty. The situation occurs as far as I can see in the following commands (but there might be more):

• [ ] spo list set
• [ ] spo customaction set
• [ ] teams channel set
• [ ] teams team set
• [ ] todo task set
• [ ] aad o365group set
• [ ] aad user set
• [ ] graph schemaextension set
• [ ] spo web set
• [ ] spo site set
• [ ] spo sitedesign set
• [ ] spo sitescript set
• [ ] planner plan set

[Jwaegebaert]

We should be careful with this change and reflect on each value if they are allowed to pass an empty string. Some values won't allow this and will cause a schema validation error to be thrown. e.g. for the command planner task set, when you want to change the title to an empty string you'll get the following.

This is a healthy error to have in the API but the error thrown could be confusing for the users. To counter this, we could also include additional validation checks to validate these occurences. That way we can implement the change to the methods mapRequestBody that you suggested and be sure that the proper validation error is thrown for the non-empty values.

[martinlingstuyl]

Absolutely agreed on that @Jwaegebaert, maybe we should pick this up command by command. I'll change this to an epic.

[waldekmastykarz]

Absolutely agreed on that @Jwaegebaert, maybe we should pick this up command by command. I'll change this to an epic.

Additionally, let's create a separate issue for each command to modify and let's spec which options allow empty values and should be changed.