From 613b9a1bfc4b99068562b4caf66115666bc0a1e1 Mon Sep 17 00:00:00 2001
From: Jover <joverlee521@gmail.com>
Date: Wed, 2 Nov 2022 15:14:06 -0700
Subject: [PATCH 1/3] helperFunctions/treeToNewick: use `idxOfFilteredRoot`

In cases where the user has filtered the tree, the `idxOfFilteredRoot`
is the root of the selected tips instead of `idxOfInViewRootNode`.
By using `idxOfFilteredRoot` as the root of the tree, this now acts the
same way as the manual work-around of clicking "Zoom to selected" then
downloading the tree.

However, this does _not_ fix the case where the filter may have created
multiple subtrees. If there are multiple subtrees without a most recent
common ancestor, this function will still produce an empty Newick tree.
---
 src/components/download/helperFunctions.js | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/src/components/download/helperFunctions.js b/src/components/download/helperFunctions.js
index 3a1a65e5e..3f91fbee1 100644
--- a/src/components/download/helperFunctions.js
+++ b/src/components/download/helperFunctions.js
@@ -36,7 +36,12 @@ const treeToNewick = (tree, temporal, internalNodeNames=false, nodeAnnotation=()
     return leaf;
   }
 
-  const rootNode = tree.nodes[tree.idxOfInViewRootNode];
+  /**
+   * Try the filtered root first as this may be different from the in view root node
+   * We still need to fallback on the idxOfInViewRootNode because the idxOfFilteredRoot
+   * is undefined when there are no filters applied.
+   */
+  const rootNode = tree.nodes[tree.idxOfFilteredRoot || tree.idxOfInViewRootNode];
   const rootXVal = getXVal(rootNode);
   return recurse(rootNode, rootXVal) + ";";
 };

From 79d3bb94c21ec14feb8d731fc8eb401e72be7613 Mon Sep 17 00:00:00 2001
From: Jover <joverlee521@gmail.com>
Date: Wed, 2 Nov 2022 17:07:16 -0700
Subject: [PATCH 2/3] Add note that we do not support downloading multiple
 subtrees

---
 src/components/download/downloadButtons.js | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/src/components/download/downloadButtons.js b/src/components/download/downloadButtons.js
index d624c9df2..4c0f57278 100644
--- a/src/components/download/downloadButtons.js
+++ b/src/components/download/downloadButtons.js
@@ -41,6 +41,9 @@ export const DownloadButtons = ({dispatch, t, tree, entropy, metadata, colorBy,
         Downloaded data represents the currently displayed view.
         By zooming the tree, changing the branch-length metric, applying filters etc, the downloaded data will change accordingly.
         <p/>
+        NOTE: We do not support downloads of multiple subtrees, which are usually created with the date range filter or genotype filters.
+        Downloading multiple subtrees will result in an empty Newick tree!
+        <p/>
         {partialData ? `Currently ${selectedTipsCount}/${totalTipCount} tips are displayed and will be downloaded.` : `Currently the entire dataset (${totalTipCount} tips) will be downloaded.`}
       </div>
       <Button

From 4ff518c06869db12242962a86cb84ab646f13ccb Mon Sep 17 00:00:00 2001
From: Jover <joverlee521@gmail.com>
Date: Wed, 2 Nov 2022 17:01:53 -0700
Subject: [PATCH 3/3] Raise an error when an empty tree is created

Raise an error when `treeToNewick` creates an empty tree is created
because this is undesired behavior. Created a custom error class for
this so that we could include details in the warning notification for
the user.
---
 src/components/download/helperFunctions.js | 13 +++++++++++--
 src/util/exceptions.js                     | 12 ++++++++++++
 2 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/src/components/download/helperFunctions.js b/src/components/download/helperFunctions.js
index 3f91fbee1..d586d8695 100644
--- a/src/components/download/helperFunctions.js
+++ b/src/components/download/helperFunctions.js
@@ -6,6 +6,7 @@ import { numericToCalendar } from "../../util/dateHelpers";
 import { NODE_VISIBLE } from "../../util/globals";
 import { datasetSummary } from "../info/datasetSummary";
 import { isColorByGenotype } from "../../util/getGenotype";
+import { EmptyNewickTreeCreated } from "../../util/exceptions";
 
 export const isPaperURLValid = (d) => {
   return (
@@ -43,7 +44,11 @@ const treeToNewick = (tree, temporal, internalNodeNames=false, nodeAnnotation=()
    */
   const rootNode = tree.nodes[tree.idxOfFilteredRoot || tree.idxOfInViewRootNode];
   const rootXVal = getXVal(rootNode);
-  return recurse(rootNode, rootXVal) + ";";
+  const newickTree = recurse(rootNode, rootXVal);
+  if (!newickTree) {
+    throw new EmptyNewickTreeCreated();
+  }
+  return newickTree + ";";
 };
 
 const MIME = {
@@ -334,7 +339,11 @@ export const exportTree = ({dispatch, filePrefix, tree, isNewick, temporal, colo
     dispatch(infoNotification({message: `${temporal ? "TimeTree" : "Tree"} written to ${fName}`}));
   } catch (err) {
     console.error(err);
-    dispatch(warningNotification({message: "Error saving tree!"}));
+    const warningObject = {message: "Error saving tree!"};
+    if (err instanceof EmptyNewickTreeCreated) {
+      warningObject.details = "An empty tree was created. If you have selected genomes, note that we do not support downloads of multiple subtrees.";
+    }
+    dispatch(warningNotification(warningObject));
   }
 };
 
diff --git a/src/util/exceptions.js b/src/util/exceptions.js
index 673e6cec3..0401739cb 100644
--- a/src/util/exceptions.js
+++ b/src/util/exceptions.js
@@ -3,3 +3,15 @@ export class NoContentError extends Error {
     super(...params);
   }
 }
+
+/**
+ * Thrown when a download produces an empty Newick tree.
+ * Usually caused by users trying to download multiple subtrees that do not
+ * share a common ancestor that is "visible"
+ */
+export class EmptyNewickTreeCreated extends Error {
+  constructor(...params) {
+    super(...params);
+    this.name = this.constructor.name;
+  }
+}