Bug 21257 - Patrons checkout table throws JS error when location/collection not defined
Summary: Patrons checkout table throws JS error when location/collection not defined
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low major (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on: 13618
Blocks:
  Show dependency treegraph
 
Reported: 2018-08-22 15:07 UTC by Nick Clemens
Modified: 2018-12-03 19:43 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 21257: Return blank strings instead of null in svc/checkouts (1.40 KB, patch)
2018-08-22 15:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 21257: Return blank strings instead of null in svc/checkouts (1.48 KB, patch)
2018-08-24 17:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call (2.83 KB, patch)
2018-08-24 19:24 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21257: Return blank strings instead of null in svc/checkouts (1.53 KB, patch)
2018-08-26 16:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call (2.88 KB, patch)
2018-08-26 16:38 UTC, Katrin Fischer
Details | Diff | Splinter Review
[ALTERNATIVE-PATCH] Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call (3.01 KB, patch)
2018-09-14 23:42 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call (3.06 KB, patch)
2018-09-16 18:00 UTC, Christopher Brannon
Details | Diff | Splinter Review
Bug 21257: (bug 13618 follow-up) Handle undef values in JS before escapeHtml call (3.12 KB, patch)
2018-09-16 19:01 UTC, Chris Cormack
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
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