Adds hostProperties option to applicationcustomizer commands. Closes …#7084
Adds hostProperties option to applicationcustomizer commands. Closes …#7084Saurabh7019 wants to merge 1 commit intopnp:mainfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds the hostProperties option to all four application customizer commands (add and set variants for both site-level and tenant-level), enabling users to configure host properties for Application Customizers. This addresses issue #6825.
Key Changes
- Added
hostPropertiesoptional parameter to all four application customizer commands - Implemented JSON validation for
hostPropertiesinapplicationcustomizer-add.ts - Added test coverage for the new option across all commands
- Updated documentation to reflect the new parameter and its JSON handling requirements
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/m365/spo/commands/tenant/tenant-applicationcustomizer-add.ts | Adds hostProperties option, telemetry tracking, and sets field value when adding tenant-wide customizers |
| src/m365/spo/commands/tenant/tenant-applicationcustomizer-add.spec.ts | Adds test case verifying hostProperties can be set when adding a tenant-wide customizer |
| src/m365/spo/commands/tenant/tenant-applicationcustomizer-set.ts | Adds hostProperties option, telemetry tracking, validation check, and update logic for tenant-wide customizers |
| src/m365/spo/commands/tenant/tenant-applicationcustomizer-set.spec.ts | Adds test case verifying hostProperties can be updated for existing tenant-wide customizers |
| src/m365/spo/commands/applicationcustomizer/applicationcustomizer-add.ts | Adds hostProperties option with JSON validation, telemetry tracking, and sets field value when adding site-level customizers |
| src/m365/spo/commands/applicationcustomizer/applicationcustomizer-add.spec.ts | Adds test cases for both successful hostProperties usage and validation of invalid JSON input |
| src/m365/spo/commands/applicationcustomizer/applicationcustomizer-set.ts | Adds hostProperties option, telemetry tracking, validation check, and update logic for site-level customizers |
| src/m365/spo/commands/applicationcustomizer/applicationcustomizer-set.spec.ts | Adds test case verifying hostProperties can be updated for existing site-level customizers |
| docs/docs/cmd/spo/tenant/tenant-applicationcustomizer-add.mdx | Documents the new hostProperties option and updates JSON escaping guidance to include it |
| docs/docs/cmd/spo/tenant/tenant-applicationcustomizer-set.mdx | Documents the new hostProperties option with clear/empty string capability and updates remarks for JSON handling |
| docs/docs/cmd/spo/applicationcustomizer/applicationcustomizer-add.mdx | Documents the new hostProperties option and updates JSON escaping guidance to include it |
| docs/docs/cmd/spo/applicationcustomizer/applicationcustomizer-set.mdx | Documents the new hostProperties option with clear/empty string capability and updates remarks for JSON handling |
|
Thanks @Saurabh7019! We'll try to review it soon. |
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Nice improvement!
Made a few comments while reviewing.
| if (args.options.hostProperties) { | ||
| requestBody.HostProperties = args.options.hostProperties; | ||
| } |
There was a problem hiding this comment.
Let's assign this value directly to the request body. Also, by default these host properties are just an empty string. Maybe we should do this as well.
| && opts.data['Location'] === 'ClientSideExtension.ApplicationCustomizer' | ||
| && opts.data['ClientSideComponentId'] === clientSideComponentId | ||
| && opts.data['HostProperties'] === clientSideComponentProperties | ||
| && opts.data['Name'] === title) { |
There was a problem hiding this comment.
Instead of checking the body like this, let's remove it from here and use assert.deepstrictequal to validate the body.
This way if there is a mismatch, it will be much clearer what the problem is.
| if (!args.options.newTitle && args.options.description === undefined && !args.options.clientSideComponentProperties && args.options.hostProperties === undefined) { | ||
| return `Please specify an option to be updated`; | ||
| } |
There was a problem hiding this comment.
Over here, we're not validating if it's a valid JSON like we do for the add command.
| if (hostProperties !== undefined) { | ||
| requestBody.HostProperties = hostProperties; | ||
| } |
There was a problem hiding this comment.
Can't we assign this directly to the request body object?
| { | ||
| option: '--hostProperties [hostProperties]' | ||
| }, |
There was a problem hiding this comment.
Could we add some JSON validation for hostProperties? Let's do the same for clientSideComponentProperties.
| }); | ||
|
|
||
| await command.action(logger, { options: { clientSideComponentId: clientSideComponentId, title: customizerTitle, hostProperties: '{ "preAllocatedApplicationCustomizerTopHeight": "50", "preAllocatedApplicationCustomizerBottomHeight": "50" }', verbose: true } }); | ||
| assert.strictEqual(executeCommandCalled, true); |
There was a problem hiding this comment.
Could we validate that the right host properties were passed?
| option: '-p, --clientSideComponentProperties [clientSideComponentProperties]' | ||
| }, | ||
| { | ||
| option: '--hostProperties [hostProperties]' |
There was a problem hiding this comment.
Let's also validate if it's a valid JSON string.
|
|
||
| await command.action(logger, { | ||
| options: { | ||
| id: id, hostProperties: hostProperties |
There was a problem hiding this comment.
Let's put the hostProperties on a new line here.
Closes #6825