LibriVox / librivox-catalog

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

Validate author field length on template generator #196

Closed garethsime closed 6 months ago

garethsime commented 7 months ago

I think a lot of the blank screen/500 ERROR messages that people in the forums have encountered with the template generator might be related to missing field length validations on the author fields -- When users enter a name that's too long, the database insert fails, which gives an ugly 500 error.

This PR doesn't solve the underlying problem, which is that the database fields are quite short, but it does at least highlight the issue for users so that they have a better idea of what's wrong. Ideally, it'd be good to bump the limits in the database at the same time so that people don't hit them so often. (Especially the author link length limit, 55 chars is nothing in terms of URL length.)

Unfortunately, this seems to reset most of the form state, so they have to enter it again, but that's no worse than what happens with the error message anyway as far as I understand it.

(For your reviewing convenience, here's a SHOW COLUMNS for the two author tables.)

MariaDB [librivox_catalog]> SHOW COLUMNS FROM form_generators_authors;
+-----------------+-------------+------+-----+---------+----------------+
| Field           | Type        | Null | Key | Default | Extra          |
+-----------------+-------------+------+-----+---------+----------------+
| auth_id         | int(11)     | NO   | PRI | NULL    | auto_increment |
| auth_first_name | varchar(55) | YES  |     | NULL    |                |
| auth_last_name  | varchar(55) | YES  |     | NULL    |                |
| auth_yob        | varchar(10) | YES  |     | NULL    |                |
| auth_yod        | varchar(10) | YES  |     | NULL    |                |
| link_to_auth    | varchar(55) | YES  |     | NULL    |                |
+-----------------+-------------+------+-----+---------+----------------+

MariaDB [librivox_catalog]> SHOW COLUMNS FROM authors;
+-------------------+-------------+------+-----+---------+----------------+
| Field             | Type        | Null | Key | Default | Extra          |
+-------------------+-------------+------+-----+---------+----------------+
| id                | int(11)     | NO   | PRI | NULL    | auto_increment |
| first_name        | varchar(55) | YES  |     | NULL    |                |
| last_name         | varchar(55) | NO   |     | NULL    |                |
| psuedo_first_name | varchar(55) | YES  |     | NULL    |                |
| psuedo_last_name  | varchar(55) | YES  |     | NULL    |                |
| author_url        | text        | YES  |     | NULL    |                |
| other_url         | text        | YES  |     | NULL    |                |
| image_url         | text        | YES  |     | NULL    |                |
| dob               | varchar(10) | YES  |     | NULL    |                |
| dod               | varchar(10) | YES  |     | NULL    |                |
| name_hash         | varchar(32) | YES  |     | NULL    |                |
| confirmed         | int(1)      | NO   |     | 0       |                |
| linked_to         | int(11)     | NO   |     | 0       |                |
| blurb             | text        | YES  |     | NULL    |                |
| meta_complete     | int(4)      | NO   |     | 0       |                |
| meta_in_progress  | int(4)      | NO   |     | 0       |                |
+-------------------+-------------+------+-----+---------+----------------+
notartom commented 7 months ago

Changing the DB schema to increase the length of those fields is totally something we can do. So if we want to make it 128 chars or something, we can effect the schema change, and still merge something like this PR to align the validations with the underlying database.

garethsime commented 7 months ago

Cool, sounds good. Let me know what you adjust it to and I'll update the PR 👍

notartom commented 6 months ago

New lenghts:

MariaDB [librivox_catalog]> SHOW COLUMNS FROM form_generators_authors;
+-----------------+--------------+------+-----+---------+----------------+
| Field           | Type         | Null | Key | Default | Extra          |
+-----------------+--------------+------+-----+---------+----------------+
| auth_id         | int(11)      | NO   | PRI | NULL    | auto_increment |
| auth_first_name | varchar(255) | YES  |     | NULL    |                |
| auth_last_name  | varchar(255) | YES  |     | NULL    |                |
| auth_yob        | varchar(255) | YES  |     | NULL    |                |
| auth_yod        | varchar(255) | YES  |     | NULL    |                |
| link_to_auth    | varchar(255) | YES  |     | NULL    |                |
+-----------------+--------------+------+-----+---------+----------------+
MariaDB [librivox_catalog]> SHOW COLUMNS FROM authors;
+-------------------+--------------+------+-----+---------+----------------+
| Field             | Type         | Null | Key | Default | Extra          |
+-------------------+--------------+------+-----+---------+----------------+
| id                | int(11)      | NO   | PRI | NULL    | auto_increment |
| first_name        | varchar(255) | YES  |     | NULL    |                |
| last_name         | varchar(255) | YES  |     | NULL    |                |
| psuedo_first_name | varchar(255) | YES  |     | NULL    |                |
| psuedo_last_name  | varchar(255) | YES  |     | NULL    |                |
| author_url        | text         | YES  |     | NULL    |                |
| other_url         | text         | YES  |     | NULL    |                |
| image_url         | text         | YES  |     | NULL    |                |
| dob               | varchar(255) | YES  |     | NULL    |                |
| dod               | varchar(255) | YES  |     | NULL    |                |
| name_hash         | varchar(32)  | YES  |     | NULL    |                |
| confirmed         | int(1)       | NO   |     | 0       |                |
| linked_to         | int(11)      | NO   |     | 0       |                |
| blurb             | text         | YES  |     | NULL    |                |
| meta_complete     | int(4)       | NO   |     | 0       |                |
| meta_in_progress  | int(4)       | NO   |     | 0       |                |
+-------------------+--------------+------+-----+---------+----------------+