Only support downloading Auspice JSONs if dataset name can be parsed

[joverlee521]

Description of proposed changes

Only supports downloading Auspice JSON if the dataset name can be parsed from the URL pathname.
This mainly hides the download Auspice JSON option in auspice.us.

Related issue(s)

Resolves #1803

Checklist

• Checks pass
• If making user-facing changes, add a message in CHANGELOG.md summarizing the changes in this PR
• (to be done by a Nextstrain team member) Create preview PRs on downstream repositories.

[jameshadfield]

datasetNames is always an array of length 2. So for /zika it's [ "zika", undefined ], and thus we end up showing a error notification and not downloading the JSON.

Perhaps the intention here was .all(Boolean)? But that's unnecessary because of the conditions under which auspiceJSON() gets called.

[joverlee521]

Ah, thanks for catching that. I meant .some(Boolean) to verify at least one of them is truthy.

But that's unnecessary because of the conditions under which auspiceJSON() gets called.

Yeah, I added this guard to get error notifications on unexpected use of auspiceJSON() instead of just letting it hang on the "Preparing Auspice JSON..." notification.

[joverlee521]

Fixed up in force-push

[jameshadfield]

Ah, thanks for catching that. I meant .some(Boolean) to verify at least one of them is truthy.

Nice one. I don't know what I meant by suggesting .all, which isn't even an array prototype method!

[jameshadfield]

There's a bug in auspice.us (?) where dragging on two trees results in them being added to the URL. As such, datasetNames contains 2 truthy values, and we allow the download, which then fails.

Using dev_multi_h3n2.json and ebola.json (any two JSONs will do) we get a URL of https://auspice-us-nextstrain-b-q2kkxc.herokuapp.com/dev/multi/h3n2:ebola, and the following error when trying to download:

[joverlee521]

Oh, I did not realize that the URL gets updated in auspice.us when there's a two trees.
Looks like it's coming from the changeURLMiddleware in auspice:

auspice/src/middleware/changeURL.js

Lines 236 to 243 in 6955728

	/* we also double check that if there are 2 trees both are represented
	in the URL */
	if (action.tree.name && action.treeToo && action.treeToo.name && !action.narrative) {
	const treeUrlShouldBe = `${action.tree.name}:${action.treeToo.name}`;
	if (!window.location.pathname.includes(treeUrlShouldBe)) {
	pathname = treeUrlShouldBe;
	}
	}

I assume the URL should never be updated in auspice.us if you are calling this a bug? How is that configured?

[joverlee521]

Hmm, maybe we just shouldn't rely on the URL parsing here to determine if download is supported. We can add a disableAuspiceJsonDownload config param to the extensions and have auspice.us explicitly disable the downloads.

[jameshadfield]

I assume the URL should never be updated in auspice.us if you are calling this a bug? How is that configured?

auspice.us, once it's parsed the dropped files, initialises the Auspice viz via:

  /* load (into redux state) the main dataset(s) */
  try {
    dispatch({
      type: "CLEAN_START",
      pathnameShouldBe: "",
      ...createStateFromQueryOrJSONs({

and the intention is that Auspice updates the pathname (URL) to the empty string via:

auspice/src/middleware/changeURL.js

Lines 231 to 234 in 6955728

	case types.CLEAN_START:
	if (action.pathnameShouldBe && !action.narrative) {
	pathname = action.pathnameShouldBe;
	break;

and then the switch statement ends. But "" is falsey, so we keep going in the switch statement and then, when there are two trees, we hit the case you linked above. I think checking if pathnameShouldBe is a property on the action rather than just the truthyness of its value is the correct fix.

Hmm, maybe we just shouldn't rely on the URL parsing here to determine if download is supported. We can add a disableAuspiceJsonDownload config param to the extensions and have auspice.us explicitly disable the downloads.

I think once the bugs identified here are fixed the URL parsing will still be appropriate. (The particular bug in this thread is, strictly speaking, separate to this PR, but it'd be good to just fix it here.)

[joverlee521]

Updated in a9c4610.

[joverlee521]

Confirmed in auspice.us test app that the URL does not update with multiple trees.

[jameshadfield]

Thanks @joverlee521!