search

LibriVox / librivox-catalog

LibriVox catalog and reader workflow application
https://librivox.org
MIT License
36 stars 17 forks source link

Drop unused column languages.sort_order #205

Open redrun45 opened 3 months ago

redrun45 commented 3 months ago

When attempting to add a new language from /admin/language_manager, we get a spinner that doesn't stop. Behind the scenes, the AJAX request returns an HTML error page with this message:

An uncaught Exception was encountered Type: mysqli_sql_exception Message: Field 'sort_order' doesn't have a default value Filename: /librivox/www/librivox.org/catalog/system/database/drivers/mysqli/mysqli_driver.php Line Number: 307

Looking at the scrubbed database, sort_order can apparently just be 0. When it's not, it lines up (mostly) with alphabetical order, and appears to be manually assigned. I don't know if the languages are ever sorted on this order, so I'm looking for that now.

redrun45 commented 3 months ago

(stolen from admin thread)

As far as I can tell, we don't actually use sort_order anywhere for languages, even in the API (in fact, https://librivox.org/api/feed/languages does not even appear to be a thing).

@notartom I concur, all of my grep skills fail to illuminate a single case where we use this column. I'd propose adding DEFAULT 0 to it for now, and then we have all the time in the world to test dropping it. :+1:

notartom commented 3 months ago

I'd propose adding DEFAULT 0 to it for now, and then we have all the time in the world to test dropping it. 👍

Yep, that seems safe and sensible. Am I to just YOLO that in prod directly?

redrun45 commented 3 months ago

Ha! You know I'm looking for the way to do it on my dev box right now. What I do know is that we have languages in the database with a sort_order of 0, and they're not hurting anything. If I'm able to add new languages in dev, and they look just like those existing ones, then I think we're as safe as it gets.

notartom commented 3 months ago 
MariaDB [librivox_catalog]> alter table languages alter sort_order set default 0;
Query OK, 0 rows affected (0.463 sec)
Records: 0  Duplicates: 0  Warnings: 0

Seems to have gone well. Let's have someone retry adding the language.

redrun45 commented 3 months ago

After doing the same to my dev box... I added it from the admin page in prod, using the settings provided. And... it appears to have taken!

I suppose we can leave this issue open until we've dropped that column entirely, and meanwhile it's here if someone discovers issues with language # 101. :+1: