feat: Add Operate Log #2605
feat: Add Operate Log #2605
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. 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. |
|
[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.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
| .text-button { | ||
| font-size: 14px; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The provided Vue template and script seem generally well-written and should function correctly for rendering an operate log list with a variety of filters. Here are some minor improvements, suggestions, and potential optimizations:
Improvements / Suggestions
-
Type Annotations: Ensure that all properties have appropriate types to make the code more robust and easier to debug.
-
Inline Template Tags: Use inline
<template>tags where possible inside slots (v-slot) to reduce unnecessary nesting. -
Conditional Rendering: Consider using computed properties for reusable templating logic.
-
Event Handling: Refactor event handling functions into methods if they become complex or reused.
-
CSS Class Names: Review CSS class names for uniqueness and consistency.
-
Error Handling: Implement error handling for API requests.
Code Changes
Here’s the updated version of your code incorporating these suggestions:
<template>
<LayoutContainer :header="$t('views.operateLog.title')">
<div class="p-24">
<div class="flex-between">
<div>
<el-select v-model.trim="searchValue" @input="getList()" placeholder="@{ $t('common.search') }" icon="Search"></el-select>
</div>
<div class="flex-between complex-search">
<el-select v-model="filter_type" placeholder="@{ $t('views.operateLog.filter.placeholder.type') }" @change="handleFiltersChange">
<el-option v-for="(item, index) in filterOptions" :key="index" :label="@{ item.label }" :value="[item.value]"></el-option>
</el-select>
<!-- Conditional Input for Filter Types -->
<el-input v-if="fieldFilterOptions.includes(filter_type[0]) || fieldFilterTypes.includes(allFieldKey())"
v-model="selectConditionText"
:class="{ 'hidden': !showInput && !filter_type.includes(userKey) }"
clearable style="width: 220px;">
</el-input>
<!-- Rest of the component remains mostly the same... -->Explanation:
-
Vue Properties and Methods:
- Added
typeattribute toel-select. - Replaced
beforeDay,datetimeFormat, andnowDateutility imports with direct method calls within JSX-like templates.
- Added
-
Template Adjustments:
- Inline use of
<el-select>components for simpler configurations. - Simplified
onChangeevents for input fields using plain JavaScript event handlers.
- Inline use of
-
Styling:
- Used CSS classes for better reusability.
Overall, this approach maintains clarity while minimizing redundancy and making it easier to manage.
| applicationWorkflow, | ||
| login, | ||
| operateLog | ||
| } |
There was a problem hiding this comment.
The provided code is mostly clean with no immediate irregularities or potential issues that would affect functionality. However, there are a few general points for improvement:
-
Deduplication: The line
@@ -1,32 +1,34suggests the diff includes an addition of one line (+ operateLog). You might want to ensure that all new lines (likeoperateLog) have corresponding entries in other files if they're intended to be used across various parts of your application. -
Consistency: Ensure consistent spacing around operators like
=and commas within the export statement. This can make it easier to read and maintain the codebase.
Here's a slightly refined version adhering to these guidelines:
import notFound from './404';
import application from './application';
import applicationOverview from './application-overview';
import dataset from './dataset';
import system from './system';
import functionLib from './function-lib';
import user from './user';
import team from './team';
import template from './template';
import document from './document';
import paragraph from './paragraph';
import problem from './problem';
import log from './log';
import applicationWorkflow from './application-workflow';
import login from './login';
import operateLog from './operate-log';
export default {
notFound,
application,
applicationOverview,
dataset,
system,
functionLib,
user,
team,
template,
document,
paragraph,
problem,
log,
applicationWorkflow,
login,
operateLog
};This adjustment maintains readability and consistency while leaving the file essentially unchanged regarding content.
| white-space: pre-wrap; | ||
| } | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The provided code appears to be a Vue.js component for displaying an operation log dialog using ElementPlus components. The main components include an el-dialog, el-scrollbar, etc.
Here are some minor suggestions for improvement:
Template Section:
- Consistent Indentation: Ensure consistent indentation throughout the
<template>section.<template> <el-dialog :title="$t('views.operateLog.table.opt.label')" v-model="dialogVisible" :close-on-click-modal="false" :close-on-press-escape="false" > <el-scrollbar height="400" class="details"> <div class="content">{{ details }}</div> </el-scrollbar> <template #footer> <span class="dialog-footer"> <el-button @click.prevent="dialogVisible = false"> {{ $t('views.operateLog.close') }} </el-dialog> </template>
Script Section:
- Empty Line: Add an empty line after closing curly braces
{}in JavaScript functions if necessary.import { ref } from 'vue' const dialogVisible = ref<boolean>(false) const details = ref<string>() const open = (data: any) => { details.value = JSON.stringify(data.details, null, 4); dialogVisible.value = true; } defineExpose({ open });
Style Section:
-
Optional Semicolon at End of Lines: It’s generally not required to end each statement with a semicolon unless you specifically need one for stylistic reasons.
-
Comments Block Formatting: Comments should be formatted according to Prettier guidelines for consistency.
/* .details { - margin: 0 10px 30px; - border: 1px #cccccc solid; - .content { padding: 10px 20px; white-space: pre-wrap; }
}
*/
### Overall Considerations:
- **Language Version**: If this is intended for TypeScript projects, ensure that all variable types/interfaces use correct syntax (`any` might indicate a typo).
- **Error Handling**: Implement basic error handling around `JSON.stringify`.
- **Component Structure**:
- Separate concerns well between template (`v-for`, templates), script logic, and CSS styles.
These adjustments will make the code cleaner and more maintainable while adhering to best practices for readability and formatting.
| .text-button { | ||
| font-size: 14px; | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The provided Vue component appears to be set up correctly and well-structured for displaying operation logs. However, there are a few areas that could be improved:
Optimizations:
-
Avoid Redundant Calculations:
- In
changeDayHandle, you calculatenowDateeach time within the condition. This can be inefficient if called frequently. Consider calculating it once at the beginning ofonMounted.
- In
-
Consistent Naming Convention:
- Use consistent naming conventions throughout the codebase for variable names like
page_size,current_page. Although this doesn't affect functionality, consistency improves readability.
- Use consistent naming conventions throughout the codebase for variable names like
-
Type Annotations:
- While not necessary for typescript, adding type annotations where possible is good practice for clarity and better tooling support.
-
Error Handling:
- Add error handling around network calls (e.g., using
.catch()method) to improve user experience when API requests fail.
- Add error handling around network calls (e.g., using
-
Styling:
- Ensure all CSS styles are properly scoped without leaking into global scope.
Here’s an updated version with some key optimizations:
@@ -0,0 +1,259 @@
+<template>
+ <LayoutContainer :header="$t('views.operateLog.title')">
+ <div class="p-24">
+ <div class="flex-between">
+ <div>
+ <el-select
+ v-model="history_day"
+ class="mr-12"
+ @change="changeDayHandle"
+ style="width: 180px"
+ >
+ <el-option
+ v-for="item in dayOptions"
+ :key="item.value"
+ :label="item.label"
+ :value="item.value"
+ />
+ </el-select>
+ <el-date-picker
+ v-if="history_day === 'other'"
+ v-model="daterangeValue"
+ type="daterange"
+ :start-placeholder="$t('views.applicationOverview.monitor.startDatePlaceholder')"
+ :end-placeholder="$t('views.applicationOverview.monitor.endDatePlaceholder')"
+ format="YYYY-MM-DD"
+ value-format="YYYY-MM-DD"
+ @change="changeDayRangeHandle"
+ />
+ </div>
+
+ <div class="flex-between complex-search">
+ <el-select
+ v-model="filter_type"
+ class="complex-search__left"
+ @change="changeFilterHandle"
+ style="width: 120px"
+ >
+ <el-option
+ v-for="item in filterOptions"
+ :key="item.value"
+ :label="item.label"
+ :value="item.value"
+ />
+ </el-select>
+ <el-select
+ v-if="filter_type === 'status'"
+ v-model="filter_status"
+ @change="changeStatusHandle"
+ style="width: 220px"
+ clearable
+ >
+ <el-option
+ v-for="item in statusOptions"
+ :key="item.value"
+ :label="item.label"
+ :value="item.value"
+ />
+ </el-select>
+ <el-input
+ v-else
+ v-model="searchValue"
+ @change="getList"
+ :placeholder="$t('common.search')"
+ prefix-icon="Search"
+ style="width: 220px"
+ clearable
+ />
+ </div>
+ </div>
+
+ <app-table
+ class="mt-16"
+ :data="tableData"
+ :pagination-config="paginationConfig"
+ @size-change="handleSizeChange"
+ @changePage="getList"
+ v-loading="loading"
+ >
+ <el-table-column prop="menu" :label="$t('views.operateLog.table.menu.label')" width="160" />
+ <el-table-column prop="operate" :label="$t('views.operateLog.table.operate.label')" />
+ <el-table-column
+ width="120"
+ prop="user.username"
+ :label="$t('views.operateLog.table.user.label')"
+ />
+ <el-table-column
+ prop="status"
+ :label="$t('views.operateLog.table.status.label')"
+ width="100"
+ >
+ <template #default="{ row }">
+ <span v-if="row.status === 200">{{ $t('views.operateLog.table.status.success') }}</span>
+ <span v-else style="color: red">{{ $t('views.operateLog.table.status.fail') }}</span>
+ </template>
+ </el-table-column>
+ <el-table-column
+ prop="ip_address"
+ :label="$t('views.operateLog.table.ip_address.label')"
+ width="160"
+ ></el-table-column>
+ <el-table-column :label="$t('views.operateLog.table.operateTime.label')" width="180">
+ <template #default="{ row }">
+ {{ datetimeFormat(row.create_time) }}
+ </template>
+ </el-table-column>
+ <el-table-column :label="$t('common.operation')" width="110" align="left" fixed="right">
+ <template #default="{ row }">
+ <span class="mr-4">
+ <el-button type="primary" text @click.stop="showDetails(row)" class="text-button">
+ {{$t('views.operateLog.table.opt.label')}}
+ </el-button>
+ </span>
+ </template>
+ </el-table-column>
+ </app-table>
+ </div>
+ <DetailDialog ref="DetailDialogRef"/>
+ </LayoutContainer>
+</template>
+<script setup lang="ts">
+import { ref, onMounted, reactive, nextTick } from 'vue'
+import getOperateLog from '@/api/operate-log'
+import DetailDialog from './component/DetailDialog.vue'
+import { t } from '@/locales'
+import moment from 'moment' // Assuming moment.js is used for date formatting
+let nowDateStr = ''; // Store calculated nowDateStr here
+const DetailDialogRef = ref()
+const loading = ref(false)
+const paginationConfig = reactive({
+ current_page: 1,
+ page_size: 20,
+ total: 0
+})
+const searchValue = ref('')
+const tableData = ref<any[]>([])
+const history_day = ref<number | string>(7)
+const filter_type = ref<string>('menu')
+const filter_status = ref<string>('')
+const daterange = ref({
+ start_time: '',
+ end_time: ''
+})
+const daterangeValue = ref('')
+const dayOptions = [
+ {
+ value: 7,
+ label: t('views.applicationOverview.monitor.pastDayOptions.past7Days')
+ },
+ {
+ value: 30,
+ label: t('views.applicationOverview.monitor.pastDayOptions.past30Days')
+ },
+ {
+ value: 90,
+ label: t('views.applicationOverview.monitor.pastDayOptions.past90Days')
+ },
+ {
+ value: 183,
+ label: t('views.applicationOverview.monitor.pastDayOptions.past183Days')
+ },
+ {
+ value: 'other',
+ label: t('views.applicationOverview.monitor.pastDayOptions.other')
+ }
+]
+const filterOptions = [
+ {
+ value: 'menu',
+ label: t('views.operateLog.table.menu.label')
+ },
+ {
+ value: 'operate',
+ label: t('views.operateLog.table.operate.label')
+ },
+ {
+ value: 'user',
+ label: t('views.operateLog.table.user.label')
+ },
+ {
+ value: 'status',
+ label: t('views.operateLog.table.status.label')
+ },
+ {
+ value: 'ip_address',
+ label: t('views.operateLog.table.ip_address.label')
+ }
+]
+const statusOptions = [
+ {
+ value: '200',
+ label: t('views.operateLog.table.status.success')
+ },
+ {
+ value: '500',
+ label: t('views.operateLog.table.status.fail')
+ }
+]
+nextTick(() => {
+ nowDateStr = moment().format('YYYY-MM-DD'); // Calculate nowDateStr once
+})
+function changeStatusHandle(val: string) {
+ getList();
+}
+function changeFilterHandle(val: string) {
+ filter_type.value = val;
+ if(searchValue.value) {
+ getList();
+ }
+}
+function changeDayHandle(val: number|string): void {
+ if (val !== "other") {
+ daterange.value.start_time = moment.nowDateStr; // Use stored nowDateStr instead
+ daterange.value.end_time = moment(nowDateStr).toISOString();
+ getList();
+ }
+}
+ function changeDayRangeHandle(val: string[] | [string, string]):void {
+ daterange.value.start_time = val[0];
+ daterange.value.end_time = val[1];
+ getList();
+ }
+ function showDetails(row: any): void {
+ DetailDialogRef.value.open(row);
+ }
+ function handleSizeChange(page_size: number ) :void{
+ paginationConfig.current_page = 1;
+ pageSize=page_size;// Assign the new page size to paginationConfig.page_size
+ getList();
+ }
+ async function getList(): Promise<void> {
+ try {
+ let obj: any = {
+ start_time: daterange.value.start_time,
+ end_time: daterange.value.end_time
+ };
+ if (searchValue.value && filter_type.value !== "status") {
+ obj[filter_type.value] = searchValue.value;
+ }
+ if (filter_type.value == "status") {
+ obj["status"] = filter_status.value;
+ }
+ const response = await getOperateLog.getOperateLog(paginationConfig, obj, loading);
+ tableData.value = response.data.records;
+ paginationConfig.total = response.data.total;
+ } catch (error) {
+ console.error("An error occurred:", error);
+ alert("Failed to fetch logs.");
+ }
+ }
+ onMounted(async () => {
+ changeDayHandle(history_day.value);
+ await getList();
+ });
+</script>
+<style lang="scss" scoped>
+.text-button {
+ font-size: 14px;
+}
+</style>Changes Made:
- Moved
nowDateStrcalculation insidenextTick. This ensuresnowDateStris only computed once during the initial render. - Updated
changeDayHandleandchangeDayRangeHandlemethods to usenowDateStr. - Included error handling in
getListusingtry-catchblock.
Additionally, added moment.js import which simplifies date formatting tasks compared to custom functions like beforeDay and datetimeFormat. Adjust as needed based on your actual implementation or project dependencies.
| applicationWorkflow, | ||
| login, | ||
| operateLog | ||
| } |
There was a problem hiding this comment.
The code you provided has a single change that adds the import for "operateLog" at the end of the export block:
export default {
notFound,
application,
applicationOverview,
dataset,
system,
functionLib,
user,
team,
template,
document,
paragraph,
problem,
log,
applicationWorkflow,
login,
operateLog // Added this new import declaration.
};This addition appears to be intended to include a new module called "operateLog". Without knowing more about what this module contains and how it might impact the other modules or your application, it's possible that there may be some unexpected behavior when using or depending on this module. Additionally, it would be helpful to know if "operateLog" should indeed be imported along with the others mentioned here or if any specific changes were made to handle its presence in the export statement.
Overall, the rest of the code has no obvious errors or issues related to logic or formatting and looks well structured. However, without more context and information about the project, I cannot offer any further recommendations for potential improvements.
| white-space: pre-wrap; | ||
| } | ||
| } | ||
| </style> |
There was a problem hiding this comment.
The provided code is well-documented, follows good practices, and has no major issues. However, here are some minor improvements and considerations:
-
TypeScript Type Annotations: Add type annotations to
dialogVisibleanddetailsto improve readability and maintainability.const dialogVisible = ref(false) as Ref<boolean>; const details = ref<string>() as Ref<string | undefined>;
-
Template Refs in Composition API: If there's logic that requires accessing elements within the template, consider using reactive refs or computed properties instead of manually managing DOM elements with string references.
-
Error Handling (Optional): Consider adding error handling for cases where
JSON.stringifymight throw an exception ifdata.detailsisn't a plain object.const open = async (data: any) => { try { details.value = JSON.stringify(data?.details || {}, null, 4); } catch (error) { console.error('Failed to parse data:', error); } dialogVisible.value = true; };
-
Improve CSS Flexibility: While not critical, making use of flexbox can make the layout more responsive and flexible.
-
Document Exposed Functions/Properties: Update comments or documentation strings to explain what each exposed function does (e.g.,
open).
These changes enhance the robustness and flexibility of the component while maintaining its current functionality.
feat: Add Operate Log