Merge pull request #1282 from criske/#1268

#1268 Check for closed issue before handling label changes.
self-xdsd · Nov 10, 2021 · 7251fe5 · 7251fe5
2 parents 36a1daf + 701683a
commit 7251fe5
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 12 deletions.
diff --git a/self-core-impl/src/main/java/com/selfxdsd/core/managers/StoredProjectManager.java b/self-core-impl/src/main/java/com/selfxdsd/core/managers/StoredProjectManager.java
@@ -49,9 +49,6 @@
  * @checkstyle ClassFanOutComplexity (1000 lines)
  * @checkstyle ClassDataAbstractionCoupling (1000 lines)
  * @since 0.0.1
- * @todo #1265:30min In method issueLabelsChanged, use the step IssueIsClosed
- *  before doing anything else. We shouldn't act on labeled/unlabeled if the
- *  Issue is closed.
  * @todo #1265:30min Send a comment after updating a task's estimation inside
  *  issueLabelsChanged.
  */
@@ -661,17 +658,23 @@ public void renamedProject(final Event event) {
 
     @Override
     public void issueLabelsChanged(final Event event) {
-        final Step steps = new TaskIsRegistered(
-            new IssueEstimationChanged(
-                new UpdateTaskEstimation(
-                    sendReply -> LOG.debug("Send updated estimation comment.")
+        final Step steps = new IssueIsClosed(
+            isClosed -> LOG.debug(
+                "Can't handle labels changes on a closed issue."
+            ),
+            isNotClosed -> new TaskIsRegistered(
+                new IssueEstimationChanged(
+                    new UpdateTaskEstimation(
+                        sendReply -> LOG.debug("Send updated estimation"
+                            + " comment.")
+                    ),
+                    notChanged -> LOG.debug(
+                        "Nothing to do on Issue label change."
+                    )
                 ),
-                notChanged -> LOG.debug(
-                    "Nothing to do on Issue label change."
+                isNotRegistered -> LOG.debug(
+                    "Issue is NOT registered as a Task. Doing nothing."
                 )
-            ),
-            isNotRegistered -> LOG.debug(
-                "Issue is NOT registered as a Task. Doing nothing."
             )
         );
         steps.perform(event);

diff --git a/self-core-impl/src/test/java/com/selfxdsd/core/managers/StoredProjectManagerTestCase.java b/self-core-impl/src/test/java/com/selfxdsd/core/managers/StoredProjectManagerTestCase.java
@@ -1917,4 +1917,33 @@ public void skipHandlingIssueLabelsChanges() {
             .updateEstimation(Mockito.anyInt());
     }
 
+    /**
+     * A PM can skip handling issue labels changes if issue is closed.
+     */
+    @Test
+    public void skipHandlingIssueLabelsChangesOnClosedIssue() {
+        final ProjectManager manager = new StoredProjectManager(
+            1,
+            "123",
+            "zoeself",
+            Provider.Names.GITHUB,
+            "123token",
+            8,
+            5,
+            Mockito.mock(Storage.class)
+        );
+        final Event event = Mockito.mock(Event.class);
+        final Issue issue = Mockito.mock(Issue.class);
+        final Task task = Mockito.mock(Task.class);
+
+        Mockito.when(issue.isClosed()).thenReturn(true);
+        Mockito.when(event.issue()).thenReturn(issue);
+        Mockito.when(issue.issueId()).thenReturn("123");
+
+        manager.issueLabelsChanged(event);
+
+        Mockito.verify(task, Mockito.never())
+            .updateEstimation(Mockito.anyInt());
+    }
+
 }