#56 Don't fill gaps with items from previous days if the deleted item was ongoing.

Author: Bytekeeper

src/main/java/org/stt/command/Activities.java

@@ -1,13 +1,17 @@
 package org.stt.command;
 
+import net.engio.mbassy.bus.BusRuntime;
+import net.engio.mbassy.bus.IMessagePublication;
 import net.engio.mbassy.bus.MBassador;
+import net.engio.mbassy.bus.common.PubSubSupport;
 import org.stt.model.ItemDeleted;
 import org.stt.model.ItemInserted;
 import org.stt.model.ItemReplaced;
 import org.stt.model.TimeTrackingItem;
 import org.stt.persistence.ItemPersister;
 import org.stt.query.Criteria;
 import org.stt.query.TimeTrackingItemQueries;
+import org.stt.time.DateTimes;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
@@ -23,13 +27,14 @@
 public class Activities implements CommandHandler {
     private ItemPersister persister;
     private final TimeTrackingItemQueries queries;
-    private Optional<MBassador<Object>> eventBus;
+    private final PubSubSupport<Object> publisher;
 
     @Inject
     public Activities(ItemPersister persister,
                       TimeTrackingItemQueries queries,
-                      Optional<MBassador<Object>> eventBus) {
-        this.eventBus = requireNonNull(eventBus);
+                      Optional<MBassador<Object>> publisher) {
+        this.publisher = requireNonNull(publisher).map(PubSubSupport.class::cast)
+                .orElseGet(DoNotPublish::new);
         this.persister = requireNonNull(persister);
         this.queries = requireNonNull(queries);
     }
@@ -46,10 +51,10 @@ public void addNewActivity(NewActivity command) {
         if (potentialItemToReplace.isPresent()) {
             TimeTrackingItem itemToReplace = potentialItemToReplace.get();
             persister.replace(itemToReplace, command.newItem);
-            eventBus.ifPresent(eb -> eb.publish(new ItemReplaced(itemToReplace, command.newItem)));
+            publisher.publish(new ItemReplaced(itemToReplace, command.newItem));
         } else {
             persister.persist(command.newItem);
-            eventBus.ifPresent(eb -> eb.publish(new ItemInserted(command.newItem)));
+            publisher.publish(new ItemInserted(command.newItem));
         }
     }
 
@@ -74,15 +79,15 @@ public void endCurrentActivity(EndCurrentItem command) {
                 .ifPresent(item -> {
                     TimeTrackingItem derivedItem = item.withEnd(command.endAt);
                     persister.replace(item, derivedItem);
-                    eventBus.ifPresent(eb -> eb.publish(new ItemReplaced(item, derivedItem)));
+                    publisher.publish(new ItemReplaced(item, derivedItem));
                 });
     }
 
     @Override
     public void removeActivity(RemoveActivity command) {
         requireNonNull(command);
         persister.delete(command.itemToDelete);
-        eventBus.ifPresent(eb -> eb.publish(new ItemDeleted(command.itemToDelete)));
+        publisher.publish(new ItemDeleted(command.itemToDelete));
     }
 
     @Override
@@ -96,11 +101,13 @@ public void removeActivityAndCloseGap(RemoveActivity command) {
             TimeTrackingItem replaceAllWith = next.get().getEnd()
                     .map(previous.get()::withEnd).orElse(previous.get().withPendingEnd());
             persister.persist(replaceAllWith);
-            eventBus.ifPresent(eb -> eb.publish(new ItemInserted(replaceAllWith)));
-        } else if (previous.isPresent() && !command.itemToDelete.getEnd().isPresent()) {
+            publisher.publish(new ItemInserted(replaceAllWith));
+        } else if (previous.isPresent()
+                && DateTimes.isOnSameDay(previous.get().getStart(), command.itemToDelete.getStart())
+                && !command.itemToDelete.getEnd().isPresent()) {
             TimeTrackingItem replaceAllWith = previous.get().withPendingEnd();
             persister.persist(replaceAllWith);
-            eventBus.ifPresent(eb -> eb.publish(new ItemInserted(replaceAllWith)));
+            publisher.publish(new ItemInserted(replaceAllWith));
         } else {
             removeActivity(command);
         }
@@ -118,7 +125,7 @@ public void resumeActivity(ResumeActivity command) {
                 .withPendingEnd()
                 .withStart(command.beginningWith);
         persister.persist(resumedItem);
-        eventBus.ifPresent(eb -> eb.publish(new ItemInserted(resumedItem)));
+        publisher.publish(new ItemInserted(resumedItem));
     }
 
     @Override
@@ -130,7 +137,7 @@ public void resumeLastActivity(ResumeLastActivity command) {
             lastTimeTrackingItem.ifPresent(timeTrackingItem -> {
                 TimeTrackingItem resumedItem = timeTrackingItem.withPendingEnd().withStart(command.resumeAt);
                 persister.persist(resumedItem);
-                eventBus.ifPresent(eb -> eb.publish(new ItemInserted(resumedItem)));
+                publisher.publish(new ItemInserted(resumedItem));
             });
         }
     }
