fix: add alert issues #9598
Open
fix: add alert issues #9598
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
Failed to generate code suggestions for PR |
Contributor
Greptile OverviewGreptile SummaryThis PR fixes three key issues with alert preview functionality:
Key improvements:
Confidence Score: 4/5
Important Files ChangedFile Analysis
Sequence DiagramsequenceDiagram
participant User
participant AddAlert
participant RealTimeAlert/ScheduledAlert
participant FilterGroup
participant conditionsFormatter
participant alertQueryBuilder
Note over User,alertQueryBuilder: Alert Preview Flow
User->>AddAlert: Edit alert conditions
AddAlert->>AddAlert: formData.query_condition.conditions updated
Note over AddAlert: Compute generatedSqlQuery
AddAlert->>alertQueryBuilder: generateSqlQuery(formData, streamFieldsMap)
alertQueryBuilder->>conditionsFormatter: buildConditionsString(conditions, {sqlMode:true})
conditionsFormatter-->>alertQueryBuilder: WHERE clause
alertQueryBuilder-->>AddAlert: Full SQL query
AddAlert->>RealTimeAlert/ScheduledAlert: Pass streamFieldsMap + generatedSqlQuery
RealTimeAlert/ScheduledAlert->>FilterGroup: Pass streamFieldsMap + generatedSqlQuery + showSqlPreview
Note over FilterGroup: Preview String Computation
alt Full SQL Query Available
FilterGroup->>FilterGroup: Show sqlQuery prop (complete SQL)
else showSqlPreview is true
FilterGroup->>conditionsFormatter: buildConditionsString({sqlMode:true, addWherePrefix:true})
conditionsFormatter-->>FilterGroup: WHERE clause only
else Default Display Mode
FilterGroup->>conditionsFormatter: buildConditionsString({sqlMode:false})
conditionsFormatter-->>FilterGroup: Display format (lowercase)
end
FilterGroup-->>User: Display preview string
Note over User,alertQueryBuilder: Alert List Lazy Conversion Flow
User->>AlertList: Load alerts page
AlertList->>AlertList: Store rawCondition without conversion
Note over AlertList: Performance optimization:<br/>No conversion on list load
User->>AlertList: Click expand on alert
AlertList->>AlertList: triggerExpand(alert)
Note over AlertList: Lazy conversion on-demand
alt V2 format
AlertList->>conditionsFormatter: buildConditionsString(v2Format, {sqlMode:false})
conditionsFormatter-->>AlertList: Display string
else V1 format
AlertList->>AlertList: transformToExpression(v1Format)
else V0 format
AlertList->>AlertList: Manual conversion (flat array)
end
AlertList-->>User: Show alert details with converted conditions
|
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+987
to
+1021
| // This improves performance by avoiding conversion of all alerts on list load | ||
| let displayConditions = "--"; | ||
| if (alert.rawCondition && Object.keys(alert.rawCondition).length) { | ||
| if (alert.rawCondition.type == 'custom') { | ||
| const conditionData = alert.rawCondition.conditions; | ||
| // Detect format by structure, not by version field (more reliable) | ||
| if (conditionData?.filterType === 'group') { | ||
| // V2 format: {filterType: "group", logicalOperator: "AND", conditions: [...]} | ||
| displayConditions = transformV2ToExpression(conditionData); | ||
| } else if (conditionData?.version === 2 && conditionData?.conditions) { | ||
| // V2 format with version wrapper: {version: 2, conditions: {filterType: "group", ...}} | ||
| displayConditions = transformV2ToExpression(conditionData.conditions); | ||
| } else if (conditionData?.or || conditionData?.and) { | ||
| // V1 format: {or: [...]} or {and: [...]} | ||
| displayConditions = transformToExpression(conditionData); | ||
| } else if (Array.isArray(conditionData) && conditionData.length > 0) { | ||
| // V0 format (legacy): flat array [{column, operator, value}, ...] | ||
| // V0 had implicit AND between all conditions (no groups) | ||
| const parts = conditionData.map((item: any) => { | ||
| const column = item.column || 'field'; | ||
| const operator = item.operator || '='; | ||
| const value = typeof item.value === 'string' ? `'${item.value}'` : item.value; | ||
| return `${column} ${operator} ${value}`; | ||
| }); | ||
| displayConditions = parts.length > 0 ? `(${parts.join(' AND ')})` : '--'; | ||
| } else { | ||
| // Unknown format or empty | ||
| displayConditions = typeof conditionData === 'string' ? conditionData : '--'; | ||
| } | ||
| } else if (alert.rawCondition.sql) { | ||
| displayConditions = alert.rawCondition.sql; | ||
| } else if (alert.rawCondition.promql) { | ||
| displayConditions = alert.rawCondition.promql; | ||
| } |
Contributor
There was a problem hiding this comment.
style: duplicated conversion logic that could be extracted to a helper function
The condition format detection and conversion logic (V0/V1/V2 format handling) is duplicated here and in transformV2ToExpression. Consider extracting this to a shared helper function for better maintainability.
Suggested change
| // This improves performance by avoiding conversion of all alerts on list load | |
| let displayConditions = "--"; | |
| if (alert.rawCondition && Object.keys(alert.rawCondition).length) { | |
| if (alert.rawCondition.type == 'custom') { | |
| const conditionData = alert.rawCondition.conditions; | |
| // Detect format by structure, not by version field (more reliable) | |
| if (conditionData?.filterType === 'group') { | |
| // V2 format: {filterType: "group", logicalOperator: "AND", conditions: [...]} | |
| displayConditions = transformV2ToExpression(conditionData); | |
| } else if (conditionData?.version === 2 && conditionData?.conditions) { | |
| // V2 format with version wrapper: {version: 2, conditions: {filterType: "group", ...}} | |
| displayConditions = transformV2ToExpression(conditionData.conditions); | |
| } else if (conditionData?.or || conditionData?.and) { | |
| // V1 format: {or: [...]} or {and: [...]} | |
| displayConditions = transformToExpression(conditionData); | |
| } else if (Array.isArray(conditionData) && conditionData.length > 0) { | |
| // V0 format (legacy): flat array [{column, operator, value}, ...] | |
| // V0 had implicit AND between all conditions (no groups) | |
| const parts = conditionData.map((item: any) => { | |
| const column = item.column || 'field'; | |
| const operator = item.operator || '='; | |
| const value = typeof item.value === 'string' ? `'${item.value}'` : item.value; | |
| return `${column} ${operator} ${value}`; | |
| }); | |
| displayConditions = parts.length > 0 ? `(${parts.join(' AND ')})` : '--'; | |
| } else { | |
| // Unknown format or empty | |
| displayConditions = typeof conditionData === 'string' ? conditionData : '--'; | |
| } | |
| } else if (alert.rawCondition.sql) { | |
| displayConditions = alert.rawCondition.sql; | |
| } else if (alert.rawCondition.promql) { | |
| displayConditions = alert.rawCondition.promql; | |
| } | |
| // LAZY CONVERSION: Convert conditions on-demand only when expanding | |
| // This improves performance by avoiding conversion of all alerts on list load | |
| let displayConditions = "--"; | |
| if (alert.rawCondition && Object.keys(alert.rawCondition).length) { | |
| displayConditions = convertConditionsToDisplay(alert.rawCondition); | |
| } | |
| // Helper function (can be extracted to utils) | |
| function convertConditionsToDisplay(rawCondition: any): string { | |
| if (rawCondition.type === 'custom') { | |
| const conditionData = rawCondition.conditions; | |
| if (conditionData?.filterType === 'group') { | |
| return transformV2ToExpression(conditionData); | |
| } else if (conditionData?.version === 2 && conditionData?.conditions) { | |
| return transformV2ToExpression(conditionData.conditions); | |
| } else if (conditionData?.or || conditionData?.and) { | |
| return transformToExpression(conditionData); | |
| } else if (Array.isArray(conditionData) && conditionData.length > 0) { | |
| const parts = conditionData.map((item: any) => { | |
| const column = item.column || 'field'; | |
| const operator = item.operator || '='; | |
| const value = typeof item.value === 'string' ? `'${item.value}'` : item.value; | |
| return `${column} ${operator} ${value}`; | |
| }); | |
| return parts.length > 0 ? `(${parts.join(' AND ')})` : '--'; | |
| } | |
| return typeof conditionData === 'string' ? conditionData : '--'; | |
| } else if (rawCondition.sql) { | |
| return rawCondition.sql; | |
| } else if (rawCondition.promql) { | |
| return rawCondition.promql; | |
| } | |
| return '--'; | |
| } |
Prompt To Fix With AI
This is a comment left during a code review.
Path: web/src/components/alerts/AlertList.vue
Line: 987:1021
Comment:
**style:** duplicated conversion logic that could be extracted to a helper function
The condition format detection and conversion logic (V0/V1/V2 format handling) is duplicated here and in `transformV2ToExpression`. Consider extracting this to a shared helper function for better maintainability.
```suggestion
// LAZY CONVERSION: Convert conditions on-demand only when expanding
// This improves performance by avoiding conversion of all alerts on list load
let displayConditions = "--";
if (alert.rawCondition && Object.keys(alert.rawCondition).length) {
displayConditions = convertConditionsToDisplay(alert.rawCondition);
}
// Helper function (can be extracted to utils)
function convertConditionsToDisplay(rawCondition: any): string {
if (rawCondition.type === 'custom') {
const conditionData = rawCondition.conditions;
if (conditionData?.filterType === 'group') {
return transformV2ToExpression(conditionData);
} else if (conditionData?.version === 2 && conditionData?.conditions) {
return transformV2ToExpression(conditionData.conditions);
} else if (conditionData?.or || conditionData?.and) {
return transformToExpression(conditionData);
} else if (Array.isArray(conditionData) && conditionData.length > 0) {
const parts = conditionData.map((item: any) => {
const column = item.column || 'field';
const operator = item.operator || '=';
const value = typeof item.value === 'string' ? `'${item.value}'` : item.value;
return `${column} ${operator} ${value}`;
});
return parts.length > 0 ? `(${parts.join(' AND ')})` : '--';
}
return typeof conditionData === 'string' ? conditionData : '--';
} else if (rawCondition.sql) {
return rawCondition.sql;
} else if (rawCondition.promql) {
return rawCondition.promql;
}
return '--';
}
```
How can I resolve this? If you propose a fix, please make it concise.Auto-generated translation updates from English source file. 🤖 Generated with automated translation workflow
Auto-generated translation updates from English source file. 🤖 Generated with automated translation workflow
…serve/openobserve into fix/add-alert-issues-main
Auto-generated translation updates from English source file. 🤖 Generated with automated translation workflow
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes the below issues