ruffle1986 commented on a change in pull request #1344: METRON-1654: findOne
request after an alert patch returns with the original state of the alert item
URL: https://github.com/apache/metron/pull/1344#discussion_r262960780
##########
File path:
metron-interface/metron-alerts/src/app/alerts/alert-details/alert-details.component.ts
##########
@@ -252,22 +234,15 @@ export class AlertDetailsComponent implements OnInit {
const confirmedSubscription =
this.dialogService.launchDialog(commentText).subscribe(action => {
if (action === ConfirmationType.Confirmed) {
- let deletedCommentWrapper = this.alertCommentsWrapper.splice(index,
1)[0];
let commentRequest = new CommentAddRemoveRequest();
commentRequest.guid = this.alertSource.guid;
commentRequest.comment =
this.alertCommentsWrapper[index].alertComment.comment;
commentRequest.username =
this.alertCommentsWrapper[index].alertComment.username;
commentRequest.timestamp =
this.alertCommentsWrapper[index].alertComment.timestamp;
commentRequest.sensorType = this.alertSourceType;
- this.updateService.removeComment(commentRequest).subscribe(
- () => {
- this.alertCommentsWrapper.map(alertsWrapper =>
alertsWrapper.alertComment)
Review comment:
It's so good to see this code to be removed!
The first parameter of the subscribe function is the success callback. So in
case of success, it does nothing but mapping through the
`this.alertCommentsWrapper` array unnecessarily.
The second parameter of `subscribe` is the failure callback where the code
puts the comment back to the list in case of failure. In the updated code, you
don't cover this case.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
