Replace Datasets DatasetSelect component on groups page with ListResources [#870]

[genehack]

Description of proposed changes

Replaces the current "datasets" DatasetSelect component with a ListResources component that hits the same backend charon API. Also refactors in useDataFetch to change the hard-coded parser into a passed-in (mandatory) callback function, and updates two existing uses to the new pattern. Finally, adds in conditional display logic to the ListResources component to hide or otherwise not set UI elements that don't make sense with the groups dataset (e.g., last modified dates, since we don't have those).

Preview

Related issue(s)

870

Checklist

• [x] Checks pass
• [x] Check if changes affect the resource index JSON revision

[genehack]

Note: CI / build will be broken until TS lint changes PR lands.

[victorlin]

Note: CI / build will be broken until TS lint changes PR lands.

This blockage doesn't seem necessary/productive. Attempting removal with #909

[genehack]

Note: CI / build will be broken until TS lint changes PR lands.

This blockage doesn't seem necessary/productive. Attempting removal with #909

it's not necessary, in that i could add lines to disable the couple lint failures -- but then those would need to be removed once #906 lands, and that feels like unproductive churn to me.

(i don't have a strong opinion about the direction #909 moves things in; i think linting being mildly annoying when broken is maybe a good thing…)

[victorlin]

@genehack continuing the tangential discussion in https://github.com/nextstrain/nextstrain.org/pull/909#issuecomment-2159141269

[genehack]

@genehack continuing the tangential discussion in #909 (comment)

👍

I merged #906 so that should get this unblocked soon (he says, as he discovers another linting problem...) — I will link a preview link once it builds.

[genehack]

Preview — note that only blab group data is being returned by the charon/getAvailable call in the staging/preview environment, and I'm not sure why that isn't reflected in the group tiles in the preview…

[genehack]

Replicating a f2f chat I had (separately) with both Victor and James:

The new updates are better, but I still see some benefits to having fetch in callback.

Originally I was thinking all the callbacks would be using fetch(), not understanding some would be static data structures. Knowing this, I agree there's some benefit to internalizing more responsibility into the callback.

Reviewing Victor's draft version of this, the callbacks relying on importing a fetch helper/wrapper out of useDataFetch seemed a bit too pretzel-y to me. I'm going to try a version where the pathogens and staging pages duplicate the ~4-6 lines of fetch() logic, and where the general signature of the callback is it gets passed the setDataError state callback. I think that will ultimately loosen the coupling between the caller and the callback in a way that allows sufficient flexibility to cover all current, and many future, use cases.

[genehack]

Okay, updated: callbacks now handle the fetch(); general other cleanups and fixups.

Previews

• groups
• pathogens
• staging

[tsibley]

note that only blab group data is being returned by the charon/getAvailable call in the staging/preview environment, and I'm not sure why that isn't reflected in the group tiles in the preview…

Maybe this was addressed in a meeting off-thread you had, but since I didn't see mention of it I wanted to clarify this for you.

Heroku review apps run in production mode with production config, but we've intentionally configured them (on the Heroku side) to use the nextstrain.org-testing AWS IAM user, which has a much more restrictive policy attached to it than the actual production IAM user (nextstrain.org)'s attached policy.

[genehack]

note that only blab group data is being returned by the charon/getAvailable call in the staging/preview environment, and I'm not sure why that isn't reflected in the group tiles in the preview…

Maybe this was addressed in a meeting off-thread you had, but since I didn't see mention of it I wanted to clarify this for you.

Heroku review apps run in production mode with production config, but we've intentionally configured them (on the Heroku side) to use the nextstrain.org-testing AWS IAM user, which has a much more restrictive policy attached to it than the actual production IAM user (nextstrain.org)'s attached policy.

nope, nobody else has shared this; thanks, this makes sense.

[trvrb]

Makes sense that this is just showing blab on the preview app, but the wider blab dataset list is a bit funny in terms of collapsing. The amount of content shown should be relative to the how many lines there are. Currently the blab dataset list is definitely too long even in collapsed view.

Ie collapsed height should be consistent across cards regardless of whether specific cards use one column, two columns or three columns.

[genehack]

Makes sense that this is just showing blab on the preview app, but the wider blab dataset list is a bit funny in terms of collapsing. The amount of content shown should be relative to the how many lines there are. Currently the blab dataset list is definitely too long even in collapsed view.

I will look at what's going on here. Interestingly, if I point my local dev instance at the production data endpoint (i.e., if I fetch the data fram https://nextstrain.org/charon/getAvailable?prefix=/groups/), this does not happen:

I wonder if the oddness in the display is because of the smaller data set size available in the preview environment?

[genehack]

I wonder if the oddness in the display is because of the smaller data set size available in the preview environment?

Experimenting further, if I take the production data but filter it down to just 2 cards worth of data, the cards I get are large. Expanding that to 3 cards, the layout is as expected.

Two groups:

Three groups:

[genehack]

I wonder if the oddness in the display is because of the smaller data set size available in the preview environment?

Experimenting further, if I take the production data but filter it down to just 2 cards worth of data, the cards I get are large. Expanding that to 3 cards, the layout is as expected.

Digging ever further, this seems to be intentional behavior. @jameshadfield could you speak to this?

I'm going to say I don't think this should be something that blocks this PR; if we do want to change this behavior, it can be a distinct thing.

[jameshadfield]

Digging ever further, this seems to be intentional behavior. @jameshadfield could you speak to this?

The intention is that if few cards are in view (commonly achieved via filtering) then we should show more entries per card. The exact thresholds are (as always) open to change. We could (probably should) also remove the expand/contract toggle if there is only 1 card.