-
Notifications
You must be signed in to change notification settings - Fork 326
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
New Command pp website webfile get #6424
base: main
Are you sure you want to change the base?
Conversation
Hi @ktskumar thank you for creating yet another PR, we're truly grateful for that. |
@ktskumar I added the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @ktskumar, I went through your PR. Let's get some things fixed before we merge this.
} | ||
|
||
class PpWebSiteWebFileGetCommand extends PowerPlatformCommand { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove this superfluous white line
); | ||
} | ||
|
||
#initValidators(): void { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we setup Zod here as well? Just like you already did on website weblink list
? Do let me know!
); | ||
} | ||
|
||
private async getWebSiteId(dynamicsApiUrl: string, args: CommandArgs): Promise<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Website is a single word. Let's cjamge the casing to reflect that:
private async getWebSiteId(dynamicsApiUrl: string, args: CommandArgs): Promise<any> { | |
private async getWebsiteId(dynamicsApiUrl: string, args: CommandArgs): Promise<any> { |
} | ||
); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's move private functions to below the commandAction function....
if (args.options.websiteId) { | ||
return args.options.websiteId; | ||
} | ||
const options: PpWebSiteOptions = { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I believe this options object is not really used. Let's remove it.
responseType: 'json' | ||
}; | ||
|
||
if (options.name) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets use args.options.websiteName... But also let's remove this if-statement.
We know for certain that args.options.websiteName should be filled at this point. Otherwise the validator would throw.
We can use args.options.websiteName!
to reflect that certainty where we need to use the parameter.
} | ||
} | ||
|
||
private async getWebSiteWebFile(dynamicsApiUrl: string, websiteId: string, options: Options): Promise<any> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
private async getWebSiteWebFile(dynamicsApiUrl: string, websiteId: string, options: Options): Promise<any> { | |
private async getWebFile(dynamicsApiUrl: string, websiteId: string, options: Options): Promise<any> { |
Also, Let's use a type instead of any
.
responseType: 'json' | ||
}; | ||
|
||
// let webfileitem: any | null = null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove these comments.
|
||
|
||
|
||
// requestOptions.url = `${dynamicsApiUrl}/api/data/v9.1/websitewebfiles?$filter=name eq '${options.name}'`; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's remove unnecessary white lines and comments.
Closes #6273
Added new command
pp website webfile get