Summary: | Add identification of boolean fields in the database | ||
---|---|---|---|
Product: | Koha | Reporter: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Component: | Architecture, internals, and plumbing | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | CLOSED FIXED | QA Contact: | Tomás Cohen Arazi (tcohen) <tomascohen> |
Severity: | enhancement | ||
Priority: | P5 - low | CC: | 1joynelson, jonathan.druart, martin.renvoize, pasi.kallinen, tomascohen |
Version: | Main | ||
Hardware: | All | ||
OS: | All | ||
See Also: |
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23133 https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=25152 |
||
Change sponsored?: | --- | Patch complexity: | --- |
Documentation contact: | Documentation submission: | ||
Text to go in the release notes: | Version(s) released in: |
20.05.00
|
|
Circulation function: | |||
Attachments: |
Bug 21294: Replace BOOLEAN with TINYINT(1) in our DB structure
Bug 21294: Replace BOOLEAN with TINYINT(1) in our DB structure |
Description
Tomás Cohen Arazi (tcohen)
2018-08-30 17:33:18 UTC
Adopting this bug as we should really be moving all cases of 'BOOLEAN' to 'TINYINT(1)' and ensuring the class files match as expected. Also, we've suggested a QA script enhancement on https://gitlab.com/koha-community/qa-test-tools/issues/20 to catch such cases. *** Bug 24216 has been marked as a duplicate of this bug. *** Wondering if locating candidates for Booleans could be a good Academy/beginners task. Created attachment 103013 [details] [review] Bug 21294: Replace BOOLEAN with TINYINT(1) in our DB structure To match the coding guideline SQL12. We do this for consistency. Note that we do not need an update DB as BOOLEAN is an alias for TINYINT(1). The is_boolean flags have been added to the schema. Test plan: 1. `dbic` should run correctly 2. kohastructure.sql should be syntaxicaly correct 3. `git grep -i boolean installer/data/mysql/kohastructure.sql` should not return relevant occurrences. Should we also add the is_boolean entry to the already existing TINYINT(1)? I would do it on a as-needed basis. As-needed would imply accepting (in QA) a commit that does it, on bugs that require it. For example, if we suddenly want to embed something that is missing that in a totally different place (e.g. $order->suggestions and Koha::Schema::Result::Suggestion missing is_boolean on an attribute) we would accept that. I'd be happy to do it codebase wide at this point.. seems a good idea to try and catch them all (In reply to Martin Renvoize from comment #6) > I'd be happy to do it codebase wide at this point.. seems a good idea to try > and catch them all Ok! IMO that's for a follow-up bug report. Here we cleaned the DB structure, I can remove the schema changes (is_boolean) if you think it belongs to another bug report. Created attachment 103165 [details] [review] Bug 21294: Replace BOOLEAN with TINYINT(1) in our DB structure To match the coding guideline SQL12. We do this for consistency. Note that we do not need an update DB as BOOLEAN is an alias for TINYINT(1). The is_boolean flags have been added to the schema. Test plan: 1. `dbic` should run correctly 2. kohastructure.sql should be syntaxicaly correct 3. `git grep -i boolean installer/data/mysql/kohastructure.sql` should not return relevant occurrences. Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io> (In reply to Jonathan Druart from comment #8) > IMO that's for a follow-up bug report. > > Here we cleaned the DB structure, I can remove the schema changes > (is_boolean) if you think it belongs to another bug report. It's looking good to me and I think it brings a real improvement to our consistency.. I'd be inclined to keep the patch as is. Yeah (In reply to Tomás Cohen Arazi from comment #11) > Yeah Is it a PQA stamp? :) Nice work everyone! Pushed to master for 20.05 enhancement not backported to 19.11 |