-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add operate log #2626
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
feat: add operate log #2626
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/test-infra repository. |
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here. DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
| getList() | ||
| }) | ||
| </script> | ||
| <style lang="scss" scoped> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The code looks good overall, but there are a few areas that could be optimized or improved:
-
Code Duplication: The filter logic is repeated for both
menuandoperate. Consider refactoring this into a separate function to make it cleaner. -
Empty Check in Filter Change: Ensure that when
valis'clear', the checkbox array is properly cleared before resetting the list of options. -
API Call Optimization: If fetching menu data from
/api/operate-log/getMenuListis expensive or frequently needed, consider caching the results locally. -
Loading State Handling: It might be beneficial to introduce more granular loading states based on different API requests.
Here's an updated version of the code with some optimizations:
<template>
<!-- Table component remains mostly unchanged -->
</template>
<script setup lang="ts">
import { ref, onMounted, reactive } from 'vue'
import getOperateLog from '@/api/operate-log'
import DetailDialog from './component/DetailDialog.vue'
import { t } from '@/locales'
import { beforeDay, datetimeFormat, nowDate } from '@/utils/time'
const popoverVisible = ref(false)
const operateTypeArr = ref<any[]>([])
const DetailDialogRef = ref()
const loading = ref(false)
// Other variables remain mostly unchanged
onMounted(() => {
getMenuList()
changeDayHandle(history_day.value)
getList()
})
async function fetchData(params: any): Promise<void> {
try {
const response = await getOperateLog.getOperateLog(paginationConfig, params, loading);
tableData.value = response.data.records;
paginationConfig.total = response.data.total;
} catch (error) {
// Handle error appropriately
}
}
function filterChange(val: string) {
if (val === 'clear') {
operateTypeArr.value = [];
} else {
const filteredParams: Record<string, any> = {};
if (filter_type.value === 'status') {
filteredParams['status'] = filter_status.value;
}
if (operateTypeArr.value.length > 0 && filter_type.value === 'menu') {
filteredParams['menu'] = JSON.stringify(operateTypeArr.value);
}
fetchData(filteredParams);
}
popoverVisible.value = false;
}
// ...Explanation of Changes:
-
Moved Fetch Logic to Separate Function: Created
fetchDatato encapsulate the API call logic, making it reusable and clean. -
Updated Filter Functionality: The filter function no longer needs checks for each property separately; they all use the same pattern now.
This should improve maintainability and reduce redundancy while keeping the app responsive and efficient.
| getOperateLog | ||
| getOperateLog, | ||
| getMenuList | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The provided code looks clean and well-structured. However, there is one minor suggestion to make it more concise:
@@ -22,7 +22,6 @@ const getOperateLog: (
export default {
- getOperateLog,
+ ...getOperateLog,
} This will import ...getOperateLog from wherever it is defined, effectively including all properties of the getOperateLog function object into the exported module. This can be useful if you want to avoid listing each property individually.
For other checks, such as type checking or security considerations, I would need additional context about how these functions are used and what kind of operations they perform with their parameters. If the endpoint requires specific types or handles sensitive data properly, ensure that this is accounted for in your environment. Additionally, consider adding input validation before making API calls to prevent common errors like missing or incorrect parameters.
feat: add operate log