Looks like this PR is not ready to merge, because of the following issues:

• This PR is targeting the wrong base branch. It should target 7.9.0, but it targets 7.8.0

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

🦋 Changeset detected

Latest commit: 8e5fc1d

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

​

All committers have signed the CLA.

✅ Test Results

$ yarn testapi -f 'Webdav'                          


  [Webdav]
    /webdav.getMyAccounts
      ✔ should return my webdav accounts
    /webdav.removeWebdavAccount
      ✔ should return an error when send an invalid request (62ms)
      ✔ should return an error when using an invalid account id


  3 passing (945ms)

Kody Review Complete

Great news! 🎉
No issues were found that match your current review configurations.

Keep up the excellent work! 🚀

​

Status

1. fix: update bodyParams to body in POSTRemoveWebdavAccount validation
2. refactor: extract common error schemas for unauthorized and bad request responses
3. fix: enforce additionalProperties: false in error schemas
4. fix: update user account properties in WebDAV response

✅ Test Results

$ yarn testapi -f 'Webdav'                          
  [Webdav]
    /webdav.getMyAccounts
      ✔ should return my webdav accounts
    /webdav.removeWebdavAccount
      ✔ should return an error when send an invalid request
      ✔ should return an error when using an invalid account id


  3 passing (576ms)

GitHub Copilot Review

Your code is well-structured and adheres to good practices for API design and TypeScript usage. However, here are some suggestions for improvement:

1. Error Handling

• Current Issue: The webdav.removeWebdavAccount endpoint does not handle cases where the accountId does not exist or the removal fails.
• Suggestion: Add explicit error handling for cases where the account is not found or the deletion fails.

if (!removed) {
	return API.v1.failure({
		error: 'Account not found or could not be removed.',
	});
}

2. Code Duplication

• Current Issue: The schemas for POSTRemoveWebdavAccount and the body validation are duplicated.
• Suggestion: Reuse the POSTRemoveWebdavAccountSchema for the body validation to avoid redundancy.

body: ajv.compile(POSTRemoveWebdavAccountSchema),

3. Response Consistency

• Current Issue: The webdav.getMyAccounts endpoint returns accounts with specific fields, but there is no explicit check to ensure sensitive fields (e.g., passwords) are excluded.
• Suggestion: Add a comment or explicit filtering to ensure only safe fields are returned.

const accountsForResponse = userAccounts.map(({ userId, serverURL, username, name }) => ({
	userId,
	serverURL,
	username,
	name,
}));

4. Logging

• Current Issue: There is no logging for critical operations like account removal.
• Suggestion: Add logging for debugging and monitoring purposes.

console.log(`User ${this.userId} removed WebDAV account ${accountId}`);

5. Security

• Current Issue: The accountId is directly passed to the removeByUserAndId method without additional validation.
• Suggestion: Ensure the accountId is sanitized or validated to prevent injection attacks.

6. Performance

• Current Issue: The findWebdavAccountsByUserId function retrieves all accounts for a user. If the dataset grows, this could become a performance bottleneck.
• Suggestion: Add pagination or limit the number of accounts returned.

7. Documentation

• Current Issue: While the schemas are well-defined, there is no inline documentation for the endpoints.
• Suggestion: Add JSDoc comments for each endpoint to improve readability and maintainability.

/**
 * GET /webdav.getMyAccounts
 * Retrieves all WebDAV accounts for the authenticated user.
 */

8. Type Safety

• Current Issue: The removed variable is not strongly typed.
• Suggestion: Ensure the return type of WebdavAccounts.removeByUserAndId is explicitly defined.

const removed: { acknowledged: boolean; deletedCount: number } | null = await WebdavAccounts.removeByUserAndId(accountId, this.userId);

9. Common Schema Extraction

• Current Issue: The commonUnauthorizedErrorSchema and commonBadRequestErrorSchema are defined inline.
• Suggestion: Extract these schemas into a shared module for reuse across multiple files.

10. Testing

• Current Issue: There is no mention of tests for these endpoints.
• Suggestion: Write unit and integration tests to verify the behavior of the endpoints under various scenarios (e.g., valid/invalid inputs, edge cases).

Example Refactor for webdav.removeWebdavAccount

.post(
	'webdav.removeWebdavAccount',
	{
		authRequired: true,
		validateParams: isPOSTRemoveWebdavAccount,
		body: ajv.compile(POSTRemoveWebdavAccountSchema),
		response: {
			200: ajv.compile({
				type: 'object',
				properties: {
					result: {
						type: 'object',
						properties: {
							acknowledged: { type: 'boolean' },
							deletedCount: { type: 'integer' },
						},
						required: ['acknowledged', 'deletedCount'],
						additionalProperties: false,
					},
					success: { type: 'boolean' },
				},
				required: ['result', 'success'],
				additionalProperties: false,
			}),
			400: ajv.compile(commonBadRequestErrorSchema),
			401: ajv.compile(commonUnauthorizedErrorSchema),
		},
	},
	async function () {
		const { accountId } = this.bodyParams;

		const removed = await WebdavAccounts.removeByUserAndId(accountId, this.userId);
		if (!removed) {
			return API.v1.failure({
				error: 'Account not found or could not be removed.',
			});
		}

		void api.broadcast('notify.webdav', this.userId, {
			type: 'removed',
			account: { _id: accountId },
		});

		return API.v1.success({ result: removed });
	},
);

Final Thoughts

Your code is already in good shape, but implementing these suggestions will improve maintainability, security, and performance. Let me know if you'd like help with specific refactoring or testing!

Code Review Completed! 🔥

The code review was successfully completed based on your current configurations.

​

Summary

1. refactor: remove AJV instance and import one from the core-typings
2. fix: update userId to _id in webdav account response structure
3. fix: add description to accountId in POSTRemoveWebdavAccountSchema

✅ Test Results

$ yarn testapi -f 'Webdav'                          
  [Webdav]
    /webdav.getMyAccounts
      ✔ should return my webdav accounts (42ms)
    /webdav.removeWebdavAccount
      ✔ should return an error when send an invalid request
      ✔ should return an error when using an invalid account id


  3 passing (625ms)

Please run the CI. I've fixed the previous errors in the Code Check / Code Lint.