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

Some characters now not allowed in author date fields #154

Closed
twinkietoes-on opened this issue Jan 4, 2023 · 6 comments
Closed

Some characters now not allowed in author date fields #154

twinkietoes-on opened this issue Jan 4, 2023 · 6 comments

Comments

@twinkietoes-on
Copy link
Collaborator

Trying to use a dash in the author date field (such as "ca. 1315-1325") or a slash (such as "1315/1325") results in the field not saving. Those are the only two characters discovered that are occasionally used.
This has only been an issue since the upgrades. It worked before.

@twinkietoes-on
Copy link
Collaborator Author

We also had a possible rejection of a period in an author name field. So:

  • non-numerical characters in author birth/death year fields
  • non-numerical characters in copyright & edition year fields
  • periods in author name fields (possibly)

garethsime added a commit to garethsime/librivox-catalog that referenced this issue Aug 21, 2023
This goes some way to addressing issue LibriVox#154 -- users won't be able to
enter non-numeric dates, which is unfortunate, but it does mean that
they'll get a proper error message instead of the random server error
that they get right now.

For the more technical explanation -- When we save the form generation,
that all goes into the `form_generators` table. Any new authors that are
created as part of the generator are saved into
`form_generators_authors`, which has the `auth_yob` and `auth_yod`
columns as four-digit integers. This doesn't align well with the actual
`authors` table, which allows pretty much anything (`varchar(10)`), and
I have no idea how people specify AD vs. BCE using the generator.

I don't know enough about how the table is used to know whether we
should consider migrating it to `varchar(10)` to match `authors`, but
having the two consistent with each other would be good probably.

I also tried specifying the input fields as numeric in the HTML, but
that didn't quite have the desired effect -- it would just submit the
field as empty if it wasn't right, which meant we'd silently drop the
data rather than get a validation error.
@notartom
Copy link
Member

notartom commented Sep 9, 2023

I believe it worked before the upgrade because the new version of the database software is more strict in what input it accepts (https://mariadb.com/kb/en/sql-mode/#strict_trans_tables). So previously, if a field in the database was listed as an integer, but if the input was some alphanumeric string of characters, there would be some magic at the database level that would accept the input, possibly mangling it somewhat, or dropping that particular field altogether while still updating the other ones in the query (I'm not very knowledgeable about this, hence the handwavy "magic" explanation).

@garethsime's PR #169 is one way to address this, but it would mean author YOB and YOD would only be numbers. That would make sense to me, but I'm not sure if there's precedent for inputing other kinds of data into those fields.

@garethsime
Copy link
Contributor

Yeah, my PR definitely doesn't solve the underlying problem, it just gives a better error message. (There were a couple of threads in the forum where people were expressing frustration that they were just getting a generic error message with no information on how to fix it.)

@twinkietoes-on
Copy link
Collaborator Author

twinkietoes-on commented Sep 9, 2023 via email

@notartom
Copy link
Member

notartom commented Sep 10, 2023

From PR #169:

For the more technical explanation -- When we save the form generation, that all goes into the form_generators table. Any new authors that are created as part of the generator are saved into form_generators_authors, which has the auth_yob and auth_yod columns as four-digit integers. This doesn't align well with the actual authors table, which allows pretty much anything (varchar(10)), and I have no idea how people specify AD vs. BCE using the generator (negative, maybe?).

From @twinkietoes-on above:

There's definitely precedent for non-numeric characters in YOB and YOD fields: ca., BC or BCE, AD or CE, and fl. being the primary ones.

So that sounds like the solution is to make auth_yob and auth_yod in form_generators_authors varchar(10) to allow non-numeric characters as well, and to align them with auth_yob and auth_yod in authors.

@garethsime
Copy link
Contributor

garethsime commented Mar 1, 2024

It looks like this has been fixed, at least it has the new column definitions in my copy of the database from a couple months ago (and it seems to work locally!)

@notartom notartom closed this as completed Mar 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants