Bug 39835 - Tidy is_boolean / tinyint
Summary: Tidy is_boolean / tinyint
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Jonathan Druart
QA Contact: Tomás Cohen Arazi (tcohen)
URL:
Keywords:
Depends on:
Blocks: 39324
  Show dependency treegraph
 
Reported: 2025-05-05 14:23 UTC by Jonathan Druart
Modified: 2025-05-12 13:17 UTC (History)
3 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
25.05.00
Circulation function:


Attachments
Bug 39835: Modify boolean column from int(1) to tinyint(1) (10.39 KB, patch)
2025-05-05 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39835: Add is_boolean for tinyint(1) (23.36 KB, patch)
2025-05-05 14:27 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39835: Modify boolean column from int(1) to tinyint(1) (10.44 KB, patch)
2025-05-05 14:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39835: Add is_boolean for tinyint(1) (24.51 KB, patch)
2025-05-05 14:56 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 39835: columns_settings should be tinyint (1.92 KB, patch)
2025-05-06 13:18 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39835: (follow-up) Fix API specs to check for boolean (3.26 KB, patch)
2025-05-07 08:41 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 39835: (follow-up) Fix API specs to check for boolean (1.47 KB, patch)
2025-05-07 12:09 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 39835: Adjust some boolean flags (4.55 KB, patch)
2025-05-07 12:09 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Druart 2025-05-05 14:23:16 UTC
We have a test in the QA script to catch missing "is_boolean" in the schema if the column is a tinyint(1)/boolean.
We need to add it to all the attributes, so the new version of the script can know if something is missing.
Comment 1 Jonathan Druart 2025-05-05 14:27:47 UTC
Created attachment 181924 [details] [review]
Bug 39835: Modify boolean column from int(1) to tinyint(1)

This is part of our guideline and we should enforce it.

This patch also add "is_boolean => 0" for int(1) that are not boolean
Comment 2 Jonathan Druart 2025-05-05 14:27:50 UTC
Created attachment 181925 [details] [review]
Bug 39835: Add is_boolean for tinyint(1)
Comment 3 Tomás Cohen Arazi (tcohen) 2025-05-05 14:56:47 UTC
Created attachment 181926 [details] [review]
Bug 39835: Modify boolean column from int(1) to tinyint(1)

This is part of our guideline and we should enforce it.

This patch also add "is_boolean => 0" for int(1) that are not boolean

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 4 Tomás Cohen Arazi (tcohen) 2025-05-05 14:56:50 UTC
Created attachment 181927 [details] [review]
Bug 39835: Add is_boolean for tinyint(1)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 5 Tomás Cohen Arazi (tcohen) 2025-05-05 14:59:57 UTC
I sorted the attributes alphabetically (inline) and all looks good. The dbrev works and doesn't fail if the attributes are already tinyint.

QA tools happy.

