[Bug 21294] New: Indentify boolean fields on the DB structure
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Bug ID: 21294 Summary: Indentify boolean fields on the DB structure Change sponsored?: --- Product: Koha Version: master Hardware: All OS: All Status: NEW Severity: enhancement Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: tomascohen@gmail.com QA Contact: testopia@bugs.koha-community.org The coding guidelines require a specific handling for boolean attributes [1]. QA should require this for new devs, but the current DB structure lacks this information in many places. We should add this metadata on the current schema files. [1] https://wiki.koha-community.org/wiki/Coding_Guidelines#SQL12:_Booleans -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@ptfs-europe | |.com --- Comment #1 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- 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. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=23133 -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |pasi.kallinen@koha-suomi.fi --- Comment #2 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- *** Bug 24216 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Katrin Fischer <katrin.fischer@bsz-bw.de> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords| |Academy CC| |katrin.fischer@bsz-bw.de --- Comment #3 from Katrin Fischer <katrin.fischer@bsz-bw.de> --- Wondering if locating candidates for Booleans could be a good Academy/beginners task. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |Needs Signoff -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #4 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- Created attachment 103013 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103013&action=edit 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. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|Academy | See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25152 Assignee|koha-bugs@lists.koha-commun |jonathan.druart@bugs.koha-c |ity.org |ommunity.org CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #5 from Tomás Cohen Arazi <tomascohen@gmail.com> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #6 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'd be happy to do it codebase wide at this point.. seems a good idea to try and catch them all -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #7 from Tomás Cohen Arazi <tomascohen@gmail.com> --- (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! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #8 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- 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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |tomascohen@gmail.com Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103013|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #9 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Created attachment 103165 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103165&action=edit 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> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #10 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- (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. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #11 from Tomás Cohen Arazi <tomascohen@gmail.com> --- Yeah -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #12 from Jonathan Druart <jonathan.druart@bugs.koha-community.org> --- (In reply to Tomás Cohen Arazi from comment #11)
Yeah
Is it a PQA stamp? :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Tomás Cohen Arazi <tomascohen@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |tomascohen@gmail.com |y.org | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |20.05.00 released in| | Status|Passed QA |Pushed to master -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 --- Comment #13 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Nice work everyone! Pushed to master for 20.05 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Joy Nelson <joy@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |joy@bywatersolutions.com --- Comment #14 from Joy Nelson <joy@bywatersolutions.com> --- enhancement not backported to 19.11 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=21294 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Summary|Indentify boolean fields on |Add identification of |the DB structure |boolean fields in the | |database -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon@bugs.koha-community.org