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

Event upserts via GUIDs should not move an event to another conference #733

Open
lukas2511 opened this issue Sep 5, 2022 · 7 comments
Open

Comments

@lukas2511
Copy link
Member

Sometimes schedules are handcrafted and GUIDs might not be as globally unique as could be assumed by their name.

Currently voctopublish seems to override/patch events of other conferences in case of a non-unique GUID... that's really bad.

It would be good to at least check if the matched existing event is inside the correct conference or otherwise simply error out without overwriting any existing talks.

@Kunsi
Copy link

Kunsi commented Jan 7, 2024

This should be handled in voctoweb IMHO, because voctopublish does not know which conference the existing recording belongs to. Changing a conference by PATCHing a recording should raise an error when calling the API.

I'll move this issue to there.

@Kunsi Kunsi transferred this issue from voc/voctopublish Jan 7, 2024
@saerdnaer saerdnaer changed the title Handle GUIDs more carefully Event upserts via GUIDs should not move an event to another conference Jan 12, 2024
@saerdnaer
Copy link
Member

saerdnaer commented Jan 12, 2024

Currently voctopublish seems to override/patch events of other conferences in case of a non-unique GUID... that's really bad.

Thats actually a feature, but we should add a warning/error when the event would be moved to another conference. The fix in these cases would be to go to the source system and choose a unique uuid.

@lukas2511 Should we add a force option in the Voctoweb Publishing API, which allows moving of an event to another conference or should such operations always be done via the Admin UI?

@lukas2511
Copy link
Member Author

Currently voctopublish seems to override/patch events of other conferences in case of a non-unique GUID... that's really bad.

Thats actually a feature, but we should add a warning/error when the event would be moved to another conference. The fix in these cases would be to go to the source system and choose a unique uuid.

I've seen this issue multiple times with hand crafted schedules. Somebody just copies an existing schedule.xml, modifies human-readable values, maybe adds a video download url and imports it into the tracker. Reusing the guid from the original schedule. And I can't blame anybody for making this mistake. I wouldn't have thought that it starts overwriting talks on media either... So yea. A hard error should happen in those cases. Reusing guids in the same schedule already results in import errors iirc, so that case should already be handled.

@lukas2511 Should we add a force option in the Voctoweb Publishing API, which allows moving of an event to another conference or should such operations always be done via the Admin UI?

Since mass-moving should not happen often (or basically ever) I guess a manual operation via the admin ui or a manually crafted db query would be good enough.

@saerdnaer
Copy link
Member

saerdnaer commented Jan 14, 2024

Reusing guids in the same schedule already results in import errors iirc, so that case should already be handled.

@lukas2511 Where should we add this unique constraint in our systems? In the import tool? In the tracker? In both?

@lukas2511
Copy link
Member Author

Reusing guids in the same schedule already results in import errors iirc, so that case should already be handled.

@lukas2511 Where should we add this unique constraint in our systems? In the import tool? In the tracker? In both?

In voctoweb itself or in the publishing scripts.

@Kunsi
Copy link

Kunsi commented Jan 14, 2024

The publishing scripts are stateless. That kind of error must be handled in c3tracker or in voctoweb.

@lukas2511
Copy link
Member Author

The tracker has absolutely no idea what voctoweb even is, so the only remaining solution would be an error thrown by voctoweb itself.

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

No branches or pull requests

3 participants