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

Adding functionality for deleted sites in site-list solving #1335 #1355

Closed
wants to merge 1 commit into from
Closed

Adding functionality for deleted sites in site-list solving #1335 #1355

wants to merge 1 commit into from

Conversation

joakimhogberg
Copy link
Contributor

Solves #1335. Included in PR:

  • Changed implementation to be able to use --deleted
  • Updated test case to cover new branch in implementation
  • Updated documentation with examples on how to get deleted sites
  • Updated PowerShell comparsion for Get-SPODeletedSite

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 17382e2 on joakimhogberg:feature/issue-1335 into f6a983c on pnp:dev.

@garrytrinder
Copy link
Member

Thank you @joakimhogberg 👏🏻

We will review shortly.

@garrytrinder garrytrinder self-assigned this Feb 15, 2020
@garrytrinder garrytrinder self-requested a review February 15, 2020 22:51
@@ -15,6 +15,7 @@ Option|Description
`--help`|output usage information
`--type [type]`|type of modern sites to list. Allowed values `TeamSite|CommunicationSite`, default `TeamSite`
`-f, --filter [filter]`|filter to apply when retrieving sites
`--deleted [deleted]`|use this switch to only return deleted sites
Copy link
Member

Choose a reason for hiding this comment

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

As we are not passing anything into this option we can just use --deleted and remove [deleted]

return telemetryProps;
}

public commandAction(cmd: CommandInstance, args: CommandArgs, cb: (err?: any) => void): void {
const siteType: string = args.options.type || 'TeamSite';
const webTemplate: string = siteType === 'TeamSite' ? 'GROUP#0' : 'SITEPAGEPUBLISHING#0';
let startIndex: string = '0';
let spoAdminUrl: string
let spoAdminUrl: string
Copy link
Member

Choose a reason for hiding this comment

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

Missing ; at the end of this line

@@ -109,6 +116,10 @@ class SpoSiteListCommand extends SpoCommand {
{
option: '-f, --filter [filter]',
description: 'filter to apply when retrieving sites'
},
{
option: '--deleted [deleted]',
Copy link
Member

Choose a reason for hiding this comment

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

As we are not passing anything into this option we can just use --deleted and remove [deleted]

@@ -32,14 +33,15 @@ class SpoSiteListCommand extends SpoCommand {
const telemetryProps: any = super.getTelemetryProperties(args);
telemetryProps.siteType = args.options.type || 'TeamSite';
telemetryProps.filter = (!(!args.options.filter)).toString();
telemetryProps.deleted = args.options.deleted
Copy link
Member

Choose a reason for hiding this comment

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

Missing ; at the end of this line

Copy link
Member

@garrytrinder garrytrinder left a comment

Choose a reason for hiding this comment

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

Great work @joakimhogberg 👏🏻

Just a few minor changes that I made before merging, please see review comments.

The branch to be merged with the dev branch is here: https://github.com/garrytrinder/office365-cli/tree/feature/issue-1335

@waldekmastykarz
Copy link
Member

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

4 participants