vincentchenjl commented on a change in pull request #5823:
URL: https://github.com/apache/incubator-pinot/pull/5823#discussion_r467272726



##########
File path: 
thirdeye/thirdeye-pinot/src/main/java/org/apache/pinot/thirdeye/dashboard/resources/AnomalyResource.java
##########
@@ -156,30 +156,49 @@ public MergedAnomalyResultDTO getMergedAnomalyDetail(
    *                        eg. payload
    *                        <p/>
    *                        { "feedbackType": "NOT_ANOMALY", "comment": "this 
is not an anomaly" }
+   * @param propagate       : a flag whether it should propagate the same 
feedback value to the parent of this anomaly
+   *                        and all its siblings if they exist
    */
   @POST
   @Path(value = "anomaly-merged-result/feedback/{anomaly_merged_result_id}")
   @ApiOperation("update anomaly merged result feedback")
-  public void 
updateAnomalyMergedResultFeedback(@PathParam("anomaly_merged_result_id") long 
anomalyResultId,
+  public void updateAnomalyMergedResultFeedback(
+      @PathParam("anomaly_merged_result_id") long anomalyResultId,
+      @QueryParam("propagate") @DefaultValue("true") boolean propagate,
       String payload) {
     try {
       MergedAnomalyResultDTO result = 
anomalyMergedResultDAO.findById(anomalyResultId);
       if (result == null) {
         throw new IllegalArgumentException("AnomalyResult not found with id " 
+ anomalyResultId);
       }
       AnomalyFeedbackDTO feedbackRequest = OBJECT_MAPPER.readValue(payload, 
AnomalyFeedbackDTO.class);
-      AnomalyFeedback feedback = result.getFeedback();
-      if (feedback == null) {
-        feedback = new AnomalyFeedbackDTO();
-        result.setFeedback(feedback);
-      }
-      feedback.setComment(feedbackRequest.getComment());
-      if (feedbackRequest.getFeedbackType() != null){
-        feedback.setFeedbackType(feedbackRequest.getFeedbackType());
+      if (propagate && result.isChild()) {
+        // propagate the feedback to its parent and siblings
+        MergedAnomalyResultDTO parent = 
anomalyMergedResultDAO.findParent(result);
+        if (parent != null) {
+          updateAnomalyFeedback(parent, feedbackRequest);
+        } else {
+          LOG.warn("Cannot find parent for anomaly : {}, thus only updating 
the feedback of it", result.getId());
+          updateAnomalyFeedback(result, feedbackRequest);
+        }
+      } else {
+        updateAnomalyFeedback(result, feedbackRequest);
       }
-      anomalyMergedResultDAO.updateAnomalyFeedback(result);
     } catch (IOException e) {
       throw new IllegalArgumentException("Invalid payload " + payload, e);
     }
   }
+
+  private void updateAnomalyFeedback(MergedAnomalyResultDTO anomaly, 
AnomalyFeedbackDTO newFeedback) {

Review comment:
       ` anomalyMergedResultDAO.updateAnomalyFeedback(anomaly)` actually 
updates the feedback labels of `anomaly` and all its child anomaly.




----------------------------------------------------------------
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:
us...@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org

Reply via email to