Bug 20886

Summary: Koha::Object::TO_JSON indiscriminately casting to integer
Product: Koha Reporter: Andrew Isherwood <bugzilla>
Component: Architecture, internals, and plumbingAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Marcel de Rooy <m.de.rooy>
Severity: normal    
Priority: P5 - low CC: fridolin.somers, jonathan.druart, m.de.rooy, martin.renvoize, nick, tomascohen
Version: unspecified   
Hardware: All   
OS: All   
See Also: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18330
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on:    
Bug Blocks: 20797    
Attachments: Bug 20886: Do not cast undef to 0 (TO_JSON)
Bug 20886: (sign-off) Koha::Object::TO_JSON indiscriminately casting to integer
Bug 20886: Do not cast undef to 0 (TO_JSON)
Bug 20886: Do not cast undef to 0 (TO_JSON)

Description Andrew Isherwood 2018-06-06 10:15:00 UTC
As noted in bug 20797 there appears to be a bug in Koha::Object::TO_JSON. Specifically:

elsif ( _numeric_column_type( $columns_info->{$col}->{data_type} ) ) {

    # TODO: Remove once the solution for
    # https://rt.cpan.org/Ticket/Display.html?id=119904
    # is ported to whatever distro we support by that time
    $unblessed->{$col} += 0;
}

Here we are explicitly casting to an integer to workaround an upstream bug. This works fine unless the data in question comes from a numeric type column but is NULL, in which case it is cast to 0.

I suspect the desired behaviour here is for it to remain unaltered unless the value is also numeric.
Comment 1 Andrew Isherwood 2018-06-06 10:16:07 UTC
CC'ing Tomas in at JD's suggestion
Comment 2 Tomás Cohen Arazi 2018-06-06 12:31:54 UTC
(In reply to Andrew Isherwood from comment #1)
> CC'ing Tomas in at JD's suggestion

You are right. Good catch!
Comment 3 Jonathan Druart 2018-06-06 14:13:31 UTC
Created attachment 75857 [details] [review]
Bug 20886: Do not cast undef to 0 (TO_JSON)

We should not cast undefined values to 0.
Comment 4 Jonathan Druart 2018-06-06 14:16:00 UTC
Any side-effects to expect with this change?
Comment 5 Andrew Isherwood 2018-06-07 09:50:11 UTC
Created attachment 75869 [details] [review]
Bug 20886: (sign-off) Koha::Object::TO_JSON indiscriminately casting to integer

Works as advertised for me.
Comment 6 Marcel de Rooy 2018-06-07 13:28:00 UTC
Created attachment 75875 [details] [review]
Bug 20886: Do not cast undef to 0 (TO_JSON)

We should not cast undefined values to 0.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 7 Jonathan Druart 2018-06-07 14:06:17 UTC
We need Tomas's stamp on this one
Comment 8 Tomás Cohen Arazi 2018-06-07 14:16:30 UTC
Created attachment 75883 [details] [review]
Bug 20886: Do not cast undef to 0 (TO_JSON)

We should not cast undefined values to 0.

Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This was a bug I introduced. I think the fix is right, and if something
gets broken for this, it means it was making the wrong assumptions on
the data.
Comment 9 Nick Clemens 2018-06-08 13:54:08 UTC
Pushed to master for 18.11, awesome work all!
Comment 10 Martin Renvoize 2018-06-08 14:39:56 UTC
This patch has been pushed to 18.05.x and will be in 18.05.01
Comment 11 Fridolin Somers 2018-06-14 13:31:37 UTC Comment hidden (obsolete)
Comment 12 Fridolin Somers 2018-06-14 13:32:31 UTC
Is this also for 17.11.x ?
It looks similar to Bug 18330