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

Add the spo list sitescript get command #758

Closed
wants to merge 1 commit into from
Closed

Add the spo list sitescript get command #758

wants to merge 1 commit into from

Conversation

ypcode
Copy link
Contributor

@ypcode ypcode commented Jan 11, 2019

Issue #713
Here is the spo list sitescript get command using the server side API on SPO.
In order to be consistent with latest other commands, I used --listId and --listTitle instead of --id and --title, if it is an issue, I can change it back, Hope you'll like it :)

@coveralls
Copy link

coveralls commented Jan 11, 2019

Coverage Status

Coverage remained the same at 100.0% when pulling 5cb696c on ypcode:dev-getsitescriptfromlist into 2b4f005 on pnp:dev.

@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 11, 2019

Awesome! I'll have a look asap. Thank you! 👏

@waldekmastykarz waldekmastykarz self-assigned this Jan 11, 2019
@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 12, 2019

Is there a reason you mimicked CSOM instead of using the REST API?

POST https://contoso.sharepoint.com/_api/Microsoft_SharePoint_Utilities_WebTemplateExtensions_SiteScriptUtility_GetSiteScriptFromList
accept: application/json;odata=nometadata
content-type: application/json;odata=nometadata
authorization: Bearer eyJ0eXAiOiJKV1...

{
  "listUrl": "https://contoso.sharepoint.com/Lists/MyList/AllItems.aspx"
}

@ypcode
Copy link
Contributor Author

ypcode commented Jan 12, 2019

@ypcode
Copy link
Contributor Author

ypcode commented Jan 12, 2019

Just ask and BOOM it is now using the REST API instead :)

@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 13, 2019

I've found it in the only true source of REST API documentation: /_api/$metadata 😉 . Thanks for the quick change!

Copy link
Member

@waldekmastykarz waldekmastykarz left a comment

Unfortunately the command fails when you're connected to the tenant admin. Would you mind adjusting the auth piece and few other things I spotted?

docs/manual/docs/cmd/spo/list/list-sitescript-get.md Outdated Show resolved Hide resolved
src/o365/spo/commands/list/list-sitescript-get.ts Outdated Show resolved Hide resolved
src/o365/spo/commands/list/list-sitescript-get.ts Outdated Show resolved Hide resolved
src/o365/spo/commands/list/list-sitescript-get.ts Outdated Show resolved Hide resolved
src/o365/spo/commands/list/list-sitescript-get.ts Outdated Show resolved Hide resolved
src/o365/spo/commands/list/list-sitescript-get.ts Outdated Show resolved Hide resolved
@ypcode
Copy link
Contributor Author

ypcode commented Jan 14, 2019

I've found it in the only true source of REST API documentation: /_api/$metadata 😉 . Thanks for the quick change!

Just out of curiosity, when I issue a simple GET on /_api/$metadata on my tenant, nothing refers to _api/Microsoft_SharePoint_Utilities_WebTemplateExtensions_SiteScriptUtility_GetSiteScriptFromList.

Do you use any special call or anything to find it ?

@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 14, 2019

You need to search for GetSiteScriptFromList. Then you will find a function import Microsoft_SharePoint_Utilities_WebTemplateExtensions_SiteScriptUtility_GetSiteScriptFromList which means that Microsoft_SharePoint_Utilities_WebTemplateExtensions_SiteScriptUtility_GetSiteScriptFromList can be called directly on /_api/. It's pretty cryptic, but you can find useful information once you understand how the XML file is structured.

@ypcode
Copy link
Contributor Author

ypcode commented Jan 19, 2019

Hi @waldekmastykarz , on my side, this one is ready to be reviewed, I see the work in progress, just to make sure you saw I resolved the change requests :)

@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 23, 2019

Thanks for confirming @ypcode!

Copy link
Member

@waldekmastykarz waldekmastykarz left a comment

Nicely done with a few fixes I've done while merging the PR


const siteScript: string | null = res.value;
if (!siteScript) {
cb(new CommandError(`An error occured, the site script could not be extracted from list '${args.options.listId || args.options.listTitle}'`));
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

Typo in occured: should be occurred.


Examples:

Extract a Site Script from an existing SharePoint List with title ${chalk.grey('CustomList')}
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

List name in the description (CustomList), doesn't match the list name in the command (ContosoList)

* Utils.getAbsoluteUrl("https://contoso.sharepoint.com/sites/team1/", "/sites/team1/Lists/MyList");
*/
public static getAbsoluteUrl(webUrl: string, serverRelativeUrl: string) : string {
const tenantUrl: string = `${url.parse(webUrl).protocol}//${url.parse(webUrl).hostname}`;
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

We shouldn't be parsing the URL twice

*/
public static getAbsoluteUrl(webUrl: string, serverRelativeUrl: string) : string {
const tenantUrl: string = `${url.parse(webUrl).protocol}//${url.parse(webUrl).hostname}`;
if (serverRelativeUrl[0] != '/') {
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

We should be using a strict comparison here

@@ -0,0 +1,44 @@
# spo list sitescript get

Extracts a site script from the SharePoint list
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

Differs from the help in command


## Remarks

To extract a site script from a list, you have to first log in to a SharePoint Online site using the [spo login](../login.md) command, eg. `spo login https://contoso.sharepoint.com`.
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

Differs from the help in command


## Examples

Extract a Site Script from an existing SharePoint List with title _CustomList_
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

This line shouldn't be indented

## Examples

Extract a Site Script from an existing SharePoint List with title _CustomList_
located in site _https://contoso.sharepoint.com/sites/project-x_
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

No need for a line break here


## Examples

Extract a Site Script from an existing SharePoint List with title _CustomList_
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

List name in the description (CustomList) doesn't match the list name in the command (ContosoList)

Extract a Site Script from an existing SharePoint List with title _CustomList_
located in site _https://contoso.sharepoint.com/sites/project-x_

```sh
Copy link
Member

@waldekmastykarz waldekmastykarz Jan 26, 2019

Choose a reason for hiding this comment

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

No need to indent the sample

@waldekmastykarz
Copy link
Member

waldekmastykarz commented Jan 26, 2019

Merged manually. Thank you 👏

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

Successfully merging this pull request may close these issues.

None yet

3 participants