Bug 21835 - Request ID is displayed as NaN
Summary: Request ID is displayed as NaN
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal (vote)
Assignee: Andrew Isherwood
QA Contact: Marcel de Rooy
URL:
Keywords:
Depends on: 20996
Blocks: 20640
  Show dependency treegraph
 
Reported: 2018-11-14 15:48 UTC by Andrew Isherwood
Modified: 2020-01-06 20:14 UTC (History)
4 users (show)

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


Attachments
Bug 21835: Fix display of request ID (1.87 KB, patch)
2018-11-14 16:14 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 21835: Fix display of request ID (1.91 KB, patch)
2018-11-14 17:06 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 21835: Fix display of request ID (2.00 KB, patch)
2018-11-16 07:16 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 21835: (QA follow-up) Fix failing test (2.12 KB, patch)
2018-11-16 15:12 UTC, Martin Renvoize
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Isherwood 2018-11-14 15:48:28 UTC
In the main ILL requests table, the request ID is displayed as NaN.

There are two problems here. Firstly, we're not populating id_prefix in the API response, it got missing in the deprecation of TO_JSON in bug 20996. As such, by the time we get to the JS that populates the ILL table, there is no id_prefix property in the row object, so concatenating undefined with an integer returns NaN.
Comment 1 Andrew Isherwood 2018-11-14 16:14:17 UTC
Created attachment 82342 [details] [review]
Bug 21835: Fix display of request ID

We now populate the id_prefix property in the API response. It turns out
that explicitly casting to a string wasn't necessary as JS does "the
right thing" when concatenating a string and an integer

Test plan:

- Before applying the patch view the ILL requests table
- TEST: Observe that the Request number column displays as NaN
- Apply the patch
- Refresh the page
- TEST: Observe that the request number now displays correctly
- Add the following block to your koha-conf.xml:

<branch>
    <code>**The code of a branch that has an ILL request attached to
    it**</code>
    <prefix>YAY</prefix>
</branch>

- Refresh the page
- TEST: Observe the request made on the branch that you specified in the
config are now prefixed with "YAY-"
Comment 2 Owen Leonard 2018-11-14 17:06:43 UTC
Created attachment 82343 [details] [review]
Bug 21835: Fix display of request ID

We now populate the id_prefix property in the API response. It turns out
that explicitly casting to a string wasn't necessary as JS does "the
right thing" when concatenating a string and an integer

Test plan:

- Before applying the patch view the ILL requests table
- TEST: Observe that the Request number column displays as NaN
- Apply the patch
- Refresh the page
- TEST: Observe that the request number now displays correctly
- Add the following block to your koha-conf.xml:

<branch>
    <code>**The code of a branch that has an ILL request attached to
    it**</code>
    <prefix>YAY</prefix>
</branch>

- Refresh the page
- TEST: Observe the request made on the branch that you specified in the
config are now prefixed with "YAY-"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Comment 3 Marcel de Rooy 2018-11-16 07:16:51 UTC
Created attachment 82381 [details] [review]
Bug 21835: Fix display of request ID

We now populate the id_prefix property in the API response. It turns out
that explicitly casting to a string wasn't necessary as JS does "the
right thing" when concatenating a string and an integer

Test plan:

- Before applying the patch view the ILL requests table
- TEST: Observe that the Request number column displays as NaN
- Apply the patch
- Refresh the page
- TEST: Observe that the request number now displays correctly
- Add the following block to your koha-conf.xml:

<branch>
    <code>**The code of a branch that has an ILL request attached to
    it**</code>
    <prefix>YAY</prefix>
</branch>

- Refresh the page
- TEST: Observe the request made on the branch that you specified in the
config are now prefixed with "YAY-"

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 4 Nick Clemens 2018-11-16 13:10:41 UTC
Awesome work all!

Pushed to master for 18.11
Comment 5 Martin Renvoize 2018-11-16 15:12:24 UTC
Created attachment 82405 [details] [review]
Bug 21835: (QA follow-up) Fix failing test

The illrequest api response should always be augmented with an id_prefix
field which is not part of the core illrequest object
Comment 6 Martin Renvoize 2018-11-30 14:18:31 UTC
Not required for 18.05.x series.