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.

Sign up for GitHub

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

Jump to bottom

ProposalC3VOCPublishingWebhook: add option to set thumbnail as well (and test naming) #1774

Merged
merged 3 commits into from
Sep 1, 2024
Merged

ProposalC3VOCPublishingWebhook: add option to set thumbnail as well (and test naming) #1774

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
ec6be7a
api.schedule.ProposalC3VOCPublishingWebhook: add option to set thumbn…
Kunsi Sep 1, 2024
86a6a78
tests/test_api_c3voc: better test naming
Kunsi Sep 1, 2024
648ce0b
Merge branch 'main' into kunsi-c3voc-publishing-webhook-fixup
russss Sep 1, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions apps/api/schedule.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -216,6 +216,20 @@ def post(self):
)
proposal.c3voc_url = None

if payload["voctoweb"]["thumb_path"]:
path = payload["voctoweb"]["thumb_path"]
if path.startswith("/static.media.ccc.de"):
path = "https://static.media.ccc.de/media" + path[len("/static.media.ccc.de"):]
if not path.startswith("https://"):
Copy link
Contributor

Choose a reason for hiding this comment

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

Are other CDNs likely to be used to host these thumbnails? (just wondering about the broader URI acceptance logic in this case)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This URL has changed in the past (some time ago it was cdn.c3voc.de i think). I don't think this will change in the future though, so we could have implemented stricter checking here, you're right.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks, OK! For consistency and safety, I think it makes sense to check the complete domain in that case too.

abort(406, message="voctoweb thumb_path must start with https:// or /static.media.ccc.de")
app.logger.info(f"C3VOC webhook set thumbnail_url for {proposal.id=} to {path}")
proposal.thumbnail_url = path
else:
app.logger.warning(
f"C3VOC webhook cleared thumbnail_url for {proposal.id=}, was {proposal.thumbnail_url}"
)
proposal.thumbnail_url = None

if payload["youtube"]["enabled"]:
if payload["youtube"]["urls"]:
# Please do not overwrite existing youtube urls
Expand Down
172 changes: 162 additions & 10 deletions tests/test_api_c3voc.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,10 +17,11 @@ def proposal(db, user):
return proposal


def clean_proposal(db, proposal, c3voc_url=None, youtube_url=None, video_recording_lost=True):
def clean_proposal(db, proposal, c3voc_url=None, youtube_url=None, thumbnail_url=None, video_recording_lost=True):
proposal.c3voc_url = c3voc_url
proposal.youtube_url = youtube_url
proposal.thumbnail_url = thumbnail_url
proposal.video_recording_lost = video_recording_lost
proposal.youtube_url = youtube_url
db.session.add(proposal)
db.session.commit()

Expand Down Expand Up @@ -109,7 +110,7 @@ def test_denies_request_wrong_year(client, app, proposal):
assert rv.status_code == 422


def test_request_none_update_none(client, app, db, proposal):
def test_request_none_unchanged(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand Down Expand Up @@ -144,7 +145,7 @@ def test_request_none_update_none(client, app, db, proposal):
assert proposal.c3voc_url is None


def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal):
def test_update_voctoweb_with_correct_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand All @@ -167,6 +168,7 @@ def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal)
"voctoweb": {
"enabled": True,
"frontend_url": "https://media.ccc.de/",
"thumb_path": "",
},
"youtube": {
"enabled": False,
Expand All @@ -181,7 +183,7 @@ def test_request_voctoweb_update_voctoweb_correct_url(client, app, db, proposal)
assert proposal.youtube_url is None


def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal):
def test_denies_voctoweb_with_wrong_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand All @@ -204,6 +206,7 @@ def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal):
"voctoweb": {
"enabled": True,
"frontend_url": "https://example.org",
"thumb_path": "",
},
"youtube": {
"enabled": False,
Expand All @@ -218,7 +221,7 @@ def test_request_voctoweb_update_voctoweb_wrong_url(client, app, db, proposal):
assert proposal.c3voc_url == "https://example.com"


def test_request_voctoweb_clears_voctoweb(client, app, db, proposal):
def test_clears_voctoweb(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand All @@ -241,6 +244,81 @@ def test_request_voctoweb_clears_voctoweb(client, app, db, proposal):
"voctoweb": {
"enabled": True,
"frontend_url": "",
"thumb_path": "",
},
"youtube": {
"enabled": False,
},
},
)
assert rv.status_code == 204

