nextstrain / nextstrain.org

The Nextstrain website
https://nextstrain.org
GNU Affero General Public License v3.0
89 stars 49 forks source link

Redirects for partial dataset paths in non-core/staging sources? #40

Open rneher opened 5 years ago

rneher commented 5 years ago

if community builds have a hierarchical structure (e.g. h3n2_ha_2y), this is parsed nicely into a selector menu. But changing anything but the last one crashes because of missing defaults or redirects. See for example here:

https://nextstrain-dev.herokuapp.com/community/rivm-syso/Dutch-NIC-EMC-RIVM/h3n2/ha/2y

using the menu to switch fro, h3n2 -> vic redirects to

https://nextstrain-dev.herokuapp.com/community/rivm-syso/Dutch-NIC-EMC-RIVM/vic

which isn't completed into a full path. @jameshadfield: does this have an easy fix?

jameshadfield commented 5 years ago

It's definitely doable -- I can look into this a bit more after v2 is released.

Nextstrain.org already has the potential to tell auspice to "redirect" -- e.g. nextstrain.org/flu goes to nextstrain.org/flu/seasonal/h3n2/ha/2y. We just need to code the logic for community redirects.

I'm guessing that in this case you'd expect it to redirect to /vic/ha/2y?

rneher commented 5 years ago

I am not sure whether things have changed, but we used to have a "default" at each level in the hierarchy of the available datasets json. this was used to complete a partial to a full path.

jameshadfield commented 5 years ago

I am not sure whether things have changed, but we used to have a "default" at each level in the hierarchy of the available datasets json. this was used to complete a partial to a full path.

Yes, that's true (implemented here) but there's no concept of a manifest for community builds.

I would propose modifying auspice to send some metadata attached to the getDataset call which includes both the new request (Dutch-NIC-EMC-RIVM/vic, all that's currently sent) and the "current" page (Dutch-NIC-EMC-RIVM/h3n2/ha/2y). The server, finding that the former doesn't exist could try to add on "ha/2y" and see if that exists.

Happy to have a crack at this once I have time.

Note that this is conceptually the same as https://github.com/nextstrain/auspice/issues/698

tsibley commented 2 years ago

Supporting redirects (i.e. resolution of partial dataset paths) for non-core/staging sources would take substantial consideration of how that information is stored (a manifest file, but we'd like to get rid of those; a separate hypothetical redirects.yaml file stored on the source?) and how we avoid negative impacts on our routing perf. when reading/using that information. These are surmountable, for sure, but substantial work to support and it still leaves users left to configure it. It's not clear at all to me that it's worth it.

However, during issue triage just now, @jameshadfield smartly noted that instead of supporting this sort of dataset resolution as the fix for the dataset switching bug @rneher noted, we could instead improve/replace the hierarchical selector UI to entirely sidestep this issue. This seems much more fruitful to me!

joverlee521 commented 6 months ago

Pinging this issue as a Nextstrain Groups user also ran into this redirect error