Bug 14670 - add 'cite' option to detail page in OPAC
Summary: add 'cite' option to detail page in OPAC
Status: Needs Signoff
Alias: None
Product: Koha
Classification: Unclassified
Component: OPAC (show other bugs)
Version: Main
Hardware: All All
: P5 - low new feature with 1 vote (vote)
Assignee: Jonathan Druart
QA Contact: Testopia
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-08-10 22:59 UTC by Nicole C. Engard
Modified: 2024-03-25 12:11 UTC (History)
11 users (show)

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


Attachments
cite option & pop up (110.18 KB, image/png)
2015-08-10 22:59 UTC, Nicole C. Engard
Details
Bug 14670: Add Cite-option to detail page in OPAC (93.94 KB, patch)
2015-09-30 12:57 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14670: Add Cite-option to detail page in OPAC (94.01 KB, patch)
2015-10-16 13:31 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14670: add related css changes to opac.less (726 bytes, patch)
2015-10-22 14:43 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14670: add test for C4::Record::marc2cites (881 bytes, patch)
2015-10-22 14:44 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14670: Fix earlier declaration in same scope (690 bytes, patch)
2015-10-23 13:25 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions (819 bytes, patch)
2015-10-23 13:25 UTC, Martin Stenberg
Details | Diff | Splinter Review
Bug 14670: Add Cite-option to detail page in OPAC (8.99 KB, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: add related css changes to opac.less (726 bytes, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: add test for C4::Record::marc2cites (881 bytes, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: Fix earlier declaration in same scope (690 bytes, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions (819 bytes, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: QA follow-up: Regenerated opac.css (85.61 KB, patch)
2015-10-24 12:42 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 14670: Add Cite-option to detail page in OPAC (8.71 KB, patch)
2018-05-30 16:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14670: add related css changes to opac.less (719 bytes, patch)
2018-05-30 16:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14670: add test for C4::Record::marc2cites (883 bytes, patch)
2018-05-30 16:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14670: Fix earlier declaration in same scope (690 bytes, patch)
2018-05-30 16:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions (819 bytes, patch)
2018-05-30 16:37 UTC, Owen Leonard
Details | Diff | Splinter Review
Bug 14670: Add Cite-option to detail page in OPAC (8.92 KB, patch)
2024-01-29 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: add test for C4::Record::marc2cites (892 bytes, patch)
2024-01-29 16:20 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: Fix earlier declaration in same scope (699 bytes, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions (828 bytes, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: Move modal code to an include file (3.32 KB, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: Deal with empty publicatoin date (1.27 KB, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: Use 260 if 264 is empty (1016 bytes, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14670: Fix tests and import (3.43 KB, patch)
2024-01-29 16:21 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nicole C. Engard 2015-08-10 22:59:36 UTC
Created attachment 41441 [details]
cite option & pop up

It would be nice if we could add a 'cite' option to the menu on the right of the opac detail pages like in some other discovery interfaces - see attached.
Comment 1 Martin Stenberg 2015-09-30 12:57:14 UTC Comment hidden (obsolete)
Comment 2 Nicole C. Engard 2015-10-16 13:31:37 UTC Comment hidden (obsolete)
Comment 3 Jonathan Druart 2015-10-22 09:46:50 UTC
QA comments:

1/ New subroutines have to be covered by tests
2/ You should provide the related change for opac.less

Marked as Failed QA.
Comment 4 Martin Stenberg 2015-10-22 14:43:48 UTC Comment hidden (obsolete)
Comment 5 Martin Stenberg 2015-10-22 14:44:01 UTC Comment hidden (obsolete)
Comment 6 Katrin Fischer 2015-10-22 23:03:11 UTC
Hi Martin, it's usually ok to switch back to signed off once you have provided patches fixing the QA issues - this way it will get reviewed again.
Comment 7 Jonathan Druart 2015-10-23 09:55:25 UTC
Please launch the QA script before submitting patches:

 FAIL   C4/Record.pm
   FAIL   critic
                # ControlStructures::ProhibitMutatingListFunctions: Got 1 violation(s).  
   FAIL   valid
                "my" variable @authors masks earlier declaration in same scope 

use perl -wc and perlcritic to get more info.

There is something wrong with the css/less files, but it already exists on master.
Comment 8 Martin Stenberg 2015-10-23 13:24:54 UTC
Sorry about that, didn't know about the QA test tool. Issues are fixed, attaching two patches below.

What is wrong with the css/less? Is it something introduced with any of the 14670 patches?
Comment 9 Martin Stenberg 2015-10-23 13:25:21 UTC Comment hidden (obsolete)
Comment 10 Martin Stenberg 2015-10-23 13:25:32 UTC Comment hidden (obsolete)
Comment 11 Katrin Fischer 2015-10-24 12:41:48 UTC
I don't feel like this is ready to be pushed quite yet. I will give some examples:

1) If a record (serials?) don't have a publication year, the styles will look like this:

APA
(). Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt.
Chicago
. Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt.
Harvard
(). Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt.
MLA
Handelsblatt. Düsseldorf: Verl.-Gruppe Handelsblatt. .

So it doesn't cover for the case the publication year doesn't exist in the record.

Another case are articles - the citation will miss the host item information, which is very vital for those:

APA
Schweizer K., . (2010). 100 Jahre Internationaler Frauentag. : .
Chicago
Schweizer Karl, . 2010. 100 Jahre Internationaler Frauentag. : .
Harvard
Schweizer K., . (2010). 100 Jahre Internationaler Frauentag. : .
MLA
Schweizer Karl, . 100 Jahre Internationaler Frauentag. : . 2010.

I think this needs more testing with different type of media.

Also one small thing, that I had fixed for a follow-up, but will put here as a comment now:
-                place     => $record->subfield("260", "a") || "",
+                place     => $record->subfield("264", "a") || $record->subfield("260", "a") || "",
                 publisher => $record->subfield("264", "b") || $record->subfield("260", "b") || "",

Last but not least: I am reattaching the patches with a follow up taking care of a conflict in the opac.css. I have removed it from the first patch and put the fixed file into a separate patch.
Comment 12 Katrin Fischer 2015-10-24 12:42:32 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2015-10-24 12:42:35 UTC Comment hidden (obsolete)
Comment 14 Katrin Fischer 2015-10-24 12:42:39 UTC Comment hidden (obsolete)
Comment 15 Katrin Fischer 2015-10-24 12:42:44 UTC Comment hidden (obsolete)
Comment 16 Katrin Fischer 2015-10-24 12:42:47 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2015-10-24 12:42:51 UTC Comment hidden (obsolete)
Comment 18 Viktor Sarge 2015-12-07 15:32:46 UTC
Thanks for all the feedback and help! We sort of hoped that this would be more of a good "warm up" patch and with time running out for Martin in our project I have to point him in other directions so we're abandoning this one. We may be able to pick it up later, but I don't dare to make any such promises.
Comment 19 Aleisha Amohia 2016-01-27 00:17:56 UTC
I'm not sure why this has been marked Failed QA - just tested and works well. I think the only thing it needs is an icon in the menu like the other menu options have! Anyone still working on this?
Comment 20 Katrin Fischer 2016-01-27 07:35:21 UTC
Hi Aleisha, have you seen my comment#11 with the examples?
Comment 21 Viktor Sarge 2016-01-27 08:47:14 UTC
(In reply to Aleisha Amohia from comment #19)
> I'm not sure why this has been marked Failed QA - just tested and works
> well. I think the only thing it needs is an icon in the menu like the other
> menu options have! Anyone still working on this?

We´re not currently working on this. There´s still a few hundred developer hours left in our project, but for now we'll have to focus on things that have higher priority. Anyone is welcome to take it further, but I'll keep following the thread and at least keep it in mind if we should have developer hours left once we get to lower priority development.
Comment 22 Owen Leonard 2018-05-30 16:37:33 UTC
Created attachment 75679 [details] [review]
Bug 14670: Add Cite-option to detail page in OPAC

Adds a "cite" option in the tools menu in OPAC detail page for easy citations.

Test plan:
1. Apply this patch
2. Got to OPAC detail page
3. In the tools menu to the right, click the new "Cite" link
4. A modal should appear with four different citation styles (APA, Chicago,
   Harvard, MLA)

Sponsored-by: Regionbibliotek Halland / County library of Halland

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 23 Owen Leonard 2018-05-30 16:37:36 UTC
Created attachment 75680 [details] [review]
Bug 14670: add related css changes to opac.less
Comment 24 Owen Leonard 2018-05-30 16:37:39 UTC
Created attachment 75681 [details] [review]
Bug 14670: add test for C4::Record::marc2cites
Comment 25 Owen Leonard 2018-05-30 16:37:42 UTC
Created attachment 75682 [details] [review]
Bug 14670: Fix earlier declaration in same scope
Comment 26 Owen Leonard 2018-05-30 16:37:45 UTC
Created attachment 75683 [details] [review]
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
Comment 27 Owen Leonard 2018-05-30 16:39:07 UTC
I rebased(In reply to Viktor Sarge from comment #21)
> We´re not currently working on this.

I have attached rebased patches in case someone wants to pick this up. It certainly looks like it's close.
Comment 28 Jonathan Druart 2024-01-29 16:20:55 UTC
Created attachment 161600 [details] [review]
Bug 14670: Add Cite-option to detail page in OPAC

Adds a "cite" option in the tools menu in OPAC detail page for easy citations.

Test plan:
1. Apply this patch
2. Got to OPAC detail page
3. In the tools menu to the right, click the new "Cite" link
4. A modal should appear with four different citation styles (APA, Chicago,
   Harvard, MLA)

Sponsored-by: Regionbibliotek Halland / County library of Halland

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>
Comment 29 Jonathan Druart 2024-01-29 16:20:58 UTC
Created attachment 161601 [details] [review]
Bug 14670: add test for C4::Record::marc2cites
Comment 30 Jonathan Druart 2024-01-29 16:21:01 UTC
Created attachment 161602 [details] [review]
Bug 14670: Fix earlier declaration in same scope
Comment 31 Jonathan Druart 2024-01-29 16:21:04 UTC
Created attachment 161603 [details] [review]
Bug 14670: fix ControlStructures::ProhibitMutatingListFunctions
Comment 32 Jonathan Druart 2024-01-29 16:21:07 UTC
Created attachment 161604 [details] [review]
Bug 14670: Move modal code to an include file

Also add an icon for the "Cite" entry.

Sponsored-by: Orex Digital
Comment 33 Jonathan Druart 2024-01-29 16:21:10 UTC
Created attachment 161605 [details] [review]
Bug 14670: Deal with empty publicatoin date

Sponsored-by: Orex Digital
Comment 34 Jonathan Druart 2024-01-29 16:21:13 UTC
Created attachment 161606 [details] [review]
Bug 14670: Use 260 if 264 is empty

Sponsored-by: Orex Digital
Comment 35 Jonathan Druart 2024-01-29 16:21:16 UTC
Created attachment 161607 [details] [review]
Bug 14670: Fix tests and import

Sponsored-by: Orex Digital
Comment 36 Jonathan Druart 2024-01-29 16:23:16 UTC
Patches rebased and added some follow-ups.

I think this is not ready yet. The code in C4::Record needs to be beautify a bit and more tests.
Also I am wondering if we should not have a syspref.

Patches for discussion, get feedback and revive this bug.
Comment 37 David Nind 2024-03-25 03:55:36 UTC
The patches still applied and everything looks OK to me.

Would people want a copy button next to each citation? Although easy enough to select and copy.

Wasn't sure whether this should be signed off, or whether you would like more extensive feedback.

As I'm not an academic, or familiar with the citation requirements, I'm probably not the best person to provide it!
Comment 38 Jonathan Druart 2024-03-25 12:11:44 UTC
Thanks, David. I am actually expecting a signoff from the sponsor to make sure they are happy with the behaviour.