-
-
Notifications
You must be signed in to change notification settings - Fork 10.2k
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
Beautify Portal TraceLog UI interface #5149
Beautify Portal TraceLog UI interface #5149
Conversation
WalkthroughThe recent changes in the Apollo Portal involve enhancing the audit log trace detail UI by integrating a Changes
Possibly related issues
Tip New Features and ImprovementsReview SettingsIntroduced new personality profiles for code reviews. Users can now select between "Chill" and "Assertive" review tones to tailor feedback styles according to their preferences. The "Assertive" profile posts more comments and nitpicks the code more aggressively, while the "Chill" profile is more relaxed and posts fewer comments. AST-based InstructionsCodeRabbit offers customizing reviews based on the Abstract Syntax Tree (AST) pattern matching. Read more about AST-based instructions in the documentation. Community-driven AST-based RulesWe are kicking off a community-driven initiative to create and share AST-based rules. Users can now contribute their AST-based rules to detect security vulnerabilities, code smells, and anti-patterns. Please see the ast-grep-essentials repository for more information. New Static Analysis ToolsWe are continually expanding our support for static analysis tools. We have added support for Tone SettingsUsers can now customize CodeRabbit to review code in the style of their favorite characters or personalities. Here are some of our favorite examples:
Revamped Settings PageWe have redesigned the settings page for a more intuitive layout, enabling users to find and adjust settings quickly. This change was long overdue; it not only improves the user experience but also allows our development team to add more settings in the future with ease. Going forward, the changes to Miscellaneous
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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.
Actionable comments posted: 1
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apollo-portal/src/main/resources/static/audit_log_trace_detail.html (4 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/AuditLogTraceDetailController.js (3 hunks)
- apollo-portal/src/main/resources/static/styles/audit-log.css (2 hunks)
Additional Context Used
Biome (20)
apollo-portal/src/main/resources/static/scripts/controller/AuditLogTraceDetailController.js (20)
58-61: This function expression can be turned into an arrow function.
51-64: This function expression can be turned into an arrow function.
47-65: This function expression can be turned into an arrow function.
70-74: Prefer for...of instead of forEach.
78-92: Prefer for...of instead of forEach.
102-105: Prefer for...of instead of forEach.
108-108: Avoid the delete operator which can impact performance.
136-149: This function expression can be turned into an arrow function.
158-170: This function expression can be turned into an arrow function.
172-179: Prefer for...of instead of forEach.
174-176: Template literals are preferred over string concatenation.
172-179: This function expression can be turned into an arrow function.
181-197: Prefer for...of instead of forEach.
183-197: This function expression can be turned into an arrow function.
22-22: Use let or const instead of var.
32-32: Use let or const instead of var.
69-70: Use let or const instead of var.
77-77: Use let or const instead of var.
95-96: Use let or const instead of var.
96-101: Use let or const instead of var.
Additional comments not posted (8)
apollo-portal/src/main/resources/static/styles/audit-log.css (5)
142-145
: The new styles for.treeview
look good and are consistent with the PR's objective to enhance UI appearance.
147-151
: The transition effect added to.node-treeview
enhances user experience by providing visual feedback on hover. Good addition!
153-155
: Removing the border from the last child of.node-treeview
is a thoughtful detail that improves the visual cleanliness of the UI.
157-159
: The hover effect on.node-treeview
is consistent with modern UI practices, providing immediate visual feedback to the user.
115-115
: Ensure theborder-radius
property change aligns with the design specifications.apollo-portal/src/main/resources/static/scripts/controller/AuditLogTraceDetailController.js (2)
19-26
: Initialization of new data structures for handling treeview data is clear and well-organized.
45-62
: The implementation ofbuildTraceDetailsTree
function is robust, ensuring that treeview data is correctly built and displayed.apollo-portal/src/main/resources/static/audit_log_trace_detail.html (1)
Line range hint
59-81
: The integration of the treeview component into the HTML structure is well-executed, enhancing the user interface as intended by the PR. Ensure that all new scripts and styles are correctly linked and loaded.Also applies to: 145-204
Verification successful
The verification script has successfully identified the inclusion of various scripts and stylesheets in the
audit_log_trace_detail.html
file. Notably, thebootstrap-treeview.min.js
script, which is essential for the treeview component, is included.
bootstrap-treeview.min.js
is present, ensuring the functionality of the treeview component.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all new scripts and styles are correctly linked and loaded. # Test: Check the network tab in browser dev tools for any 404 errors on the new resources. echo "Please check the browser's network tab for any loading issues with new scripts and styles."Length of output: 193
Script:
#!/bin/bash # Description: Search for <script> and <link> tags in the audit_log_trace_detail.html file to verify inclusion of new resources. fd -e html -x grep -E '<script|<link' {}Length of output: 64608
function buildTree(data) { | ||
// 构建 spanId 到节点的映射 | ||
var nodeMap = new Map(); | ||
data.forEach(item => { | ||
nodeMap.set(item.logDTO.spanId, item); | ||
}); | ||
|
||
// 构建图的根节点列表 | ||
var roots = []; | ||
|
||
data.forEach(item => { | ||
const log = item.logDTO; | ||
const parentSpanId = log.parentSpanId; | ||
|
||
if (parentSpanId && nodeMap.has(parentSpanId)) { | ||
const parent = nodeMap.get(parentSpanId); | ||
if (!parent.children) { | ||
parent.children = []; | ||
} | ||
parent.children.push(item); | ||
} else { | ||
roots.push(item); | ||
} | ||
}); | ||
|
||
// 递归生成 Treeview 格式的节点 | ||
function buildTreeNode(node) { | ||
var log = node.logDTO; | ||
var treeNode = { | ||
text: log.opName, | ||
nodes: [], | ||
metaDetail: node | ||
}; | ||
if (node.children) { | ||
node.children.forEach(child => { | ||
treeNode.nodes.push(buildTreeNode(child)); | ||
}); | ||
} | ||
if (treeNode.nodes.length === 0) { | ||
delete treeNode.nodes; | ||
} | ||
return treeNode; | ||
} | ||
|
||
}); | ||
return roots.map(root => buildTreeNode(root)); | ||
} |
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 buildTree
function is well-implemented, with clear logic for constructing the tree nodes. Consider using for...of
instead of forEach
for better performance.
- data.forEach(item => {
+ for (const item of data) {
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
function buildTree(data) { | |
// 构建 spanId 到节点的映射 | |
var nodeMap = new Map(); | |
data.forEach(item => { | |
nodeMap.set(item.logDTO.spanId, item); | |
}); | |
// 构建图的根节点列表 | |
var roots = []; | |
data.forEach(item => { | |
const log = item.logDTO; | |
const parentSpanId = log.parentSpanId; | |
if (parentSpanId && nodeMap.has(parentSpanId)) { | |
const parent = nodeMap.get(parentSpanId); | |
if (!parent.children) { | |
parent.children = []; | |
} | |
parent.children.push(item); | |
} else { | |
roots.push(item); | |
} | |
}); | |
// 递归生成 Treeview 格式的节点 | |
function buildTreeNode(node) { | |
var log = node.logDTO; | |
var treeNode = { | |
text: log.opName, | |
nodes: [], | |
metaDetail: node | |
}; | |
if (node.children) { | |
node.children.forEach(child => { | |
treeNode.nodes.push(buildTreeNode(child)); | |
}); | |
} | |
if (treeNode.nodes.length === 0) { | |
delete treeNode.nodes; | |
} | |
return treeNode; | |
} | |
}); | |
return roots.map(root => buildTreeNode(root)); | |
} | |
function buildTree(data) { | |
// 构建 spanId 到节点的映射 | |
var nodeMap = new Map(); | |
for (const item of data) { | |
nodeMap.set(item.logDTO.spanId, item); | |
} | |
// 构建图的根节点列表 | |
var roots = []; | |
for (const item of data) { | |
const log = item.logDTO; | |
const parentSpanId = log.parentSpanId; | |
if (parentSpanId && nodeMap.has(parentSpanId)) { | |
const parent = nodeMap.get(parentSpanId); | |
if (!parent.children) { | |
parent.children = []; | |
} | |
parent.children.push(item); | |
} else { | |
roots.push(item); | |
} | |
} | |
// 递归生成 Treeview 格式的节点 | |
function buildTreeNode(node) { | |
var log = node.logDTO; | |
var treeNode = { | |
text: log.opName, | |
nodes: [], | |
metaDetail: node | |
}; | |
if (node.children) { | |
node.children.forEach(child => { | |
treeNode.nodes.push(buildTreeNode(child)); | |
}); | |
} | |
if (treeNode.nodes.length === 0) { | |
delete treeNode.nodes; | |
} | |
return treeNode; | |
} | |
return roots.map(root => buildTreeNode(root)); | |
} |
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.
Actionable comments posted: 7
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- apollo-portal/src/main/resources/static/audit_log_trace_detail.html (5 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/AuditLogMenuController.js (1 hunks)
- apollo-portal/src/main/resources/static/scripts/controller/AuditLogTraceDetailController.js (1 hunks)
Files not reviewed due to errors (1)
- apollo-portal/src/main/resources/static/scripts/controller/AuditLogTraceDetailController.js (no review received)
Files skipped from review as they are similar to previous changes (1)
- apollo-portal/src/main/resources/static/audit_log_trace_detail.html
$scope.showOptions = function (query) { | ||
$scope.options = []; | ||
searchAuditLogs(query); | ||
}; |
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.
Consider using arrow functions for cleaner and more concise syntax.
- $scope.showOptions = function (query) {
- $scope.options = [];
- searchAuditLogs(query);
- };
+ $scope.showOptions = (query) => {
+ $scope.options = [];
+ searchAuditLogs(query);
+ };
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
$scope.showOptions = function (query) { | |
$scope.options = []; | |
searchAuditLogs(query); | |
}; | |
$scope.showOptions = (query) => { | |
$scope.options = []; | |
searchAuditLogs(query); | |
}; |
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.
😅
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.
@BlackBear2003, glad to see the suggestion got a smile! 😄 If you have any questions or need further assistance, feel free to ask. Happy coding!
AuditLogService.get_properties().then(function (result) { | ||
$scope.auditEnabled = result.enabled; | ||
}); |
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.
Use arrow function for a more concise and modern approach.
- function getAuditProperties() {
- AuditLogService.get_properties().then(function (result) {
- $scope.auditEnabled = result.enabled;
- });
- }
+ const getAuditProperties = () => {
+ AuditLogService.get_properties().then((result) => {
+ $scope.auditEnabled = result.enabled;
+ });
+ };
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
AuditLogService.get_properties().then(function (result) { | |
$scope.auditEnabled = result.enabled; | |
}); | |
const getAuditProperties = () => { | |
AuditLogService.get_properties().then((result) => { | |
$scope.auditEnabled = result.enabled; | |
}); | |
}; |
$window.location.href = AppUtil.prefixPath() + "/audit_log_trace_detail.html?#traceId=" + traceId; | ||
} | ||
function goToTraceDetailsPage(traceId) { | ||
$window.location.href = AppUtil.prefixPath() + "/audit_log_trace_detail.html?#traceId=" + traceId; |
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.
Use template literals for string concatenation to improve readability and maintainability.
- $window.location.href = AppUtil.prefixPath() + "/audit_log_trace_detail.html?#traceId=" + traceId;
+ $window.location.href = `${AppUtil.prefixPath()}/audit_log_trace_detail.html?#traceId=${traceId}`;
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
$window.location.href = AppUtil.prefixPath() + "/audit_log_trace_detail.html?#traceId=" + traceId; | |
$window.location.href = `${AppUtil.prefixPath()}/audit_log_trace_detail.html?#traceId=${traceId}`; |
AuditLogService.find_all_logs($scope.page, PAGE_SIZE).then(function (result) { | ||
if (!result || result.length < PAGE_SIZE) { | ||
$scope.hasLoadAll = true; | ||
} | ||
if (endDate !== null) { | ||
$scope.endDateFmt = new Date(endDate).Format("yyyy-MM-dd hh:mm:ss.S"); | ||
if (result.length === 0) { | ||
return; | ||
} | ||
$scope.auditLogList = []; | ||
$scope.page = 0; | ||
$scope.opName = opName; | ||
$scope.startDate = startDate; | ||
$scope.endDate = endDate; | ||
$scope.auditLogList = $scope.auditLogList.concat(result); | ||
}); |
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.
Refactor to use arrow functions for asynchronous operations.
- function initSearchingMenu() {
- AuditLogService.find_all_logs($scope.page, PAGE_SIZE).then(function (result) {
- if (!result || result.length < PAGE_SIZE) {
- $scope.hasLoadAll = true;
- }
- if (result.length === 0) {
- return;
- }
- $scope.auditLogList = $scope.auditLogList.concat(result);
- });
- }
+ const initSearchingMenu = () => {
+ AuditLogService.find_all_logs($scope.page, PAGE_SIZE).then((result) => {
+ if (!result || result.length < PAGE_SIZE) {
+ $scope.hasLoadAll = true;
+ }
+ if (result.length === 0) {
+ return;
+ }
+ $scope.auditLogList = $scope.auditLogList.concat(result);
+ });
+ };
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
AuditLogService.find_all_logs($scope.page, PAGE_SIZE).then(function (result) { | |
if (!result || result.length < PAGE_SIZE) { | |
$scope.hasLoadAll = true; | |
} | |
if (endDate !== null) { | |
$scope.endDateFmt = new Date(endDate).Format("yyyy-MM-dd hh:mm:ss.S"); | |
if (result.length === 0) { | |
return; | |
} | |
$scope.auditLogList = []; | |
$scope.page = 0; | |
$scope.opName = opName; | |
$scope.startDate = startDate; | |
$scope.endDate = endDate; | |
$scope.auditLogList = $scope.auditLogList.concat(result); | |
}); | |
const initSearchingMenu = () => { | |
AuditLogService.find_all_logs($scope.page, PAGE_SIZE).then((result) => { | |
if (!result || result.length < PAGE_SIZE) { | |
$scope.hasLoadAll = true; | |
} | |
if (result.length === 0) { | |
return; | |
} | |
$scope.auditLogList = $scope.auditLogList.concat(result); | |
}); | |
}; |
$scope.selectOption = function (option) { | ||
$scope.opName = option.opName; | ||
$scope.showSearchDropdown = false; | ||
}; |
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.
Convert this function expression to an arrow function for consistency and improved readability.
- $scope.selectOption = function (option) {
- $scope.opName = option.opName;
- $scope.showSearchDropdown = false;
- };
+ $scope.selectOption = (option) => {
+ $scope.opName = option.opName;
+ $scope.showSearchDropdown = false;
+ };
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
$scope.selectOption = function (option) { | |
$scope.opName = option.opName; | |
$scope.showSearchDropdown = false; | |
}; | |
$scope.selectOption = (option) => { | |
$scope.opName = option.opName; | |
$scope.showSearchDropdown = false; | |
}; |
).then(function (result) { | ||
if (!result || result.length < PAGE_SIZE) { | ||
$scope.hasLoadAll = true; | ||
} | ||
if (result.length === 0) { | ||
return; | ||
} | ||
$scope.auditLogList = $scope.auditLogList.concat(result); | ||
}); |
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.
Convert this function to use arrow syntax for consistency and modern JavaScript practices.
- function searchByOpNameAndDate(opName, startDate, endDate) {
- if (startDate !== null) {
- $scope.startDateFmt = new Date(startDate).Format("yyyy-MM-dd hh:mm:ss.S");
- }
- if (endDate !== null) {
- $scope.endDateFmt = new Date(endDate).Format("yyyy-MM-dd hh:mm:ss.S");
- }
- $scope.auditLogList = [];
- $scope.page = 0;
- $scope.opName = opName;
- $scope.startDate = startDate;
- $scope.endDate = endDate;
- AuditLogService.find_logs_by_opName(
- $scope.opName,
- $scope.startDateFmt,
- $scope.endDateFmt,
- $scope.page,
- PAGE_SIZE
- ).then(function (result) {
- if (!result || result.length < PAGE_SIZE) {
- $scope.hasLoadAll = true;
- }
- if (result.length === 0) {
- return;
- }
- $scope.auditLogList = $scope.auditLogList.concat(result);
- });
- }
+ const searchByOpNameAndDate = (opName, startDate, endDate) => {
+ if (startDate !== null) {
+ $scope.startDateFmt = new Date(startDate).Format("yyyy-MM-dd hh:mm:ss.S");
+ }
+ if (endDate !== null) {
+ $scope.endDateFmt = new Date(endDate).Format("yyyy-MM-dd hh:mm:ss.S");
+ }
+ $scope.auditLogList = [];
+ $scope.page = 0;
+ $scope.opName = opName;
+ $scope.startDate = startDate;
+ $scope.endDate = endDate;
+ AuditLogService.find_logs_by_opName(
+ $scope.opName,
+ $scope.startDateFmt,
+ $scope.endDateFmt,
+ $scope.page,
+ PAGE_SIZE
+ ).then((result) => {
+ if (!result || result.length < PAGE_SIZE) {
+ $scope.hasLoadAll = true;
+ }
+ if (result.length === 0) {
+ return;
+ }
+ $scope.auditLogList = $scope.auditLogList.concat(result);
+ });
+ };
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
).then(function (result) { | |
if (!result || result.length < PAGE_SIZE) { | |
$scope.hasLoadAll = true; | |
} | |
if (result.length === 0) { | |
return; | |
} | |
$scope.auditLogList = $scope.auditLogList.concat(result); | |
}); | |
).then((result) => { | |
if (!result || result.length < PAGE_SIZE) { | |
$scope.hasLoadAll = true; | |
} | |
if (result.length === 0) { | |
return; | |
} | |
$scope.auditLogList = $scope.auditLogList.concat(result); | |
}); |
AuditLogService.search_by_name_or_type_or_operator(query, 0, 20).then(function (result) { | ||
result.forEach(function (log) { | ||
var optionDisplay = log.opName + '-(' + log.opType + ').by:' + log.operator; | ||
var option = { | ||
id: log.id, | ||
display: optionDisplay, | ||
opName: log.opName | ||
}; | ||
$scope.options.push(option); | ||
}); | ||
} | ||
$scope.showSearchDropdown = $scope.options.length > 0; | ||
}); |
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.
Prefer using for...of
instead of forEach
for better performance and readability.
- result.forEach(function (log) {
- var optionDisplay = log.opName + '-(' + log.opType + ').by:' + log.operator;
- var option = {
- id: log.id,
- display: optionDisplay,
- opName: log.opName
- };
- $scope.options.push(option);
- });
+ for (const log of result) {
+ const optionDisplay = `${log.opName}-(${log.opType}).by:${log.operator}`;
+ const option = {
+ id: log.id,
+ display: optionDisplay,
+ opName: log.opName
+ };
+ $scope.options.push(option);
+ }
Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
AuditLogService.search_by_name_or_type_or_operator(query, 0, 20).then(function (result) { | |
result.forEach(function (log) { | |
var optionDisplay = log.opName + '-(' + log.opType + ').by:' + log.operator; | |
var option = { | |
id: log.id, | |
display: optionDisplay, | |
opName: log.opName | |
}; | |
$scope.options.push(option); | |
}); | |
} | |
$scope.showSearchDropdown = $scope.options.length > 0; | |
}); | |
AuditLogService.search_by_name_or_type_or_operator(query, 0, 20).then(function (result) { | |
for (const log of result) { | |
const optionDisplay = `${log.opName}-(${log.opType}).by:${log.operator}`; | |
const option = { | |
id: log.id, | |
display: optionDisplay, | |
opName: log.opName | |
}; | |
$scope.options.push(option); | |
} | |
$scope.showSearchDropdown = $scope.options.length > 0; | |
}); |
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.
LGTM
What's the purpose of this PR
Beautify Portal TraceLog UI interface
Which issue(s) this PR fixes:
Fixes ##5147
Brief changelog
XXXXX
Follow this checklist to help us incorporate your contribution quickly and easily:
mvn clean test
to make sure this pull request doesn't break anything.CHANGES
log.Summary by CodeRabbit
New Features
Style
Chores
bootstrap-treeview.min.js
to support the treeview functionality.