Waiting for a missing commit in [this branch](https://gitlab.com/joubu/qa-test-tools/-/commits/issue_93?ref_type=heads) to properly check if we would catch missing `is_boolean` entries in new submissions.
Comment 6 Jonathan Druart 2025-05-06 13:18:55 UTC
Created attachment 181971 [details] [review]
Bug 39835: columns_settings should be tinyint

They are boolean in the schema file already
Comment 7 Katrin Fischer 2025-05-06 16:16:32 UTC
Please don't omit the success messages in your database updates...
Comment 8 Katrin Fischer 2025-05-06 16:54:23 UTC
Pushed for 25.05!

Well done everyone, thank you!
Comment 9 Matt Blenkinsop 2025-05-07 08:41:40 UTC
Created attachment 182015 [details] [review]
Bug 39835: (follow-up) Fix API specs to check for boolean
Comment 10 Katrin Fischer 2025-05-07 09:40:39 UTC
Sorry for not realizing this sooner, but looking at Matts patch I belive there is a mistake here that is causing a lot of tests to fail now:

None of the new ones below are booleans. They are actually numbers, some can even be negative, like notforloan. They are actual tinyints that can take more than 0/1 values in Koha.

damaged
itemlost
notforloan
restricted
stack
withdrawn
 
 =head2 koha_objects_class
diff --git a/Koha/Schema/Result/Deleteditem.pm b/Koha/Schema/Result/Deleteditem.pm
index f0ffca3875e..ae17dc6d48d 100644
--- a/Koha/Schema/Result/Deleteditem.pm
+++ b/Koha/Schema/Result/Deleteditem.pm
@@ -536,7 +536,13 @@ __PACKAGE__->set_primary_key("itemnumber");
 
 __PACKAGE__->add_columns(
     '+bookable'                          => { is_boolean => 1 },
+    '+damaged'                           => { is_boolean => 1 },
     '+exclude_from_local_holds_priority' => { is_boolean => 1 },
+    '+itemlost'                          => { is_boolean => 1 },
+    '+notforloan'                        => { is_boolean => 1 },
+    '+restricted'                        => { is_boolean => 1 },
+    '+stack'                             => { is_boolean => 1 },
+    '+withdrawn'                         => { is_boolean => 1 },
 );
Comment 11 Katrin Fischer 2025-05-07 09:51:16 UTC
2) Item

Same as last comment, but for Item.

3) Also tab here is 1-10 (the tab to display on):

index 24a15f7e824..0dd90736f43 100644
--- a/Koha/Schema/Result/AuthSubfieldStructure.pm
+++ b/Koha/Schema/Result/AuthSubfieldStructure.pm
@@ -220,6 +220,12 @@ __PACKAGE__->belongs_to(
 # Created by DBIx::Class::Schema::Loader v0.07049 @ 2022-01-19 06:49:06
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:c6rPINoF/ZP4YzXU1VR+UQ
 
+__PACKAGE__->add_columns(
+    '+isurl'  => { is_boolean => 1 },
+    '+linkid' => { is_boolean => 1 },
+    '+tab'    => { is_boolean => 1 },
+);

4) Same as before, tab AND hidden are wrong:

Hidden takes numeric values.

+++ b/Koha/Schema/Result/MarcSubfieldStructure.pm
@@ -233,6 +233,10 @@ __PACKAGE__->belongs_to(
 # Created by DBIx::Class::Schema::Loader v0.07049 @ 2020-12-10 09:06:23
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:KVbDXEVf96eDkbPYpcmQVA
 
+__PACKAGE__->add_columns(
+    '+hidden' => { is_boolean => 1 },
+    '+isurl'  => { is_boolean => 1 },
+    '+tab'    => { is_boolean => 1 },
+);

5) Very doubtful about rating_value:

-# You can replace this text with custom content, and it will be preserved on regeneration
 1;
diff --git a/Koha/Schema/Result/Rating.pm b/Koha/Schema/Result/Rating.pm
index a25f1eae727..78ae27d8b27 100644
--- a/Koha/Schema/Result/Rating.pm
+++ b/Koha/Schema/Result/Rating.pm
@@ -121,6 +121,8 @@ __PACKAGE__->belongs_to(
 # Created by DBIx::Class::Schema::Loader v0.07049 @ 2021-01-21 13:39:29
 # DO NOT MODIFY THIS OR ANYTHING ABOVE! md5sum:wUwI/h1WR8kVGMNCrv/tUQ
 
+__PACKAGE__->add_columns(
+    '+rating_value' => { is_boolean => 1 },
+);
Comment 12 Jonathan Druart 2025-05-07 12:09:44 UTC
Created attachment 182020 [details] [review]
Bug 39835: (follow-up) Fix API specs to check for boolean

Amended-by: Jonathan Druart
Removed changes to item.yaml
Comment 13 Jonathan Druart 2025-05-07 12:09:47 UTC
Created attachment 182021 [details] [review]
Bug 39835: Adjust some boolean flags
Comment 14 Katrin Fischer 2025-05-12 13:17:21 UTC
Follow-ups pushed to main, sorry I forgot to note here!