fix(notifications): Prevent duplicate notifications for MJ (backport #1506)

echo-core/src/main/java/com/netflix/spinnaker/echo/notification/AbstractEventNotificationAgent.java

@@ -108,6 +108,21 @@ public void processEvent(Event event) {
       return;
     }
 
+    if (STAGE.equals(configType)) {
+      Object when = event.getContent().get("when");
+      if (when != null) {
+        if (when instanceof String) {
+          if (!isManualJudgmentMatchStringCase((String) when, status)) {
+            return;
+          }
+        } else if (when instanceof Collection) {
+          if (!isManualJudgmentMatchCollectionCase((Collection<String>) when, status)) {
+            return;
+          }
+        }
+      }
+    }
+
     // TODO(lpollo): why do we have a 'CANCELED' status and a canceled property, which are prime for
     // inconsistency?
     if (isExecution(configType) && isExecutionCanceled(event)) {
@@ -241,11 +256,26 @@ String getName() {
   }
 
   private static boolean isManualJudgmentMatchStringCase(String when, String status) {
+    // For Manual Judgment stages, only send specialized notifications, not standard stage ones
+    // This prevents duplicate notifications between standard stage and manual judgment events
+    if (ManualJudgmentCondition.MANUAL_JUDGMENT.getName().equals(when)
+        || ManualJudgmentCondition.CONTINUE.getName().equals(when)
+        || ManualJudgmentCondition.STOP.getName().equals(when)) {
+      return false;
+    }
     return status.equals(MANUAL_JUDGMENT_CONDITIONS.get(when));
   }
 
   private static boolean isManualJudgmentMatchCollectionCase(
       Collection<String> when, String status) {
+    // For Manual Judgment stages, only send specialized notifications, not standard stage ones
+    // This prevents duplicate notifications between standard stage and manual judgment events
+    if (when.contains(ManualJudgmentCondition.MANUAL_JUDGMENT.getName())
+        || when.contains(ManualJudgmentCondition.CONTINUE.getName())
+        || when.contains(ManualJudgmentCondition.STOP.getName())) {
+      return false;
+    }
+
     for (String condition : when) {
       if (status.equals(MANUAL_JUDGMENT_CONDITIONS.get(condition))) {
         return true;

echo-core/src/test/groovy/com/netflix/spinnaker/echo/notification/AbstractEventNotificationAgentSpec.groovy

@@ -92,6 +92,45 @@ class AbstractEventNotificationAgentSpec extends Specification {
     // notifications ON, stage is synthetic
     fakeStageEvent("orca:stage:complete", "stage.complete", false, true)                      || 0
   }
+
+  @Unroll
+  def "prevents stage notifications for Manual Judgment stages with specialized notifications"() {
+    given:
+    subclassMock.sendNotifications(*_) >> { notification, application, event_local, config, status_param -> }
+
+    when:
+    // Create a stage event with specialized Manual Judgment notification type
+    def event = createManualJudgmentStageEvent("orca:stage:" + status, notificationType)
+    agent.processEvent(event)
+
+    then:
+    // Verify no notifications are sent due to our fix
+    0 * subclassMock.sendNotifications(*_)
+
+    where:
+    status     | notificationType
+    "starting" | "manualJudgment"
+    "complete" | "manualJudgmentContinue"
+    "failed"   | "manualJudgmentStop"
+  }
+
+  @Unroll
+  def "still sends stage notifications for non-Manual Judgment stages"() {
+    given:
+    subclassMock.sendNotifications(*_) >> { notification, application, event_local, config, status_param -> }
+
+    when:
+    // Create a stage event with regular stage notification type
+    def event = createRegularStageEvent("orca:stage:" + status, "stage." + status)
+    agent.processEvent(event)
+
+    then:
+    // Verify notifications are still sent for normal stages
+    1 * subclassMock.sendNotifications(*_)
+
+    where:
+    status << ["starting", "complete", "failed"]
+  }
 
   @Unroll
   def "sends notifications for ManualJudgment stage based on status and configuration"() {
@@ -106,9 +145,9 @@ class AbstractEventNotificationAgentSpec extends Specification {
 
     where:
     event                                                           || expectedNotifications
-    fakeStageEvent("orca:stage:complete", "manualJudgmentContinue") || 1
-    fakeStageEvent("orca:stage:starting", "manualJudgment")         || 1
-    fakeStageEvent("orca:stage:failed", "manualJudgmentStop")       || 1
+    fakeStageEvent("orca:stage:complete", "manualJudgmentContinue") || 0
+    fakeStageEvent("orca:stage:starting", "manualJudgment")         || 0
+    fakeStageEvent("orca:stage:failed", "manualJudgmentStop")       || 0
   }
 
   private def fakePipelineEvent(String type, String status, String notifyWhen, Map extraExecutionProps = [:]) {
@@ -173,4 +212,46 @@ class AbstractEventNotificationAgentSpec extends Specification {
 
     return new Event(eventProps)
   }
+
+  private Event createManualJudgmentStageEvent(String eventType, String notificationType) {
+    return new Event(
+      details: [
+        type: eventType,
+        application: "testapp"
+      ],
+      content: [
+        context: [
+          type: "manualJudgment",
+          sendNotifications: true,
+          notifications: [
+            [
+              type: "fake",
+              when: [notificationType]
+            ]
+          ]
+        ]
+      ]
+    )
+  }
+
+  private Event createRegularStageEvent(String eventType, String notificationType) {
+    return new Event(
+      details: [
+        type: eventType,
+        application: "testapp"
+      ],
+      content: [
+        context: [
+          type: "regularStage",
+          sendNotifications: true,
+          notifications: [
+            [
+              type: "fake",
+              when: [notificationType]
+            ]
+          ]
+        ]
+      ]
+    )
+  }
 }