[GitHub] [incubator-pinot] jasonyanwenl commented on a change in pull request #5823: [TE] enchance anomaly api to propagate feedback

[GitBox <gi...@apache.org>]

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



##########
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:
       Just a quick question, where is the sibling anomaly updated? Thanks!




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



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