Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

api.schedule: add C3VOCPublishingWebhook #1770

Merged
merged 12 commits into from
Sep 1, 2024

Conversation

Kunsi
Copy link
Contributor

@Kunsi Kunsi commented Aug 31, 2024

This will allow c3voc to send us video information automatically upon release of videos, as documented here: https://github.com/voc/voctopublish/blob/main/voctopublish/api_client/webhook_client.py#L28-L56

@Kunsi Kunsi marked this pull request as ready for review August 31, 2024 11:51
@Kunsi
Copy link
Contributor Author

Kunsi commented Aug 31, 2024

Tested:

# no json content-type
/home/kunsi➤ curl -X POST -H"Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook
{
    "message": "The server does not support the media type transmitted in the request."
}

# no json data
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.
138.22:2342/api/c3voc/publishing-webhook
{
    "message": "Failed to decode JSON object: Expecting value: line 1 column 1 (char 0)"
}

# empty json
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook --data '{}'
{
    "message": "The request was well-formed but was unable to be followed due to semantic errors."
}

# no master ticket
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook --data '{"is_master": false, "fahrplan": {"conference": "emf2024", "id": 12}}'
{
    "message": "The resource identified by the request is only capable of generating response entities which have content characteristics not acceptable according to the accept headers sent in the request."
}

# sets youtube url only
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook --data '{"is_master": true, "fahrplan": {"conference": "emf2024", "id": 14}, "voctoweb": {"enabled": false}, "youtube": {"enabled": true, "urls": ["https://www.youtube.com/watch?v=hsoiHKaqG2s"]}}'

# sets both youtube url and media.ccc.de url
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook --data '{"is_master": true, "fahrplan": {"conference": "emf2024", "id": 14}, "voctoweb": {"enabled": true, "frontend_url": "https://media.ccc.de/v/eh21-61-state-of-bahn-reisendeninformation"}, "youtube": {"enabled": true, "urls": ["https://www.youtube.com/watch?v=hsoiHKaqG2s"]}}'

# unsets youtube url
/home/kunsi➤ curl -X POST -H "Content-Type: application/json" -H "Authorization: Bearer video-api-token" http://172.19.138.22:2342/api/c3voc/publishing-webhook --data '{"is_master": true, "fahrplan": {"conference": "emf2024", "id": 14}, "voctoweb": {"enabled": true, "frontend_url": "https://media.ccc.de/v/eh21-61-state-of-bahn-reisendeninformation"}, "youtube": {"enabled": false}}'

apps/api/schedule.py Outdated Show resolved Hide resolved
apps/api/schedule.py Outdated Show resolved Hide resolved
apps/api/schedule.py Outdated Show resolved Hide resolved
apps/api/schedule.py Outdated Show resolved Hide resolved
apps/api/schedule.py Outdated Show resolved Hide resolved
@Kunsi Kunsi force-pushed the kunsi-c3voc-publishing-webhook branch from 57c4183 to 106545d Compare August 31, 2024 13:16
@Kunsi Kunsi force-pushed the kunsi-c3voc-publishing-webhook branch from 106545d to bff1726 Compare August 31, 2024 13:28
@jayaddison

This comment was marked as outdated.

jayaddison

This comment was marked as outdated.

@jayaddison

This comment was marked as resolved.

apps/api/schedule.py Outdated Show resolved Hide resolved
apps/api/schedule.py Outdated Show resolved Hide resolved
Copy link
Contributor

@jayaddison jayaddison left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me 👍

My two remaining nitpicks would be:

  • We don't have much info-level logging in the API modules of the codebase so far, so I don't know whether we should include those.
  • The try...catch for KeyError seems overbroad to me; but I'm unaware of any particular problem that would cause.

@jayaddison

This comment was marked as resolved.

@Kunsi Kunsi force-pushed the kunsi-c3voc-publishing-webhook branch from 98c1eae to ac82add Compare August 31, 2024 19:53
@jayaddison
Copy link
Contributor

Another one-more-thing: we should probably add some test coverage for this. I'm prety convinced from manually testing it that it works well, but for the longer-term, some automated coverage would be nice. Borrowing some of the existing test code for the PATCH endpoint could provide a base for that.

@Kunsi Kunsi force-pushed the kunsi-c3voc-publishing-webhook branch from 46e80fd to 1a5c786 Compare September 1, 2024 07:03
@Kunsi Kunsi force-pushed the kunsi-c3voc-publishing-webhook branch from 1a5c786 to 97ebe3a Compare September 1, 2024 07:43
@Kunsi Kunsi mentioned this pull request Sep 1, 2024
@Kunsi
Copy link
Contributor Author

Kunsi commented Sep 1, 2024

I think my tests cover all the cases we'd want to test. Let me know if i missed any.

@russss russss merged commit f6116cc into emfcamp:main Sep 1, 2024
3 checks passed
@Kunsi Kunsi deleted the kunsi-c3voc-publishing-webhook branch September 1, 2024 12:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants