Bug 20797 - If an Ill request has an associated bib record, the detail view should contain a link to the record
Summary: If an Ill request has an associated bib record, the detail view should contai...
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: ILL (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Target Milestone: ---
Assignee: Andrew Isherwood
QA Contact: Marcel de Rooy
URL:
Whiteboard:
Keywords:
Depends on: 20772 20886
Blocks: 20799 20995
  Show dependency treegraph
 
Reported: 2018-05-21 08:23 UTC by Martin Renvoize
Modified: 2018-10-16 12:57 UTC (History)
6 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:
Version(s) released in:


Attachments
Bug 20797 - Add link to biblio detail view from ILL detail view (1.42 KB, patch)
2018-05-21 08:35 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.29 KB, patch)
2018-05-23 11:31 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.63 KB, patch)
2018-05-23 12:23 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (4.39 KB, patch)
2018-05-30 10:49 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.28 KB, patch)
2018-06-07 10:16 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (4.47 KB, patch)
2018-09-27 12:40 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (4.47 KB, patch)
2018-09-27 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.42 KB, patch)
2018-09-27 12:41 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20996: Remove prefix use of borrower category (13.21 KB, patch)
2018-09-28 13:17 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20996: Fix unit tests (8.04 KB, patch)
2018-09-28 13:17 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20996: Further enhance performance (4.01 KB, patch)
2018-09-28 13:17 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20996: (follow-up) Add <branch> to conf tpl (2.16 KB, patch)
2018-09-28 13:17 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20996: (follow-up) Fix column name expectation (1.87 KB, patch)
2018-09-28 13:17 UTC, Magnus Enger
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.27 KB, patch)
2018-10-04 12:49 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.28 KB, patch)
2018-10-04 12:49 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.36 KB, patch)
2018-10-05 10:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.37 KB, patch)
2018-10-05 10:09 UTC, Marcel de Rooy
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.33 KB, patch)
2018-10-05 10:43 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.34 KB, patch)
2018-10-05 10:43 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797 - Add link to biblio detail view from ILL detail view (2.33 KB, patch)
2018-10-05 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.34 KB, patch)
2018-10-05 11:06 UTC, Andrew Isherwood
Details | Diff | Splinter Review
k (7.21 KB, patch)
2018-10-05 11:17 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: Add link to biblio detail view from ILL detail view (2.43 KB, patch)
2018-10-05 11:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.39 KB, patch)
2018-10-05 11:18 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: (QA follow-up) Fix ill requests table (961 bytes, patch)
2018-10-05 12:19 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: Add link to biblio detail view from ILL detail view (2.38 KB, patch)
2018-10-05 12:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.40 KB, patch)
2018-10-05 12:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: (QA follow-up) Fix ill requests table (968 bytes, patch)
2018-10-05 12:57 UTC, Andrew Isherwood
Details | Diff | Splinter Review
Bug 20797: Add link to biblio detail view from ILL detail view (2.43 KB, patch)
2018-10-05 14:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: (follow-up) Conditionally link (1.39 KB, patch)
2018-10-05 14:37 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 20797: (QA follow-up) Fix ill requests table (961 bytes, patch)
2018-10-05 14:37 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Martin Renvoize 2018-05-21 08:23:16 UTC
Currently, the ILL request detail view shows a plain biblionumber.. this should be a link through to the bib record view.
Comment 1 Martin Renvoize 2018-05-21 08:35:56 UTC Comment hidden (obsolete)
Comment 2 Magnus Enger 2018-05-22 07:31:45 UTC Comment hidden (obsolete)
Comment 3 Martin Renvoize 2018-05-23 11:07:53 UTC Comment hidden (obsolete)
Comment 4 Andrew Isherwood 2018-05-23 11:31:28 UTC Comment hidden (obsolete)
Comment 5 Andrew Isherwood 2018-05-23 11:32:02 UTC Comment hidden (obsolete)
Comment 6 Magnus Enger 2018-05-23 12:23:35 UTC Comment hidden (obsolete)
Comment 7 Jonathan Druart 2018-05-28 19:57:42 UTC
Please surround 'N/A' with tags (to help translators)
Comment 8 Andrew Isherwood 2018-05-30 10:49:42 UTC
Created attachment 75656 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view
Comment 9 Andrew Isherwood 2018-05-30 10:50:22 UTC
I've now added <span> tags around all "N/A" strings. Thanks for the feedback Jonathan.
Comment 10 Jonathan Druart 2018-05-30 19:35:26 UTC
If "Biblio ID" is not filled, the column contains "0" with a link to /catalogue/detail.pl?biblionumber=0

Also, Andrew, I do not know your workflow when you submit a new patch, but you removed the signed-off-by line and the commit message.
You should prefer a follow-up patch (which will be squashed if needed by QAer or RM). It also eases the review.
Comment 11 Andrew Isherwood 2018-05-31 09:42:52 UTC
Thanks for that Jonathan. I've been squashing fix commits into the original commit, then using git-bz to attach it. So, yes, I can see how that would wipe stuff out, apologies for that. I'll submit follow up patches from now on.
Comment 12 Andrew Isherwood 2018-06-05 10:23:20 UTC
Hi Jonathan

Further to your comment:

If "Biblio ID" is not filled, the column contains "0" with a link to /catalogue/detail.pl?biblionumber=0

I've done some digging. It appears that, for some reason, the API response contains a biblio_id of 0, even though the value in the database is NULL.

I've checked and there's definitely no casting of this value to an integer in my code so I'm wondering if it's happening at a lower level. The schema definition for the column defines it as an integer, but it's also nullable.

I'm not sure what to do about this. I can obviously work around it in the template, but that would just be masking the problem. Any ideas?
Comment 13 Andrew Isherwood 2018-06-05 10:32:09 UTC
Further to this, I think this is the problem:

https://github.com/PTFS-Europe/koha/blob/master/Koha/Object.pm#L249

If it's a numeric column, the value is being cast into an integer regardless of whether it's populated or not.

