New command: spo folder unarchive. Closes #7182 - #7405
Saurabh7019 wants to merge 3 commits into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The prompt-confirmed execution path lacks coverage, preventing the required 100% branch coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds the spo folder unarchive command for reactivating SharePoint folders by URL or ID.
Changes:
- Implements validation, confirmation, and SharePoint REST calls.
- Adds unit tests and documentation.
- Registers the command and sidebar entry.
File summaries
| File | Description |
|---|---|
folder-unarchive.ts |
Implements the command. |
folder-unarchive.spec.ts |
Tests validation and execution. |
commands.ts |
Registers the command name. |
sidebars.ts |
Adds documentation navigation. |
folder-unarchive.mdx |
Documents usage and permissions. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
milanholemans
left a comment
milanholemans
left a comment
There was a problem hiding this comment.
Looks almost good to go. Noticed a few things we should take a look at before shipping this.
| let loggerLogSpy: sinon.SinonSpy; | ||
|
|
||
| const unarchiveResponse = { | ||
| value: '{"IsArchive":false,"TotalFileCount":2,"CreatedUtcDateTime":"2026-06-17T12:32:15.3479567Z","LastStartedUtcDateTime":"0001-01-01T00:00:00","FolderArchiveStatus":"Unknown","ProcessedFileCount":0,"SuccessCount":0,"FailureCount":0,"NotArchivableFileCount":0,"ProgressPercentage":0.0}' |
There was a problem hiding this comment.
Is this response really a JSON string instead of just a JSON object?
There was a problem hiding this comment.
yes, the response is a JSON string, not an object.
| if (!folderInfo.Exists) { | ||
| throw `The folder '${url || id}' does not exist.`; | ||
| } |
There was a problem hiding this comment.
Does this Exists parameter have any use? If a folder doesn't exist you just receive a 404, right?
There was a problem hiding this comment.
yes, it is needed. both GetFolderByServerRelativePath and GetFolderById return 200 with {"Exists": false} when a folder doesn't exist.
Response:
{
"url": "https://contoso.sharepoint.com/sites/M365SOBA/_api/web/GetFolderByServerRelativePath(DecodedUrl='%2Fsites%2FM365SOBA%2FShared%20Documents%2FGeneral%2FStream123')?$select=Exists,ListItemAllFields/Id,ListItemAllFields/ParentList/Id&$expand=ListItemAllFields,ListItemAllFields/ParentList",
"status": 200,
"statusText": "OK",
"headers": {
},
"data": {
"Exists": false
}
}
| it('throws an error when the folder does not exist by url', async () => { | ||
| sinon.stub(request, 'get').resolves({}); | ||
|
|
||
| await assert.rejects(command.action(logger, { | ||
| options: { | ||
| webUrl: 'https://contoso.sharepoint.com/sites/test', | ||
| url: '/Shared Documents/temp1', | ||
| force: true | ||
| } | ||
| }), new CommandError(`The folder '/Shared Documents/temp1' does not exist.`)); | ||
| }); |
There was a problem hiding this comment.
If it doesn't exist, you get a 404, right?
There was a problem hiding this comment.
No, it returns 200.
| : The server- or site-relative decoded URL of the folder to unarchive. Specify either `url` or `id`, but not both. | ||
|
|
||
| `-i, --id [id]` | ||
| : The UniqueId (GUID) of the folder to unarchive. Specify either `url` or `id`, but not both. |
There was a problem hiding this comment.
UniqueId is not really a word.
| : The UniqueId (GUID) of the folder to unarchive. Specify either `url` or `id`, but not both. | |
| : The unique ID (GUID) of the folder to unarchive. Specify either `url` or `id`, but not both. |
Noticed this word is also used in other unarchive/archive commands. Would be appreciated if you fix it there as well.
There was a problem hiding this comment.
this is fixed in archive/unarchive commands.. but the same problem in several other commands
should we create a separate issue to track and fix the remaining cases?
8347597 to
0d94371
Compare
0d94371 to
b7fb142
Compare
Closes #7182