SONAR-5056 Fix issue on move up/down

6 files changed, 48 insertions, 25 deletions

diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java
index aa79358d0ad..da97cadd0d3 100644
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelLookup.java
@@ -47,7 +47,7 @@ public class DebtModelLookup implements ServerComponent {
     return toCharacteristics(dao.selectEnabledRootCharacteristics());
   }
 
-  public List<DebtCharacteristic> characteristics() {
+  public List<DebtCharacteristic> allCharacteristics() {
     return toCharacteristics(dao.selectEnabledCharacteristics());
   }
 
diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
index 871f777a894..4b7c6004746 100644
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelOperations.java
@@ -21,6 +21,8 @@
 package org.sonar.server.debt;
 
 import com.google.common.annotations.VisibleForTesting;
+import com.google.common.base.Predicate;
+import com.google.common.collect.Iterables;
 import org.apache.ibatis.session.SqlSession;
 import org.sonar.api.ServerComponent;
 import org.sonar.api.server.debt.DebtCharacteristic;
@@ -126,14 +128,25 @@ public class DebtModelOperations implements ServerComponent {
 
     SqlSession session = mybatis.openSession();
     try {
-      CharacteristicDto dto = findCharacteristic(characteristicId, session);
+      final CharacteristicDto dto = findCharacteristic(characteristicId, session);
       int currentOrder = dto.getOrder();
-      CharacteristicDto dtoToSwitchOrderWith = moveUpOrDown ? dao.selectPrevious(currentOrder, session) : dao.selectNext(currentOrder, session);
 
-      // Do nothing when characteristic is already to the new location
-      if (dtoToSwitchOrderWith == null) {
+      // characteristics should be order by 'order'
+      List<CharacteristicDto> rootCharacteristics = dao.selectEnabledRootCharacteristics(session);
+      int currentPosition = Iterables.indexOf(rootCharacteristics, new Predicate<CharacteristicDto>() {
+        @Override
+        public boolean apply(CharacteristicDto input) {
+          return input.getKey().equals(dto.getKey());
+        }
+      });
+      Integer nextMove = moveUpOrDown ? (currentPosition > 0 ? currentPosition - 1 : null) : (currentPosition < rootCharacteristics.size()-1 ? currentPosition + 1 : null);
+
+      // Do nothing when characteristic is already to the good location
+      if (nextMove == null) {
         return toCharacteristic(dto);
       }
+
+      CharacteristicDto dtoToSwitchOrderWith = Iterables.get(rootCharacteristics, nextMove);
       int nextOrder = dtoToSwitchOrderWith.getOrder();
       dtoToSwitchOrderWith.setOrder(currentOrder);
       dtoToSwitchOrderWith.setUpdatedAt(new Date(system2.now()));
diff --git a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
index c21e1d16b8a..483f4fa5c28 100644
--- a/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
+++ b/sonar-server/src/main/java/org/sonar/server/debt/DebtModelService.java
@@ -44,12 +44,12 @@ public class DebtModelService implements DebtModel {
     this.debtModelRestore = debtModelRestore;
   }
 
-  public List<DebtCharacteristic> rootCharacteristics() {
+  public List<DebtCharacteristic> characteristics() {
     return debtModelLookup.rootCharacteristics();
   }
 
-  public List<DebtCharacteristic> characteristics() {
-    return debtModelLookup.characteristics();
+  public List<DebtCharacteristic> allCharacteristics() {
+    return debtModelLookup.allCharacteristics();
   }
 
   @CheckForNull
diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java
index 6c72ca00e31..eedc79f38af 100644
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelLookupTest.java
@@ -60,9 +60,9 @@ public class DebtModelLookupTest {
   }
 
   @Test
-  public void find_characteristics() {
+  public void find_all_characteristics() {
     when(dao.selectEnabledCharacteristics()).thenReturn(newArrayList(characteristicDto));
-    assertThat(service.characteristics()).hasSize(1);
+    assertThat(service.allCharacteristics()).hasSize(1);
   }
 
   @Test
diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
index cc8400499a1..f3ab22fe6b3 100644
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelOperationsTest.java
@@ -108,7 +108,7 @@ public class DebtModelOperationsTest {
     }).when(dao).insert(any(CharacteristicDto.class), any(SqlSession.class));
 
     when(mybatis.openSession()).thenReturn(session);
-    service = new DebtModelOperations(mybatis, dao, ruleDao,system2);
+    service = new DebtModelOperations(mybatis, dao, ruleDao, system2);
   }
 
   @Test
@@ -234,8 +234,11 @@ public class DebtModelOperationsTest {
 
   @Test
   public void move_up() {
-    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setOrder(2));
-    when(dao.selectPrevious(2, session)).thenReturn(new CharacteristicDto().setId(2).setOrder(1));
+    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2));
+    when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList(
+      new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(1),
+      new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2)
+    ));
 
     DebtCharacteristic result = service.moveUp(10);
 
@@ -251,9 +254,11 @@ public class DebtModelOperationsTest {
 
   @Test
   public void do_nothing_when_move_up_and_already_on_top() {
-    CharacteristicDto dto = new CharacteristicDto().setId(10).setOrder(1);
-    when(dao.selectById(10, session)).thenReturn(dto);
-    when(dao.selectPrevious(1, session)).thenReturn(null);
+    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(1));
+    when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList(
+      new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(1),
+      new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(2)
+    ));
 
     service.moveUp(10);
 
@@ -262,8 +267,11 @@ public class DebtModelOperationsTest {
 
   @Test
   public void move_down() {
-    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setOrder(2));
-    when(dao.selectNext(2, session)).thenReturn(new CharacteristicDto().setId(2).setOrder(3));
+    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2));
+    when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList(
+      new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2),
+      new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(3)
+    ));
 
     DebtCharacteristic result = service.moveDown(10);
 
@@ -279,9 +287,11 @@ public class DebtModelOperationsTest {
 
   @Test
   public void do_nothing_when_move_down_and_already_on_bottom() {
-    CharacteristicDto dto = new CharacteristicDto().setId(10).setOrder(5);
-    when(dao.selectById(10, session)).thenReturn(dto);
-    when(dao.selectNext(5, session)).thenReturn(null);
+    when(dao.selectById(10, session)).thenReturn(new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2));
+    when(dao.selectEnabledRootCharacteristics(session)).thenReturn(newArrayList(
+      new CharacteristicDto().setId(2).setKey("PORTABILITY").setOrder(1),
+      new CharacteristicDto().setId(10).setKey("MEMORY_EFFICIENCY").setOrder(2)
+    ));
 
     service.moveDown(10);
 
diff --git a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
index e02bb0133a3..9c044e6508a 100644
--- a/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
+++ b/sonar-server/src/test/java/org/sonar/server/debt/DebtModelServiceTest.java
@@ -48,14 +48,14 @@ public class DebtModelServiceTest {
 
   @Test
   public void find_root_characteristics() {
-    service.rootCharacteristics();
+    service.characteristics();
     verify(debtModelLookup).rootCharacteristics();
   }
 
   @Test
-  public void find_characteristics() {
-    service.characteristics();
-    verify(debtModelLookup).characteristics();
+  public void find_all_characteristics() {
+    service.allCharacteristics();
+    verify(debtModelLookup).allCharacteristics();
   }
 
   @Test