Skip to content

Fix playlist sync db issue #1366

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

Merged
merged 7 commits into from
Jun 4, 2025
Merged

Fix playlist sync db issue #1366

merged 7 commits into from
Jun 4, 2025

Conversation

ferishili
Copy link
Contributor

This PR fixes #1255, which enhances playlist migration by enforcing foreign key constraints and make sure the non-nullable columns changes are correctly applied!

PS: the testing environement was a bit confusing for me to make sure that it is working properly, so please test this through!

@ferishili ferishili requested a review from tgloeggl May 12, 2025 12:45
@ferishili ferishili self-assigned this May 12, 2025
@ferishili ferishili added type:bug v:3 Everything related to the Stud.IP Opencast Plugin Version 3.x labels May 12, 2025
@dennis531
Copy link
Collaborator

dennis531 commented May 13, 2025

It seems that the playlists migration step fails in the tests:

image

With the change in #1368, the curl command should fail on errors.

@ferishili
Copy link
Contributor Author

ferishili commented May 22, 2025

The tests are green now!
However, I had to change the foreign key name from oc_playlist_ibfk_1 to oc_playlist_ibfk_2 in order for that to work!
This name change would be later confusing from developers because it happens not in migrations but in some files somewhere else!

Please help me make this name change known/notified!

[UPDATE]: with separate query we use the same fkey! So ignore this comment!

@ferishili
Copy link
Contributor Author

Separate query worked thanks @dennis531

Copy link
Collaborator

@dennis531 dennis531 left a comment

Choose a reason for hiding this comment

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

Thanks for the fix! The tests are successful and the code looks good.

@ferishili
Copy link
Contributor Author

Discussion:

  • A best practice wiki page is yet to come to cover the fk index etc.

@ferishili ferishili added the Ready to merge For those PRs that ready to merge label Jun 4, 2025
@tgloeggl tgloeggl added this to the 3.30 milestone Jun 4, 2025
@tgloeggl tgloeggl merged commit 0d16102 into elan-ev:main Jun 4, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority:critical Ready to merge For those PRs that ready to merge type:bug v:3 Everything related to the Stud.IP Opencast Plugin Version 3.x
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"Synchronisierung aktivieren und Wiedergabelisten übertragen" funktioniert nicht
4 participants