Bug 33946 - biblio-title.inc should not add a link if biblio does not exist
Summary: biblio-title.inc should not add a link if biblio does not exist
Status: RESOLVED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low normal (vote)
Assignee: Jonathan Druart
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks: 33948 33949
  Show dependency treegraph
 
Reported: 2023-06-08 07:44 UTC by Jonathan Druart
Modified: 2023-12-28 20:46 UTC (History)
2 users (show)

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


Attachments
Bug 33946: Do not display link to non-existing bibliographic record (2.96 KB, patch)
2023-06-08 10:53 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 33946: Do not display link to non-existing bibliographic record (3.02 KB, patch)
2023-06-08 12:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33946: Resync opac/intranet includes (2.58 KB, patch)
2023-06-08 12:21 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 33946: Do not display link to non-existing bibliographic record (3.02 KB, patch)
2023-07-10 21:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33946: Resync opac/intranet includes (2.64 KB, patch)
2023-07-10 21:10 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 33946: (QA follow-up) Add spans and classes for styling and translatability (1.87 KB, patch)
2023-07-10 21:10 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 Jonathan Druart 2023-06-08 07:44:59 UTC
If you use biblio-title.inc on a biblio that does not longer exist, "No title" is shown with a link to catalogue/detail?.pl?biblionumber=

We should at least remove the link.

We could even do better and display "No bibliographic record" if the biblio does not longer exist.
Comment 1 Katrin Fischer 2023-06-08 08:56:18 UTC
I like this idea for improvement. In this case we have no biblionumber at all, right?
Comment 2 Jonathan Druart 2023-06-08 10:53:39 UTC
Created attachment 152145 [details] [review]
Bug 33946: Do not display link to non-existing bibliographic record

If you use biblio-title.inc on a biblio that does not longer exist, "No title" is shown with a link to catalogue/detail?.pl?biblionumber=

We should at least remove the link.

We could even do better and display "No bibliographic record" if the biblio does not longer exist.

Test plan:
Edit mainpage.pl, add
$template->param( your_bib => Koha::Biblios->find(42) );
Edit intranet-main.tt, add
[% INCLUDE 'biblio-title.inc' biblio=your_bib link = 1 %]

Not hit the main page and see what's hapenning.
If you have a bibliographic record with biblionumber=42, its info will
be displayed.
Remove this record and try again => it's replaced with "No bibliographic record"
without a link
Comment 3 Martin Renvoize 2023-06-08 12:14:45 UTC
Hmm.. this patch made me digg the opac and intranet includes again.. and they're more different that I would expect again.. mostly chomping values, but also some escaping differences but also intranet includes medium where opac doesn't (assume this is deliberate?)

Anyway.. I wonder if we aught to also update js-biblio-format.inc to match this behaviour.
Comment 4 Martin Renvoize 2023-06-08 12:15:47 UTC
The js equivilent include returns an empty string if the biblio is found to be empty.
Comment 5 Martin Renvoize 2023-06-08 12:21:13 UTC
Created attachment 152178 [details] [review]
Bug 33946: Do not display link to non-existing bibliographic record

If you use biblio-title.inc on a biblio that does not longer exist, "No title" is shown with a link to catalogue/detail?.pl?biblionumber=

We should at least remove the link.

We could even do better and display "No bibliographic record" if the biblio does not longer exist.

Test plan:
Edit mainpage.pl, add
$template->param( your_bib => Koha::Biblios->find(42) );
Edit intranet-main.tt, add
[% INCLUDE 'biblio-title.inc' biblio=your_bib link = 1 %]

Not hit the main page and see what's hapenning.
If you have a bibliographic record with biblionumber=42, its info will
be displayed.
Remove this record and try again => it's replaced with "No bibliographic record"
without a link

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 6 Martin Renvoize 2023-06-08 12:21:15 UTC
Created attachment 152179 [details] [review]
Bug 33946: Resync opac/intranet includes

This patch resyncs the whitespace chomping in the opac include to match
that in the intranet include so aid in diffing between the two.

No changes should be visible in the UI but there may be cleaner markup
as a result.
Comment 7 Katrin Fischer 2023-07-10 20:56:40 UTC
This is a nice little improvement. Is there any plan to pull the records title from deletedbiblio maybe? Thinking of acq as one of the spots this would really be useful!
Comment 8 Katrin Fischer 2023-07-10 21:10:21 UTC
Created attachment 153297 [details] [review]
Bug 33946: Do not display link to non-existing bibliographic record

If you use biblio-title.inc on a biblio that does not longer exist, "No title" is shown with a link to catalogue/detail?.pl?biblionumber=

We should at least remove the link.

We could even do better and display "No bibliographic record" if the biblio does not longer exist.

Test plan:
Edit mainpage.pl, add
$template->param( your_bib => Koha::Biblios->find(42) );
Edit intranet-main.tt, add
[% INCLUDE 'biblio-title.inc' biblio=your_bib link = 1 %]

Not hit the main page and see what's hapenning.
If you have a bibliographic record with biblionumber=42, its info will
be displayed.
Remove this record and try again => it's replaced with "No bibliographic record"
without a link

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Katrin Fischer 2023-07-10 21:10:24 UTC
Created attachment 153298 [details] [review]
Bug 33946: Resync opac/intranet includes

This patch resyncs the whitespace chomping in the opac include to match
that in the intranet include so aid in diffing between the two.

No changes should be visible in the UI but there may be cleaner markup
as a result.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 10 Katrin Fischer 2023-07-10 21:10:27 UTC
Created attachment 153299 [details] [review]
Bug 33946: (QA follow-up) Add spans and classes for styling and translatability

Adds back the spans around No title and more spans for
'No bibliographic record' with individual classes for each
to allow for styling later.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 11 Tomás Cohen Arazi 2023-07-14 12:01:19 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 12 Martin Renvoize 2023-07-18 11:52:14 UTC
Thanks for all the hard work!

Pushed to 23.05.x for the next release
Comment 13 Matt Blenkinsop 2023-07-18 14:23:51 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x