Bug 38342 - Koha::Object->store warning on invalid ENUM value
Summary: Koha::Object->store warning on invalid ENUM value
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Architecture, internals, and plumbing (show other bugs)
Version: Main
Hardware: All All
: P5 - low minor
Assignee: Tomás Cohen Arazi (tcohen)
QA Contact: Martin Renvoize (ashimema)
URL:
Keywords:
Depends on: 38175
Blocks:
  Show dependency treegraph
 
Reported: 2024-11-04 13:28 UTC by Tomás Cohen Arazi (tcohen)
Modified: 2024-12-05 22:04 UTC (History)
3 users (show)

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


Attachments
Bug 38342: Pass the value to exception to avoid warnings (1.22 KB, patch)
2024-11-04 13:32 UTC, Tomás Cohen Arazi (tcohen)
Details | Diff | Splinter Review
Bug 38342: Pass the value to exception to avoid warnings (1.27 KB, patch)
2024-11-05 13:42 UTC, Paul Derscheid
Details | Diff | Splinter Review
Bug 38342: Pass the value to exception to avoid warnings (1.34 KB, patch)
2024-11-06 17:11 UTC, Martin Renvoize (ashimema)
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Tomás Cohen Arazi (tcohen) 2024-11-04 13:28:31 UTC
Bug 38175 introduced handling for truncated enum values. But the exception that is thrown is missing a parameter, and as such prints a warning when stringifying the exception.

To test:
1. On KTD, run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Booking.t
=> FAIL: There's:
```
DBIx::Class::Storage::DBI::_dbh_execute(): DBI Exception: DBD::mysql::st execute failed: Data truncated for column 'status' at row 1 at /kohadevbox/koha/Koha/Object.pm line 172
Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 84.
Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 84.
        ok 1 - Throws exception when passed booking status would fail enum constraint
```

The 'Use of initialized...' ones are not correct.
Comment 1 Tomás Cohen Arazi (tcohen) 2024-11-04 13:32:25 UTC
Created attachment 173919 [details] [review]
Bug 38342: Pass the value to exception to avoid warnings

This patch makes the exception on bad enum values in Koha::Object->store
not print a warning in the event of stringifying the exception.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Booking.t
=> FAIL: You get this warnings:

```
Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 84.
```
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more uninitialized values warnings
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 2 Tomás Cohen Arazi (tcohen) 2024-11-04 13:36:49 UTC
@ashimema: I have doubts about the handling you added for the 'property' value. Specifically:

```
   property => $property =~ /(\w+\.\w+)$/
         ? $1
         : $property,    # results in table.column without quotes or backtics
```

Before that check you added for cleaning it up, there's a use of `$property` as-is for checking the column is of type ENUM. So I feel either my patch is correct, or we might miss some cases for enum types (i.e. the cleanup needs to happen for all uses of `$property`, not just the last one).
Comment 3 Paul Derscheid 2024-11-05 13:42:03 UTC
Created attachment 173978 [details] [review]
Bug 38342: Pass the value to exception to avoid warnings

This patch makes the exception on bad enum values in Koha::Object->store
not print a warning in the event of stringifying the exception.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Booking.t
=> FAIL: You get this warnings:

```
Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 84.
```
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more uninitialized values warnings
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Comment 4 Martin Renvoize (ashimema) 2024-11-06 17:11:15 UTC
Created attachment 174089 [details] [review]
Bug 38342: Pass the value to exception to avoid warnings

This patch makes the exception on bad enum values in Koha::Object->store
not print a warning in the event of stringifying the exception.

To test:
1. Run:
   $ ktd --shell
  k$ prove t/db_dependent/Koha/Booking.t
=> FAIL: You get this warnings:

```
Use of uninitialized value in sprintf at /kohadevbox/koha/Koha/Exceptions/Object.pm line 84.
```
2. Apply this patch
3. Repeat 1
=> SUCCESS: No more uninitialized values warnings
4. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 5 Katrin Fischer 2024-11-07 17:46:12 UTC
Should we have a unit test here?
Comment 6 Katrin Fischer 2024-11-07 17:51:36 UTC
Pushed for 24.11!

Well done everyone, thank you!