From a00403c60e6134ff101f35f93e51f0f99317d870 Mon Sep 17 00:00:00 2001
From: Alok Saldanha <aloksaldanha@gmail.com>
Date: Sun, 21 Aug 2022 08:15:09 -0400
Subject: [PATCH] #73 moved new annotation link to front

---
 cellxgene_gateway/filecrawl.py | 20 +++++------
 tests/test_filecrawl.py        | 66 +++++++++++++++++++++++++++++-----
 2 files changed, 67 insertions(+), 19 deletions(-)

diff --git a/cellxgene_gateway/filecrawl.py b/cellxgene_gateway/filecrawl.py
index f812091..21d8b02 100644
--- a/cellxgene_gateway/filecrawl.py
+++ b/cellxgene_gateway/filecrawl.py
@@ -7,7 +7,7 @@
 # OR CONDITIONS OF ANY KIND, either express or implied. See the License for
 # the specific language governing permissions and limitations under the License.
 
-import os
+import html
 import urllib.parse
 
 from cellxgene_gateway import env, flask_util
@@ -19,19 +19,17 @@ def render_annotations(item, item_source):
     url = flask_util.view_url(
         item_source.get_annotations_subpath(item), item_source.name
     )
-    new_annotation = f"<a class='new' href='{url}'>new</a>"
+    new_annotation = [f"<a class='new' href='{url}'>new</a>"]
+
     annotations = (
-        ", ".join(
-            [
-                f"<a href='{CacheKey(item, item_source, a).view_url}/'>{a.name}</a>"
-                for a in item.annotations
-            ]
-        )
-        + ", "
+        [
+            f"<a href='{CacheKey(item, item_source, a).view_url}/'>{html.escape(a.name)}</a>"
+            for a in item.annotations
+        ]
         if item.annotations
-        else ""
+        else []
     )
-    return " | annotations: " + annotations + new_annotation
+    return "| annotations: " + ", ".join(new_annotation + annotations)
 
 
 def render_item(item, item_source):
diff --git a/tests/test_filecrawl.py b/tests/test_filecrawl.py
index 349d1cc..6adf59b 100644
--- a/tests/test_filecrawl.py
+++ b/tests/test_filecrawl.py
@@ -13,26 +13,76 @@ from cellxgene_gateway.items.item import ItemTree, ItemType
 source = FileItemSource("/tmp")
 
 
+def make_entry(subpath="somepath", annotations=None):
+    return FileItem(
+        subpath=subpath,
+        name="entry",
+        ext=".h5ad",
+        type=ItemType.h5ad,
+        annotations=annotations,
+    )
+
+
 class TestRenderEntry(unittest.TestCase):
     def test_GIVEN_path_both_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_starts_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="/somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="/somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_ends_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath/", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath/")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
 
     def test_GIVEN_path_no_slash_THEN_view_has_single_slash(self):
-        entry = FileItem(subpath="somepath", name="entry", type=ItemType.h5ad)
+        entry = make_entry(subpath="somepath")
         rendered = render_item(entry, source)
-        self.assertIn("view/somepath/entry/'", rendered)
+        self.assertIn("view/somepath/entry.h5ad/'", rendered)
+
+
+class TestRenderAnnotation(unittest.TestCase):
+    def test_GIVEN_no_annotation_THEN_new_alone(self):
+        entry = make_entry(annotations=None)
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_new_before(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="annot",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/annot.csv/'>annot</a></li>",
+            rendered,
+        )
+
+    def test_GIVEN_annotation_THEN_escaped(self):
+        annotation = FileItem(
+            subpath="somepath/entry_annotations",
+            name="hot&cold",
+            ext=".csv",
+            type=ItemType.annotation,
+        )
+        entry = make_entry(annotations=[annotation])
+        rendered = render_item(entry, source)
+        self.assertIn(
+            "> | annotations: <a class='new' href='/source/Files:/tmp/view/somepath/entry_annotations'>new</a>,"
+            " <a href='/source/Files:/tmp/view/somepath/entry_annotations/hot%26cold.csv/'>hot&amp;cold</a></li>",
+            rendered,
+        )
 
 
 class TestRenderItemSource(unittest.TestCase):