@@ -140,8 +147,30 @@ public void bulkChangeActivity(Collection<TimeTrackingItem> itemsToChange, Strin
         requireNonNull(itemsToChange);
         requireNonNull(activity);
         Collection<ItemPersister.UpdatedItem> updatedItems = persister.updateActivitities(itemsToChange, activity);
-        eventBus.ifPresent(eb -> updatedItems.stream()
+        updatedItems.stream()
                 .map(updatedItem -> new ItemReplaced(updatedItem.original, updatedItem.updated))
-                .forEach(eb::publish));
+                .forEach(publisher::publish);
+    }
+
+    private static class DoNotPublish implements PubSubSupport<Object> {
+        @Override
+        public void subscribe(Object listener) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public boolean unsubscribe(Object listener) {
+            throw new IllegalStateException();
+        }
+
+        @Override
+        public IMessagePublication publish(Object message) {
+            return null;
+        }
+
+        @Override
+        public BusRuntime getRuntime() {
+            throw new IllegalStateException();
+        }
     }
 }

src/main/java/org/stt/gui/jfx/ActivitiesController.java

@@ -480,31 +480,27 @@ public void endCurrentActivity(EndCurrentItem command) {
 
         @Override
         public void removeActivity(RemoveActivity command) {
-            activities.removeActivity(command);
-            clearCommand();
+            throw new IllegalStateException();
         }
 
         @Override
         public void removeActivityAndCloseGap(RemoveActivity command) {
-            activities.removeActivityAndCloseGap(command);
-            clearCommand();
+            throw new IllegalStateException();
         }
 
         @Override
         public void resumeActivity(ResumeActivity command) {
-            activities.resumeActivity(command);
+            throw new IllegalStateException();
         }
 
         @Override
         public void resumeLastActivity(ResumeLastActivity command) {
-            activities.resumeLastActivity(command);
-            clearCommand();
-
+            throw new IllegalStateException();
         }
 
         @Override
         public void bulkChangeActivity(Collection<TimeTrackingItem> itemsToChange, String activity) {
-            activities.bulkChangeActivity(itemsToChange, activity);
+            throw new IllegalStateException();
         }
 
         private boolean validateItemIsFirstItemAndLater(LocalDateTime start) {

src/test/java/org/stt/command/ActivitiesTest.java

@@ -107,6 +107,28 @@ public void shouldFillGapOnDeletedIsOngoing() {
         verifyNoMoreInteractions(persister);
     }
 
+    @Test
+    public void shouldNotFillGapIfPreviousItemStartsOnDifferentDay() {
+        // GIVEN
+        TimeTrackingItem before = new TimeTrackingItem("expected",
+                LocalDateTime.of(2000, 10, 9, 10, 9),
+                LocalDateTime.of(2000, 10, 10, 10, 10));
+        TimeTrackingItem itemToDelete = new TimeTrackingItem("toDelete",
+                LocalDateTime.of(2000, 10, 10, 10, 10));
+
+        given(queries.getAdjacentItems(itemToDelete)).willReturn(new TimeTrackingItemQueries.AdjacentItems(before, null));
+
+        RemoveActivity removeActivity = new RemoveActivity(itemToDelete);
+
+        // WHEN
+        sut.removeActivityAndCloseGap(removeActivity);
+
+        // THEN
+        verify(persister).delete(itemToDelete);
+        verifyNoMoreInteractions(persister);
+    }
+
+
     @Test
     public void shouldDeletedWithoutPrevious() {
         // GIVEN

src/test/java/org/stt/gui/jfx/ActivitiesControllerTest.java

@@ -184,22 +184,6 @@ public void shouldClearCommandAreaOnExecuteCommand() throws Exception {
         assertThat(sut.commandText.getText(), equalTo(""));
     }
 
-    @Test
-    public void shouldDelegateCommandExecutionToCommandHandler()
-            throws Exception {
-        // GIVEN
-        String testCommand = "test";
-
-        givenCommand(testCommand);
-        given(commandFormatter.parse(anyString())).willReturn(new RemoveActivity(new TimeTrackingItem("", LocalDateTime.now())));
-
-        // WHEN
-        sut.executeCommand();
-
-        // THEN
-        verify(commandFormatter).parse(testCommand);
-    }
-
     @Test
     public void shouldNotCloseWindowOnSimpleCommandExecution() {
         // GIVEN