Do you agree this is the bug?
Comment 14 Jonathan Druart 2018-06-05 14:28:48 UTC
(In reply to Andrew Isherwood from comment #13)
> Further to this, I think this is the problem:
> 
> https://github.com/PTFS-Europe/koha/blob/master/Koha/Object.pm#L249
> 
> If it's a numeric column, the value is being cast into an integer regardless
> of whether it's populated or not.
> 
> Do you agree this is the bug?

That sounds like a bug yes, can you report it and CC Tomas please?
Comment 15 Andrew Isherwood 2018-06-06 10:16:41 UTC
Done: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=20886
Comment 16 Andrew Isherwood 2018-06-07 10:16:08 UTC Comment hidden (obsolete)
Comment 17 Andrew Isherwood 2018-06-07 10:18:45 UTC Comment hidden (obsolete)
Comment 18 Martin Renvoize 2018-06-07 11:05:35 UTC Comment hidden (obsolete)
Comment 19 Magnus Enger 2018-09-27 12:35:11 UTC Comment hidden (obsolete)
Comment 20 Magnus Enger 2018-09-27 12:40:35 UTC Comment hidden (obsolete)
Comment 21 Magnus Enger 2018-09-27 12:41:07 UTC
Created attachment 79496 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view

Signed-off-by: Magnus Enger <magnus@enger.priv.no>
Comment 22 Magnus Enger 2018-09-27 12:41:12 UTC
Created attachment 79497 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Magnus Enger <magnus@enger.priv.no>
Works as advertised. The link is only displayed if there is a
biblionumber to link to.
Comment 23 Magnus Enger 2018-09-28 13:17:12 UTC Comment hidden (obsolete)
Comment 24 Magnus Enger 2018-09-28 13:17:21 UTC Comment hidden (obsolete)
Comment 25 Magnus Enger 2018-09-28 13:17:31 UTC Comment hidden (obsolete)
Comment 26 Magnus Enger 2018-09-28 13:17:37 UTC Comment hidden (obsolete)
Comment 27 Magnus Enger 2018-09-28 13:17:43 UTC Comment hidden (obsolete)
Comment 28 Magnus Enger 2018-09-28 13:20:46 UTC
Ooops, attached some patches to the wrong bug. Sorry!
Comment 29 Josef Moravec 2018-10-04 11:46:53 UTC
Please add the filters: 

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
   FAIL	  filters
		missing_filter at line 568 (                                        [% request.cost %])
		missing_filter at line 456 (                                        [% request.cost %])
		missing_filter at line 528 (
Comment 30 Andrew Isherwood 2018-10-04 12:49:06 UTC
Created attachment 79946 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view
Comment 31 Andrew Isherwood 2018-10-04 12:49:09 UTC
Created attachment 79947 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null
Comment 32 Andrew Isherwood 2018-10-04 12:49:51 UTC
Rebased and fixed problem raised in comment #29 - thanks Josef!
Comment 33 Marcel de Rooy 2018-10-05 10:09:52 UTC
Created attachment 80018 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 34 Marcel de Rooy 2018-10-05 10:09:57 UTC
Created attachment 80019 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 35 Andrew Isherwood 2018-10-05 10:43:01 UTC
Created attachment 80021 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 36 Andrew Isherwood 2018-10-05 10:43:05 UTC
Created attachment 80022 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 37 Andrew Isherwood 2018-10-05 11:06:16 UTC
Created attachment 80038 [details] [review]
Bug 20797 - Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 38 Andrew Isherwood 2018-10-05 11:06:20 UTC
Created attachment 80040 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Comment 39 Katrin Fischer 2018-10-05 11:17:45 UTC
Created attachment 80047 [details] [review]
k
Comment 40 Katrin Fischer 2018-10-05 11:18:35 UTC
Created attachment 80048 [details] [review]
Bug 20797: Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 41 Katrin Fischer 2018-10-05 11:18:39 UTC
Created attachment 80049 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 42 Katrin Fischer 2018-10-05 11:19:49 UTC
Retested as we rebased it on top of 20772 - trying to build a path through the jungle here :)
Comment 43 Katrin Fischer 2018-10-05 12:19:04 UTC
Created attachment 80055 [details] [review]
Bug 20797: (QA follow-up) Fix ill requests table

... by adding a missing comma.
Comment 44 Andrew Isherwood 2018-10-05 12:57:15 UTC
Created attachment 80071 [details] [review]
Bug 20797: Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 45 Andrew Isherwood 2018-10-05 12:57:21 UTC
Created attachment 80072 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 46 Andrew Isherwood 2018-10-05 12:57:29 UTC
Created attachment 80073 [details] [review]
Bug 20797: (QA follow-up) Fix ill requests table

... by adding a missing comma.
Comment 47 Katrin Fischer 2018-10-05 14:37:25 UTC
Created attachment 80120 [details] [review]
Bug 20797: Add link to biblio detail view from ILL detail view

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 48 Katrin Fischer 2018-10-05 14:37:30 UTC
Created attachment 80121 [details] [review]
Bug 20797: (follow-up) Conditionally link

We don't want to display the link to the biblio if the biblio_id is null

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Comment 49 Katrin Fischer 2018-10-05 14:37:34 UTC
Created attachment 80122 [details] [review]
Bug 20797: (QA follow-up) Fix ill requests table

... by adding a missing comma.
Comment 50 Tomás Cohen Arazi 2018-10-13 12:21:27 UTC
Pushed for 18.11. Thanks all!
Comment 51 Martin Renvoize 2018-10-16 12:57:08 UTC
Enhancement, will not be backported to 18.05.x series.