Skip to content

Refactor spo file rename to use util instead of calling other command, Closes #5262#5263

Draft
nicodecleyre wants to merge 3 commits intopnp:mainfrom
nicodecleyre:Refactor-spo-file-rename
Draft

Refactor spo file rename to use util instead of calling other command, Closes #5262#5263
nicodecleyre wants to merge 3 commits intopnp:mainfrom
nicodecleyre:Refactor-spo-file-rename

Conversation

@nicodecleyre
Copy link
Contributor

Closes #5262

@Adam-it
Copy link
Member

Adam-it commented Jul 18, 2023

Thanks for another awesome PR 💪👍. You are on 🔥 today. We will review it ASAP

@Adam-it Adam-it self-assigned this Aug 20, 2023
@nicodecleyre nicodecleyre force-pushed the Refactor-spo-file-rename branch from 1ab2d80 to f8f99cd Compare September 2, 2023 15:58
@Adam-it Adam-it marked this pull request as draft September 17, 2023 22:42
@Adam-it
Copy link
Member

Adam-it commented Sep 17, 2023

setting to draft due to the following observations
I will provide more guidance how we should proceed with this PR in near future

@Adam-it
Copy link
Member

Adam-it commented Apr 3, 2025

@nicodecleyre how about we re-take this PR as well?

@Adam-it Adam-it reopened this Apr 3, 2025
Copy link
Member

@Adam-it Adam-it left a comment

Choose a reason for hiding this comment

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

@nicodecleyre This PR is pretty straight forward and it is not possible to combine it with any other change. The biggest issue it has is the duplicated logic which we discussed in the parent epic issue. If we solve that then we may merge this one as well.
Let me know if you are still willing to take on this action?

* @param logger The logger object
* @param verbose Set for verbose logging
*/
async removeFile(webUrl: string, url: string, recycle?: boolean, logger?: Logger, verbose?: boolean): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

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

@nicodecleyre the main issue of this change is that this util method introduced duplicated logic/code that is present also here

const serverRelativePath = urlUtil.getServerRelativePath(args.options.webUrl, args.options.url!);
requestUrl = `${args.options.webUrl}/_api/web/GetFileByServerRelativePath(DecodedUrl='${formatting.encodeQueryParameter(serverRelativePath)}')`;
}
if (args.options.recycle) {
requestUrl += `/recycle()`;
}
const requestOptions: CliRequestOptions = {
url: requestUrl,
method: 'POST',
headers: {
'X-HTTP-Method': 'DELETE',
'If-Match': '*',
'accept': 'application/json;odata=nometadata'
},
responseType: 'json'
};

In order to introduce less logic we need to maintain we should also reuse this util method in the file-remove.ts as well before we proceed

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor spo file rename to use util instead of calling other command

2 participants