Skip to content
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: reactivate natural sorting order of notification by severity #777

Conversation

poojapatel23
Copy link

@poojapatel23 poojapatel23 commented Mar 21, 2024

This PR introduces a new feature: reactivate natural sorting order of notifications by severity

[Issue]:

[Summary]:

  • This commit provides the sorting order of notifications (alerts and investigations) to follow a natural sorting order based on severity. Previously, the sorting method was sorted alphabetically

[Changes Made]:

  • Modified/Added the SQL query responsible for sorting notifications by severity.

[Context/Reasoning]:

  • Natural sorting by severity provides a more intuitive and user-friendly display of notifications, ensuring that higher severity issues are prioritized appropriately.

…s by severity

[Summary]:
This commit reverts the sorting order of notifications (alerts and investigations) to follow a natural sorting order based on severity. Previously, the sorting method was sorted alphabetically

[Changes Made]:
- Modified/Added the SQL query responsible for sorting notifications by severity.
- Utilized a formula for natural sorting order based on severity.

[Context/Reasoning]:
Natural sorting by severity provides a more intuitive and user-friendly display of notifications, ensuring that higher severity issues are prioritized appropriately.
"WHEN A.severity = 'CRITICAL' THEN 2 " +
"WHEN A.severity = 'LIFE_THREATENING' THEN 3 " +
"ELSE -1 END FROM alert_notification A WHERE A.alert_id = id LIMIT 1)")
private Integer severityRank;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think its kind of confusing adding this logic as sql. It would be cleaner to add the rank of the natural order to the QualityNotificationSeverity Enum and sort its value

@@ -74,7 +74,7 @@ private static Sort toDomainSort(final List<String> sorts, BaseRequestFieldMappe
try {
String[] sortParams = sort.split(",");
orders.add(new Sort.Order(Sort.Direction.valueOf(sortParams[1].toUpperCase()),
fieldMapper.mapRequestFieldName(sortParams[0])));
handleEnumColumns(fieldMapper.mapRequestFieldName(sortParams[0]))));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add the new supported field to QualityNotificationFieldMapper and remove the newly created method.

Copy link
Contributor

@ds-lcapellino ds-lcapellino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please see comments

@poojapatel23 poojapatel23 marked this pull request as draft April 4, 2024 13:59
@poojapatel23 poojapatel23 deleted the feature/severity-according-to-the-natural-order branch May 7, 2024 11:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants