From f5c1419b96dc540087bcf565140e6bbcdc2096aa Mon Sep 17 00:00:00 2001
From: gmungoc <g.m.carstairs@dundee.ac.uk>
Date: Tue, 9 May 2017 09:25:49 +0100
Subject: [PATCH] JAL-2483 JAL-2481 fixed findAllFeatures to handle null group
 correctly

---
 .../seqfeatures/FeatureRendererModel.java          |   23 +++++++++++++-------
 .../renderer/seqfeatures/FeatureRendererTest.java  |   16 +++++++++-----
 2 files changed, 26 insertions(+), 13 deletions(-)

diff --git a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
index 5d817c7..5bbdbec 100644
--- a/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
+++ b/src/jalview/viewmodel/seqfeatures/FeatureRendererModel.java
@@ -339,15 +339,22 @@ public abstract class FeatureRendererModel implements
       SequenceI asq = alignment.getSequenceAt(i);
       for (String group : asq.getFeatures().getFeatureGroups(true))
       {
-        if (group == null)
+        /*
+         * features in null group are always displayed; other groups
+         * keep their current visibility; new groups as 'newMadeVisible'
+         */
+        boolean groupDisplayed = true;
+        if (group != null)
         {
-          continue;
-        }
-        Boolean groupDisplayed = featureGroups.get(group);
-        if (groupDisplayed == null)
-        {
-          groupDisplayed = Boolean.valueOf(newMadeVisible);
-          featureGroups.put(group, groupDisplayed);
+          if (featureGroups.containsKey(group))
+          {
+            groupDisplayed = featureGroups.get(group);
+          }
+          else
+          {
+            groupDisplayed = newMadeVisible;
+            featureGroups.put(group, groupDisplayed);
+          }
         }
         if (groupDisplayed)
         {
diff --git a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
index 5bda316..ab5c137 100644
--- a/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
+++ b/test/jalview/renderer/seqfeatures/FeatureRendererTest.java
@@ -63,16 +63,21 @@ public class FeatureRendererTest
     seqs.get(2).addSequenceFeature(
             new SequenceFeature("Rfam", "Desc", 5, 9, Float.NaN,
                     "RfamGroup"));
+    // existing feature type with null group
     seqs.get(3).addSequenceFeature(
             new SequenceFeature("Rfam", "Desc", 5, 9, Float.NaN, null));
+    // new feature type with null group
+    seqs.get(3).addSequenceFeature(
+            new SequenceFeature("Scop", "Desc", 5, 9, Float.NaN, null));
     // null value for type produces NullPointerException
     fr.findAllFeatures(true);
     types = fr.getRenderOrder();
     groups = fr.getFeatureGroups();
-    assertEquals(types.size(), 2);
+    assertEquals(types.size(), 3);
     assertFalse(types.contains("Type"));
     assertTrue(types.contains("Pfam"));
     assertTrue(types.contains("Rfam"));
+    assertTrue(types.contains("Scop"));
     assertEquals(groups.size(), 2);
     assertFalse(groups.contains("Group"));
     assertTrue(groups.contains("PfamGroup"));
@@ -104,18 +109,19 @@ public class FeatureRendererTest
      * check render order (last is on top)
      */
     List<String> renderOrder = fr.getRenderOrder();
-    assertEquals(renderOrder, Arrays.asList("Rfam", "Pfam"));
+    assertEquals(renderOrder, Arrays.asList("Scop", "Rfam", "Pfam"));
 
     /*
      * change render order (todo: an easier way)
      * nb here last comes first in the data array
      */
-    Object[][] data = new Object[2][];
+    Object[][] data = new Object[3][];
     FeatureColourI colour = new FeatureColour(Color.RED);
     data[0] = new Object[] { "Rfam", colour, true };
     data[1] = new Object[] { "Pfam", colour, false };
+    data[2] = new Object[] { "Scop", colour, false };
     fr.setFeaturePriority(data);
-    assertEquals(fr.getRenderOrder(), Arrays.asList("Pfam", "Rfam"));
+    assertEquals(fr.getRenderOrder(), Arrays.asList("Scop", "Pfam", "Rfam"));
     assertEquals(fr.getDisplayedFeatureTypes(), Arrays.asList("Rfam"));
 
     /*
@@ -126,7 +132,7 @@ public class FeatureRendererTest
             new SequenceFeature("Metal", "Desc", 14, 22, 8f, "MetalGroup"));
     fr.findAllFeatures(true);
     assertEquals(fr.getRenderOrder(),
-            Arrays.asList("Pfam", "Rfam", "Metal"));
+            Arrays.asList("Scop", "Pfam", "Rfam", "Metal"));
     assertEquals(fr.getDisplayedFeatureTypes(),
             Arrays.asList("Rfam", "Metal"));
   }
-- 
1.7.10.2