proposal = Proposal.query.get(proposal.id)
assert proposal.c3voc_url is None


def test_update_thumbnail_with_path(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
}
)

clean_proposal(db, proposal)
jayaddison marked this conversation as resolved.
Show resolved Hide resolved

rv = client.post(
f"/api/proposal/c3voc-publishing-webhook",
headers={
"Authorization": "Bearer api-key",
},
json={
"is_master": True,
"fahrplan": {
"conference": f"emf{event_year()}",
"id": proposal.id,
},
"voctoweb": {
"enabled": True,
"frontend_url": "",
"thumb_path": "/static.media.ccc.de/thumb.jpg",
},
"youtube": {
"enabled": False,
},
},
)
assert rv.status_code == 204

proposal = Proposal.query.get(proposal.id)
assert proposal.thumbnail_url == "https://static.media.ccc.de/media/thumb.jpg"
assert proposal.c3voc_url is None
assert proposal.youtube_url is None


def test_update_thumbnail_with_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
}
)

clean_proposal(db, proposal)

rv = client.post(
f"/api/proposal/c3voc-publishing-webhook",
headers={
"Authorization": "Bearer api-key",
},
json={
"is_master": True,
"fahrplan": {
"conference": f"emf{event_year()}",
"id": proposal.id,
},
"voctoweb": {
"enabled": True,
"frontend_url": "",
"thumb_path": "https://example.com/thumb.jpg",
},
"youtube": {
"enabled": False,
Expand All @@ -250,10 +328,84 @@ def test_request_voctoweb_clears_voctoweb(client, app, db, proposal):
assert rv.status_code == 204

proposal = Proposal.query.get(proposal.id)
assert proposal.thumbnail_url == "https://example.com/thumb.jpg"
assert proposal.c3voc_url is None
assert proposal.youtube_url is None


def test_denies_thumbnail_not_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
}
)

clean_proposal(db, proposal, thumbnail_url="https://example.com/thumb.jpg")

rv = client.post(
f"/api/proposal/c3voc-publishing-webhook",
headers={
"Authorization": "Bearer api-key",
},
json={
"is_master": True,
"fahrplan": {
"conference": f"emf{event_year()}",
"id": proposal.id,
},
"voctoweb": {
"enabled": True,
"frontend_url": "",
"thumb_path": "/example.com/thumb.jpg",
},
"youtube": {
"enabled": False,
},
},
)
assert rv.status_code == 406

proposal = Proposal.query.get(proposal.id)
assert proposal.thumbnail_url == "https://example.com/thumb.jpg"


def test_clears_thumbnail(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
}
)

clean_proposal(db, proposal, thumbnail_url="https://example.com/thumb.jpg")

rv = client.post(
f"/api/proposal/c3voc-publishing-webhook",
headers={
"Authorization": "Bearer api-key",
},
json={
"is_master": True,
"fahrplan": {
"conference": f"emf{event_year()}",
"id": proposal.id,
},
"voctoweb": {
"enabled": True,
"frontend_url": "",
"thumb_path": "",
},
"youtube": {
"enabled": False,
},
},
)
assert rv.status_code == 204

proposal = Proposal.query.get(proposal.id)
assert proposal.thumbnail_url is None


def test_request_youtube_update_youtube_correct_url(client, app, db, proposal):
def test_update_youtube_with_correct_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand Down Expand Up @@ -292,7 +444,7 @@ def test_request_youtube_update_youtube_correct_url(client, app, db, proposal):
assert proposal.youtube_url == "https://www.youtube.com/watch"


def test_request_youtube_update_youtube_correct_url_but_existing_url(client, app, db, proposal):
def test_denies_youtube_update_with_exisiting_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand Down Expand Up @@ -331,7 +483,7 @@ def test_request_youtube_update_youtube_correct_url_but_existing_url(client, app
assert proposal.youtube_url == "https://example.com"


def test_request_youtube_update_youtube_wrong_url(client, app, db, proposal):
def test_denies_youtube_update_with_wrong_url(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand Down Expand Up @@ -370,7 +522,7 @@ def test_request_youtube_update_youtube_wrong_url(client, app, db, proposal):
assert proposal.youtube_url == "https://example.com"


def test_request_youtube_clears_youtube(client, app, db, proposal):
def test_clears_youtube(client, app, db, proposal):
app.config.update(
{
"VIDEO_API_KEY": "api-key",
Expand Down