Skip to content

fix: Fix the problem that jumping to a directory that does not exist … #8353

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

Merged
merged 1 commit into from
Apr 8, 2025

Conversation

ssongliu
Copy link
Member

@ssongliu ssongliu commented Apr 8, 2025

No description provided.

Copy link

f2c-ci-robot bot commented Apr 8, 2025

Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

return
}
helper.SuccessWithData(c, true)
helper.SuccessWithData(c, false)
}

// @Tags File
Copy link
Member

Choose a reason for hiding this comment

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

The code appears to be designed for checking whether a file exists at the specified path or creating it if necessary. However, there are a few areas that could be improved:

  1. Simplify Error Handling: The error handling logic is somewhat repetitive. Instead of repeating helper.SuccessWithData(c, false) multiple times inside different branches of the conditional statements, consider consolidating those into a single call after detecting an error.

    if err := helper.CheckBindAndValidate(&req, c); err != nil {
        return
    }
    
    ...

2. **Use Consistent Function Calls**: Ensure consistent function calls across the codebase. For example, use the same method name (`Stat`), parameter list, and context parameters for operations like checking existence and directory creation.

   ```go
   fileOp := files.NewFileOp()
   
   ...
  1. Return Value Check: After attempting to perform operations, check the result before returning success or failure directly. This helps avoid unexpected behavior due to asynchronous operations or incomplete setup.
if !fileOp.Stat(req.Path) {
    // Handle specific cases where file does not exist
} else if err := fileOp.CreateDir(req.Path, 0644); err != nil {
    helper.SuccessWithData(c, false)
    // Additional logs or retry can be added here
}
  1. Logging for Debugging: Consider adding logging within error paths to help debug issues more easily. You can use Go's standard library log package or third-party libraries like zap.
import "log"

func (b *BaseApi) CheckFile(c *gin.Context) {
    // Existing code
}

...

log.Fatalf("Failed to create directory %s: %v", req.Path, err)

...
  1. Comments for Complex Logic: Add comments around complex sections of code that might not be immediately clear to others reading the code.

By making these improvements, the code will be clearer, maintainable, and less prone to errors.

router.push({ path: '/hosts/files', query: { path: backupPath.value } });
} else {
MsgError(i18n.global.t('file.noSuchFile'));
}
};

const submitImport = async (formEl: FormInstance | undefined) => {
Copy link
Member

Choose a reason for hiding this comment

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

  1. The function toFolder now includes an asynchronous check before navigating to the files page. It calls the CheckFile API to verify if the file exists at the specified path. If the file does not exist, it displays a message error.

        const res = await CheckFile(backupPath.value, true);
        if (res.data) {
            router.push({ path: '/hosts/files', query: { path: backupPath.value } });
        } else {
            MsgError(i18n.global.t('file.noSuchFile'));
        }
  2. In the submitImport function, you should handle errors properly when using promises returned by the Axios request (snapshotImport). Use .catch() to catch the rejected promise and display appropriate messages if something goes wrong during the upload process. Additionally, ensure that all variables used within the .then() block are correctly scoped.

            snapshotImport(params).then(() => {
                // Handle success, e.g., hide dialog, clear fields, notify user
                drawerVisible.value = false;
                formRef.value?.resetFields();
            }).catch(error => {
                console.error("Upload failed:", error); // Log error for debugging purposes
                MsgError(i18n.global.t('upload.fail')); // Display error message
            });

These changes improve robustness of your application, ensuring better user experience in certain situations where there may be issues with the data being processed.

export const CheckFile = (path: string) => {
return http.post<boolean>('files/check', { path: path });
export const CheckFile = (path: string, withInit: boolean) => {
return http.post<boolean>('files/check', { path: path, withInit: withInit });
};

export const BatchCheckFiles = (paths: string[]) => {
Copy link
Member

Choose a reason for hiding this comment

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

The provided code contains a minor issue in the CheckFile function:

  1. Parameter Change: The CheckFile function now takes an additional parameter withInit. This appears to be intended as part of the request data being sent to the server, but there are no changes needed in the logic.

  2. Return Type: The response from both functions is expected to be either a boolean indicating success or failure (boolean). No other types should typically be returned unless they have specific meanings based on context.

Overall, the main difference is simply adding another attribute to one of the function parameters and not altering the return type or logic significantly.

Copy link

sonarqubecloud bot commented Apr 8, 2025

Copy link
Member

@wanghe-fit2cloud wanghe-fit2cloud left a comment

Choose a reason for hiding this comment

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

/lgtm

@wanghe-fit2cloud
Copy link
Member

/approve

Copy link

f2c-ci-robot bot commented Apr 8, 2025

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: wanghe-fit2cloud

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@f2c-ci-robot f2c-ci-robot bot added the approved label Apr 8, 2025
@f2c-ci-robot f2c-ci-robot bot merged commit 74eb2c8 into dev Apr 8, 2025
6 checks passed
@f2c-ci-robot f2c-ci-robot bot deleted the pr@dev@fix_file_jump branch April 8, 2025 14:33
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.

3 participants