diff --git a/.idea/caches/build_file_checksums.ser b/.idea/caches/build_file_checksums.ser index b902e1e49..35297e4b4 100644 Binary files a/.idea/caches/build_file_checksums.ser and b/.idea/caches/build_file_checksums.ser differ diff --git a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java index af7a0e81f..07cf5e3b4 100644 --- a/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java +++ b/domain/src/main/java/org/sagebionetworks/research/domain/task/navigation/strategy/StrategyBasedNavigator.java @@ -34,22 +34,15 @@ import android.support.annotation.NonNull; import android.support.annotation.Nullable; -import android.support.annotation.VisibleForTesting; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableMap; -import com.google.common.collect.ImmutableMap.Builder; import org.sagebionetworks.research.domain.result.Result; import org.sagebionetworks.research.domain.result.TaskResult; import org.sagebionetworks.research.domain.step.Step; -import org.sagebionetworks.research.domain.step.ui.action.ActionType.Navigation; import org.sagebionetworks.research.domain.task.Task.Progress; import org.sagebionetworks.research.domain.task.navigation.StepNavigator; import org.sagebionetworks.research.domain.task.navigation.TreeNavigator; -import org.sagebionetworks.research.domain.task.navigation.strategy.ConditionalStepNavigationStrategy.ReplacementStepNavigationStrategy; import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy.Identifiers; import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; import java.util.List; @@ -111,7 +104,7 @@ public Step getPreviousStep(@NonNull final Step step, @NonNull TaskResult taskRe // If backward navigation is allowed we check the result. Step previousStep = null; List stepHistory = taskResult.getStepHistory(); - int idx = stepHistory.indexOf(step); + int idx = stepHistory.indexOf(taskResult.getResult(step.getIdentifier())); if (idx > 0) { String previousStepId = stepHistory.get(idx - 1).getIdentifier(); previousStep = this.getStep(previousStepId); @@ -126,7 +119,7 @@ public Step getPreviousStep(@NonNull final Step step, @NonNull TaskResult taskRe return previousStep; } - @NonNull + @Nullable @Override public Progress getProgress(@NonNull final Step step, @NonNull TaskResult taskResult) { return this.treeNavigator.getProgress(step, taskResult); diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java index 5c2035414..b4b24fe5b 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/IndividualNavigatorTests.java @@ -23,10 +23,9 @@ import static org.mockito.Mockito.*; public abstract class IndividualNavigatorTests { - private static final String STEP_TYPE = "step"; - public static final List TEST_STEPS; - public static final List TEST_PROGRESS_MARKERS; + protected static final List TEST_STEPS; + protected static final List TEST_PROGRESS_MARKERS; static { List steps = new ArrayList<>(createSteps(new String[]{"introduction", "step1", "step2", "step3"})); steps.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.C"}))); @@ -46,6 +45,7 @@ public IndividualNavigatorTests(StepNavigator navigator) { this.steps = TEST_STEPS; } + // region Mocking public static Step mockStep(String identifier) { Step step = mock(Step.class); when(step.getIdentifier()).thenReturn(identifier); @@ -104,13 +104,13 @@ public static TaskResult mockTaskResultFromResults(String identifier, List()); @@ -226,9 +226,9 @@ public void testProgress_MarkersAndSections_Completion() { Task.Progress progress = navigator.getProgress(steps.get(8), taskResult); assertNull(progress); } + // endregion - // MARK: test getPreviousStep() - + // region Test getPreviousStep() @Test public void testBack_From5X() { List stepHistory = new ArrayList<>(); @@ -270,9 +270,9 @@ public void testBack_From3() { assertNotNull(previousStep); assertEquals("step2", previousStep.getIdentifier()); } + // endregion - // MARK: test getNextStep() - + // region Test getNextStep() @Test public void testForward_From5X() { List stepHistory = new ArrayList<>(); @@ -314,10 +314,9 @@ public void testForward_From2() { assertNotNull(nextStep); assertEquals("step3", nextStep.getIdentifier()); } + // endregion - - // MARK: test getStep() - + // region Test getStep() @Test public void testGetStep() { assertEquals(steps.get(0), navigator.getStep("introduction")); @@ -325,4 +324,5 @@ public void testGetStep() { assertEquals(steps.get(7), navigator.getStep("step7")); assertEquals(steps.get(5), navigator.getStep("step5")); } + // endregion } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java index f19ebaddf..0cba002a4 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/StrategyBasedNavigatorTests.java @@ -7,6 +7,9 @@ import org.sagebionetworks.research.domain.step.SectionStep; import org.sagebionetworks.research.domain.step.Step; import org.sagebionetworks.research.domain.task.Task; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; +import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; import org.sagebionetworks.research.domain.task.navigation.strategy.StrategyBasedNavigator; import java.util.ArrayList; @@ -23,15 +26,40 @@ public class StrategyBasedNavigatorTests extends IndividualNavigatorTests { private static final String SKIP_RESULT_IDENTIFIER = "skip"; private static final String TEST_STEP_TYPE = "test step"; + private static final List STRATEGY_TEST_STEPS; + private static final StrategyBasedNavigator STRATEGY_TEST_NAVIGATOR; + static { + STRATEGY_TEST_STEPS = new ArrayList<>(); + STRATEGY_TEST_STEPS.add(mockTestStep("introduction", true, "step2", false)); + STRATEGY_TEST_STEPS.add(mockStep("step1")); + STRATEGY_TEST_STEPS.add(mockTestStep("step2", true, null, true)); + STRATEGY_TEST_STEPS.add(mockTestStep("step3", true, "step1", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.c"}))); + List step5Substeps = new ArrayList<>(); + step5Substeps.add(mockTestStep("step5.X", true, null, true)); + step5Substeps.add(mockTestStep("step5.Y", true, "step7", false)); + step5Substeps.add(mockTestStep("step5.Z", true, "step4.B", false)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step5", step5Substeps)); + STRATEGY_TEST_STEPS.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.c"}))); + STRATEGY_TEST_STEPS.add(mockTestStep("step7", false, "step6.A", false)); + STRATEGY_TEST_STEPS.add(mockStep("conclusion")); + STRATEGY_TEST_NAVIGATOR = new StrategyBasedNavigator(STRATEGY_TEST_STEPS, TEST_PROGRESS_MARKERS); + } + + public StrategyBasedNavigatorTests() { + super(new StrategyBasedNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + } + // region Mocking private static Step mockTestStep(String identifier, boolean isBackAllowed, String nextStepId, boolean shouldAddSkipRule) { - TestStep step = mock(TestStep.class); + Step step = mock(Step.class, withSettings().extraInterfaces(NextStepStrategy.class, + BackStepStrategy.class, SkipStepStrategy.class)); when(step.getIdentifier()).thenReturn(identifier); when(step.getType()).thenReturn(TEST_STEP_TYPE); when(step.toString()).thenReturn(identifier + ": " + TEST_STEP_TYPE); - when(step.isBackAllowed(any(TaskResult.class))).thenReturn(isBackAllowed); - when(step.getNextStepIdentifier(any(TaskResult.class))).thenReturn(nextStepId); + when(((BackStepStrategy)step).isBackAllowed(any(TaskResult.class))).thenReturn(isBackAllowed); + when(((NextStepStrategy)step).getNextStepIdentifier(any(TaskResult.class))).thenReturn(nextStepId); if (shouldAddSkipRule) { // If we are adding a skip rule, return true for any argument in which the given TaskResult contains a // result with the SKIP_RESULT_IDENTIFIER, false otherwise. @@ -41,28 +69,29 @@ public boolean matches(final TaskResult argument) { return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) != null; } }); - when(step.shouldSkip(match)).thenReturn(true); + when(((SkipStepStrategy)step).shouldSkip(match)).thenReturn(true); TaskResult notMatch = argThat(new ArgumentMatcher() { @Override public boolean matches(final TaskResult argument) { return argument != null && argument.getResult(SKIP_RESULT_IDENTIFIER) == null; } }); - when(step.shouldSkip(notMatch)).thenReturn(false); + when(((SkipStepStrategy)step).shouldSkip(notMatch)).thenReturn(false); } else { // If we aren't adding a skip rule we always return false. - when(step.shouldSkip(any(TaskResult.class))).thenReturn(false); + when(((SkipStepStrategy)step).shouldSkip(any(TaskResult.class))).thenReturn(false); } return step; } - public StrategyBasedNavigatorTests() { - super(new StrategyBasedNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); + private TaskResult mockSectionResult(Step step, int from, int to) { + SectionStep sectionStep = (SectionStep) step; + return mockTaskResult(sectionStep.getIdentifier(), sectionStep.getSteps().subList(from, to)); } + // endregion - // MARK: Test getProgress(). - + // region Test getProgress() @Test public void testProgess_NoMarkers_FlatHierarchy() { List steps = createSteps(new String[]{"1", "2", "3", "4"}); @@ -75,34 +104,9 @@ public void testProgess_NoMarkers_FlatHierarchy() { assertEquals(4, progress.getTotal()); assertTrue(progress.isEstimated()); } + // endregion - private TaskResult mockSectionResult(Step step, int from, int to) { - SectionStep sectionStep = (SectionStep) step; - return mockTaskResult(sectionStep.getIdentifier(), sectionStep.getSteps().subList(from, to)); - } - - private static final List STRATEGY_TEST_STEPS; - private static final StrategyBasedNavigator STRATEGY_TEST_NAVIGATOR; - static { - STRATEGY_TEST_STEPS = new ArrayList<>(); - STRATEGY_TEST_STEPS.add(mockTestStep("introduction", true, "step2", false)); - STRATEGY_TEST_STEPS.add(mockStep("step1")); - STRATEGY_TEST_STEPS.add(mockTestStep("step2", true, null, true)); - STRATEGY_TEST_STEPS.add(mockTestStep("step3", true, "step1", false)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step4", createSteps(new String[]{"step4.A", "step4.B", "step4.c"}))); - List step5Substeps = new ArrayList<>(); - step5Substeps.add(mockTestStep("step5.X", true, null, true)); - step5Substeps.add(mockTestStep("step5.Y", true, "step7", false)); - step5Substeps.add(mockTestStep("step5.Z", true, "step4.B", false)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step5", step5Substeps)); - STRATEGY_TEST_STEPS.add(mockSectionStep("step6", createSteps(new String[]{"step6.A", "step6.B", "step6.c"}))); - STRATEGY_TEST_STEPS.add(mockTestStep("step7", false, "step6.A", false)); - STRATEGY_TEST_STEPS.add(mockStep("conclusion")); - STRATEGY_TEST_NAVIGATOR = new StrategyBasedNavigator(STRATEGY_TEST_STEPS, TEST_PROGRESS_MARKERS); - } - - // MARK: test skip rule - + // region Test With SkipStepStrategy @Test public void testSkip_resultPresent_From2() { List stepHistory = new ArrayList<>(); @@ -162,9 +166,9 @@ public void testSkip_resultNotPresent_From5X() { assertNotNull(nextStep); assertEquals("step5.X", nextStep.getIdentifier()); } + // endregion - // MARK: test back rule - + // region Test With BackStepStrategy @Test public void testBackAllowed_From2() { List stepHistory = new ArrayList<>(); @@ -207,9 +211,9 @@ public void testBackNotAllowed_From7() { Step previousStep = STRATEGY_TEST_NAVIGATOR.getPreviousStep(STRATEGY_TEST_STEPS.get(7), taskResult); assertNull(previousStep); } + // endregion - // MARK: test next step rule - + // region Test With NextStepStrategy @Test public void testNext_fromStep2() { TaskResult taskResult = mockTaskResult("task", STRATEGY_TEST_STEPS.subList(0, 2)); @@ -288,4 +292,5 @@ public void testNext_from7() { assertNotNull(nextStep); assertEquals("step6.A", nextStep.getIdentifier()); } + // endregion } \ No newline at end of file diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java deleted file mode 100644 index 3aec13b15..000000000 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TestStep.java +++ /dev/null @@ -1,8 +0,0 @@ -package org.sagebionetworks.research.domain.navigation; - -import org.sagebionetworks.research.domain.step.Step; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.BackStepStrategy; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.NextStepStrategy; -import org.sagebionetworks.research.domain.task.navigation.strategy.StepNavigationStrategy.SkipStepStrategy; - -public interface TestStep extends Step, NextStepStrategy, BackStepStrategy, SkipStepStrategy { } diff --git a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java index e4394be19..4e2ca29ec 100644 --- a/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java +++ b/domain/src/test/java/org/sagebionetworks/research/domain/navigation/TreeNavigatorTests.java @@ -19,8 +19,7 @@ public TreeNavigatorTests() { super(new TreeNavigator(TEST_STEPS, TEST_PROGRESS_MARKERS)); } - // MARK: Test getProgress(). - + // region Test getProgress() @Test public void testProgess_NoMarkers_FlatHierarchy() { List steps = createSteps(new String[]{"1", "2", "3", "4"}); @@ -33,5 +32,6 @@ public void testProgess_NoMarkers_FlatHierarchy() { assertEquals(4, progress.getTotal()); assertTrue(progress.isEstimated()); } + // endregion }