Add RESTful API endpoints for Groups customizations

[victorlin]

Description of proposed changes

Adds RESTful API endpoints for downloading/uploading/removing group overviews and logos.

Related issue(s)

• Closes Add RESTful API endpoints for Groups customizations #547

Save for later

• 34f02d5 from Add RESTful API endpoints for Groups customizations #570 (comment)

Testing

Automated:

• Added test for GET /groups/test/overview removed, see Add RESTful API endpoints for Groups customizations #570 (comment)

Manual:

Local testing from a member of test-private/owners. Maybe automate?

Token setup

nextstrain login
id_token_line=$(grep id_token ~/.nextstrain/secrets)
token=${id_token_line#"id_token = "}

GET private overview works

curl http://localhost:5000/groups/test-private/settings/overview \
    --header "Authorization: Bearer $token" \
    --header 'Accept: text/markdown' \
    --compressed \
        | tee overview.md

DELETE private overview works

curl http://localhost:5000/groups/test-private/settings/overview \
    --header "Authorization: Bearer $token" \
    --request "DELETE"

PUT private overview works

curl http://localhost:5000/groups/test-private/settings/overview \
    --header "Authorization: Bearer $token" \
    --header 'Content-Type: text/markdown' \
    --upload-file overview.md

GET private logo works

curl http://localhost:5000/groups/test-private/settings/logo \
    --header "Authorization: Bearer $token" \
    --header 'Accept: image/png' \
    --compressed > logo.png

DELETE private logo works

curl http://localhost:5000/groups/test-private/settings/logo \
    --header "Authorization: Bearer $token" \
    --request "DELETE"

PUT private logo works

curl http://localhost:5000/groups/test-private/settings/logo \
    --header "Authorization: Bearer $token" \
    --header 'Content-Type: image/png' \
    --upload-file logo.png

Test these since the underlying code was refactored

GET private narrative still works

curl http://localhost:5000/groups/test-private/narratives/foo \
    --header "Authorization: Bearer $token" \
    --header 'Accept: text/markdown' \
    --compressed \
        > narrative.md

DELETE private narrative still works

curl http://localhost:5000/groups/test-private/narratives/victorlin-test \
    --header "Authorization: Bearer $token" \
    --request "DELETE"

PUT private narrative still works

curl http://localhost:5000/groups/test-private/narratives/victorlin-test \
    --header "Authorization: Bearer $token" \
    --header 'Content-Type: text/vnd.nextstrain.narrative+markdown' \
    --upload-file narrative.md

GET private dataset still works

curl http://localhost:5000/groups/test-private/victorlin-test \
    --header "Authorization: Bearer $token" \
    --header 'Accept: application/json' \
    --compressed \
        > dataset.json

DELETE private dataset still works

curl http://localhost:5000/groups/test-private/victorlin-test \
    --header "Authorization: Bearer $token" \
    --request "DELETE"

PUT private dataset still works

curl http://localhost:5000/groups/test-private/victorlin-test \
    --header "Authorization: Bearer $token" \
    --header 'Content-Type: application/vnd.nextstrain.dataset.main+json' \
    --upload-file dataset.json

[victorlin]

@tsibley this is still in early stages, just focused on GET /groups/:groupName/overview. I have some https://github.com/nextstrain/nextstrain.org/labels/request%20for%20comments to make sure I'm doing this right!

[victorlin]

@tsibley progress here is just GET/PUT overview. Would you be able to review before I extend this approach to more endpoints?

[tsibley]

Overall I think this is the right direction!

Left lots of minor notes and comments for your discretion, plus a few actual requested changes.

[tsibley]

BTW, in your testing notes above, the PUT of the overview markdown uses Accept instead of Content-Type.

[victorlin]

BTW, in your testing notes above, the PUT of the overview markdown uses Accept instead of Content-Type.

Good catch, but also this means it worked without the Content-Type header when it shouldn't. Will fix this.

[tsibley]

Ah, good point. Looking closer, that's because the PUT endpoints are using contentTypesProvided instead of contentTypesConsumed.

[victorlin]

@tsibley I added DELETE which was simpler: d70cd69. Could you review before I use these approaches for /groups/:groupName/logo?

[tsibley]

@victorlin Sorry this fell off my radar for a bit. I submitted some comments above (some of which were sitting pending for a week without me realizing they hadn't been submitted :-/). I want to get this across the line soon—it's been dragging out thru no fault of your own—so I'll prioritize more conversation around it. I also need to swap context back in for how it interacts with #581 and your comments there.

[victorlin]

Force-pushing rebase onto latest changes from #583.

@tsibley see 9a1902b...ecca684 which is the new version of e98a3ea...39d4db2 (commits that were shared between this PR + #581). You may want to rebase your PR onto ecca684 and drop the old commits.

EDIT: see #570 (comment) for a better version of this.

[victorlin]

@tsibley see 9a1902b...833ea30 which is the new version of e98a3ea...39d4db2 (commits that were shared between this PR + #581). You may want to rebase your PR onto 833ea30 and drop the old commits.

Notably, 6ef1ec2 has been added so that tests will pass with the circular dependency used in bf7eecf. Context: #570 (comment)

[tsibley]

Does 7184cd7 still apply?

[tsibley]

LGTM! ✨ Sorry for the several dead ends and fits and starts review that turned this into a bit of a slog. 🙏

[victorlin]

@tsibley no worries! Thanks for your helpful comments here, learned a lot during the process!