Bug 21257

Summary: Patrons checkout table throws JS error when location/collection not defined
Product: Koha Reporter: Nick Clemens <nick>
Component: CirculationAssignee: Jonathan Druart <jonathan.druart>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: major    
Priority: P5 - low CC: cbrannon, gmcharlt, jonathan.druart, katrin.fischer, kyle.m.hall, nick
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Trivial patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 13618    
Bug Blocks:    
Attachments: Bug 21257: Return blank strings instead of null in svc/checkouts
Bug 21257: Return blank strings instead of null in svc/checkouts
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call
Bug 21257: Return blank strings instead of null in svc/checkouts
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

Description Nick Clemens 2018-08-22 15:07:47 UTC
To recreate:
1 - Check out an item to a patron
2 - Make sure that item has no collection of location defined
3 - Try to view patrons checkouts - table does not load
4 - Check the console: TypeError: oObj.collection is null
Comment 1 Nick Clemens 2018-08-22 15:12:54 UTC
Created attachment 78086 [details] [review]
Bug 21257: Return blank strings instead of null in svc/checkouts

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load
Comment 2 Tomás Cohen Arazi 2018-08-24 17:25:59 UTC
Created attachment 78145 [details] [review]
Bug 21257: Return blank strings instead of null in svc/checkouts

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected!
Comment 3 Jonathan Druart 2018-08-24 19:17:13 UTC
It works but I am wondering if we should not use the same method for/as lost and damaged.

So either in the controller, init the var with an empty string instead of undef, or in the js:

if ( oObj.damaged ) {
    due += "<span class='dmg'>" + oObj.damaged.escapeHtml() + "</span>";
}

VS

{
    "mDataProp": function ( oObj ) { 
        return oObj.location.escapeHtml();
    }
},

As escapeHtml does not handle undef I think we should do the check in the js instead, like:

{
    "mDataProp": function ( oObj ) {
        return ( oObj.location ? oObj.location.escapeHtml() : '' )
    }
},
Comment 4 Jonathan Druart 2018-08-24 19:24:48 UTC
Created attachment 78156 [details] [review]
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load
Comment 5 Katrin Fischer 2018-08-26 16:38:09 UTC
Created attachment 78173 [details] [review]
Bug 21257: Return blank strings instead of null in svc/checkouts

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Works as expected!

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 6 Katrin Fischer 2018-08-26 16:38:13 UTC
Created attachment 78174 [details] [review]
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 7 Katrin Fischer 2018-08-26 16:38:30 UTC
Is the second really an alternative patch? Because with only the second patch the checkouts table still won't load. Both together work well.
Comment 8 Jonathan Druart 2018-08-29 18:56:53 UTC
(In reply to Katrin Fischer from comment #7)
> Is the second really an alternative patch? Because with only the second
> patch the checkouts table still won't load. Both together work well.

Yes, only second patch should be needed. Which JS error do you get?
Comment 9 Katrin Fischer 2018-08-30 06:05:03 UTC
With only the second patch applied, I still get:
TypeError: oObj.collection is null
Comment 11 Jonathan Druart 2018-09-14 23:42:23 UTC
Created attachment 78849 [details] [review]
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 12 Jonathan Druart 2018-09-14 23:42:51 UTC
(In reply to Katrin Fischer from comment #9)
> With only the second patch applied, I still get:
> TypeError: oObj.collection is null

Can you try again with the last patch only please?
Comment 13 Christopher Brannon 2018-09-16 17:58:01 UTC
I tried last patch and it failed.  Needed to refresh screen.  :/
Comment 14 Christopher Brannon 2018-09-16 18:00:54 UTC
Created attachment 78973 [details] [review]
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Comment 15 Chris Cormack 2018-09-16 19:01:05 UTC
Created attachment 78991 [details] [review]
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call

To test:
1 - Find an item with no collection or location defined
2 - Check it out to a patron
3 - Note you cannot view the checkouts table
4 - Check the console: TypeError: oObj.collection is null
5 - Apply patch
6 - Restart all the things
7 - Checkouts table should load

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Christopher Brannon <cbrannon@cdalibrary.org>
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Comment 16 Nick Clemens 2018-09-16 23:10:37 UTC
Awesome work all!

Pushed to master for 18.11