fix: Fix some snapshot anomaly issues #8926
Conversation
|
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. DetailsInstructions 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-sigs/prow repository. |
| i++ | ||
| } | ||
| return data, nil | ||
| } |
There was a problem hiding this comment.
There is an issue in the LoadSnapshotData function where the labels "app" and "system_snapshot" are removed from the list, but there's no corresponding logic to replace these with valid options if they exist. The loadPanelFile function has issues related to handling different types of items ('clamav', 'download', 'resource') differently compared to others ('apps', 'backup', 'log', 'db', 'tmp'). Additionally, the loadFile function does not handle directories correctly since it doesn't add them as children nodes when appropriate. Finally, the use of variable names can be improved for clarity.
| if !req.WithLoginLog && global.IsMaster { | ||
| err = snap.snapCoreDB.Exec("DELETE FROM login_logs").Error | ||
| snap.Task.LogWithStatus(i18n.GetMsgByKey("SnapDeleteLoginLog"), err) | ||
| if err != nil { |
There was a problem hiding this comment.
There are no significant issues with the provided code snippet. The changes made to handle deletion of operation_logs and login_logs when requested depend on whether they are enabled (WithOperationLog) and if it's a master server (global.IsMaster). This structure enhances code clarity and ensures that the necessary operations are performed only under specific conditions. However, since these operations involve potentially deleting sensitive data, careful consideration should be given to how this logic is implemented in a production environment. If error handling can be moved elsewhere more efficiently without repeating, further optimizations may be beneficial, but overall, the current implementation seems well-structured.
| <el-button type="primary" v-if="nowIndex === 4" :disabled="loading" @click="submitAddSnapshot"> | ||
| {{ $t('commons.button.create') }} | ||
| </el-button> | ||
| <el-button @click="next" v-else>{{ $t('commons.button.next') }}</el-button> |
There was a problem hiding this comment.
In the provided code snippet, there is an issue with the button label that appears after the fourth step of the process (index 4). The button is intended to show "Create Snapshot", but it incorrectly has type="primary" applied when you are on the last step instead. This can be confusing for users.
To fix this problem and ensure consistency across all steps, you should change v-if="nowIndex === 4" to use :disabled instead, so that the button remains enabled throughout all steps except the final one where only the "Next" button would appear:
<el-button type="primary" v-if="nowIndex < 3" @click="submitAddSnapshot"> This modification ensures that the "Create Snapshot" button is disabled on the fifth step (nowIndex === 4), maintaining proper functionality and user interface.
Additionally, it's a good practice to avoid having conditional classes if they can be handled using conditional rendering directly in Vue components, rather than trying to force them into different types based on conditions.
|
|
/approve |
|
[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 DetailsNeeds approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Refs #8917
Refs #8924
Refs #8897