Bug 14244 - viewing a bib item's circ history requires circulation permissions
Summary: viewing a bib item's circ history requires circulation permissions
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Circulation (show other bugs)
Version: master
Hardware: All All
: P5 - low normal (vote)
Assignee: Tom Misilo
QA Contact: Katrin Fischer
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-05-21 14:42 UTC by Nicole C. Engard
Modified: 2017-06-14 22:11 UTC (History)
11 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
Change permissions of issue history (1.41 KB, patch)
2015-09-16 17:48 UTC, Tom Misilo
Details | Diff | Splinter Review
Bug 14244: viewing a bib item's circ history requires circulation permissions (1.47 KB, patch)
2015-10-15 15:39 UTC, Nicole C. Engard
Details | Diff | Splinter Review
Bug 14244: viewing a bib item's circ history requires circulation permissions (1.54 KB, patch)
2015-10-16 11:03 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14244: Does not display patron's info in the checkout history if not authorised (2.07 KB, patch)
2016-01-25 10:35 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 14244: viewing a bib item's circ history requires circulation permissions (1.67 KB, patch)
2016-01-25 14:25 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14244: Does not display patron's info in the checkout history if not authorised (2.21 KB, patch)
2016-01-25 14:26 UTC, Marc Véron
Details | Diff | Splinter Review
Bug 14244: viewing a bib item's circ history requires circulation permissions (1.72 KB, patch)
2016-02-05 12:42 UTC, Kyle M Hall
Details | Diff | Splinter Review
Bug 14244: Does not display patron's info in the checkout history if not authorised (2.26 KB, patch)
2016-02-05 12:42 UTC, Kyle M Hall
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-05-21 14:42:31 UTC
On issuehistory.pl you have:

 flagsrequired   => { circulate => "circulate_remaining_permissions" },

But that really doesn't make sense. People should be able to view the history of circulation without having permission to circulate items themselves.
Comment 1 Tom Misilo 2015-08-18 12:14:12 UTC
Would catalogue => "*" make sense here? If so I can make a patch for it.
Comment 2 Tom Misilo 2015-09-16 17:48:22 UTC Comment hidden (obsolete)
Comment 3 Nicole C. Engard 2015-10-15 15:39:16 UTC Comment hidden (obsolete)
Comment 4 Jonathan Druart 2015-10-16 11:03:29 UTC Comment hidden (obsolete)
Comment 5 Tomás Cohen Arazi 2015-10-21 12:47:58 UTC
Clarification, just in case: this bug is passed-QA, I've just requested the QAM's opinion privately, but I also marked with the rel_3_22_candidate keyword to make it clear it will be pushed for 3.22.
Comment 6 Tom Misilo 2015-11-28 14:12:49 UTC
Did this make it into 3.22? Or was it pushed back to the next release? I didn't notice it in the release notes. Thanks!
Comment 7 Jonathan Druart 2015-12-01 09:53:16 UTC
(In reply to Tom Misilo from comment #6)
> Did this make it into 3.22? Or was it pushed back to the next release? I
> didn't notice it in the release notes. Thanks!

No, it has not been pushed yet.
Comment 8 Jonathan Druart 2016-01-07 13:59:38 UTC
Still waiting for QAM's opinion.
Comment 9 Kyle M Hall 2016-01-15 17:52:42 UTC
Still waiting for QAM's opinion.
Comment 10 Katrin Fischer 2016-01-19 23:17:52 UTC
Hm, at the risk of people not being happy with me... this is what I think:

The circulation history tab includes information about borrowers - the patrons who have checked out items of the record.

If you have access to the circulation module, it seems 'more ok' to see that, then if you just have catalog permission in order to be able to do some minor tasks in the staff interface.

A suggestion: If someone has only catalog - don't show the information about the patrons. If someone has circulation permissions - show all the columns.

Could that work?
Comment 11 Kyle M Hall 2016-01-20 11:41:15 UTC
(In reply to Katrin Fischer from comment #10)
> Hm, at the risk of people not being happy with me... this is what I think:
> 
> The circulation history tab includes information about borrowers - the
> patrons who have checked out items of the record.
> 
> If you have access to the circulation module, it seems 'more ok' to see
> that, then if you just have catalog permission in order to be able to do
> some minor tasks in the staff interface.
> 
> A suggestion: If someone has only catalog - don't show the information about
> the patrons. If someone has circulation permissions - show all the columns.
> 
> Could that work?

That sounds like a good approach. So, to be clear about it, you are proposing that if the librarian does not have the circulation permission, the first column ( Patron ) should not be displayed, correct? This would need to be done at the template level so the data is not just hidden from view, but would not be in the HTML source at all.
Comment 12 Katrin Fischer 2016-01-20 11:45:08 UTC
Yep, that's what I was thinking. The page level permission could be catalog - which every staff member should have. Then adjust the datatable to hide the first column iirc in the template itself.
Comment 13 Jonathan Druart 2016-01-25 10:35:11 UTC Comment hidden (obsolete)
Comment 14 Marc Véron 2016-01-25 13:09:56 UTC
The patches work as described.

However, I can still see patron information in the Normal view on  ...cgi-bin/koha/catalogue/detail.pl?biblionumber=xxxxx, in column "Status".

Example:

Currently in local use by Edna Acosta : due 11/12/2015 23:59 

...and the link on the name leads to the "No permission" page.
Comment 15 Katrin Fischer 2016-01-25 13:12:14 UTC
Hi Marc, I think this bug is only about the reading history - so the detail page/normal view woudl be better on another bug report. Do you agree with the general approach or would you like it to work differently?
Comment 16 Marc Véron 2016-01-25 14:22:58 UTC
Hi Katrin, no problem with fixing it with a separate bug.
Comment 17 Marc Véron 2016-01-25 14:25:36 UTC Comment hidden (obsolete)
Comment 18 Marc Véron 2016-01-25 14:26:28 UTC Comment hidden (obsolete)
Comment 19 Marc Véron 2016-01-25 14:53:21 UTC
(In reply to Katrin Fischer from comment #15)
> Hi Marc, I think this bug is only about the reading history - so the detail
> page/normal view woudl be better on another bug report. Do you agree with
> the general approach or would you like it to work differently?

H Katrin, I filed a separate bug: Bug 15655
Comment 20 Kyle M Hall 2016-02-05 12:42:33 UTC
Created attachment 47694 [details] [review]
Bug 14244: viewing a bib item's circ history requires circulation permissions

On issuehistory.pl you have:

 flagsrequired   => { circulate => "circulate_remaining_permissions" },

But that really doesn't make sense. People should be able to view the history of circulation without having permission to circulate items themselves

This patch changes the required permissions from circulate to catalogue.

Test Plan:
1. Login as user without the circulate_remaining_permissions
2. Attempt to view Circulation History of an Item
3. Does not work
4. Apply Patch
5. Attempt to view circulation history with a user that has access to the staff side.
6. Can view circulation history now

Signed-off-by: Nicole Engard <nengard@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 21 Kyle M Hall 2016-02-05 12:42:41 UTC
Created attachment 47695 [details] [review]
Bug 14244: Does not display patron's info in the checkout history if not authorised

The librarian needs CAN_user_circulate_circulate_remaining_permissions
to see the checkouts patron's information.

Test plan:
The "Patron" column of the checkout history page should only be
displayed of you have the circulate > circulate_remaining_permissions
permission.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 22 Brendan Gallagher 2016-02-24 01:04:54 UTC
pushed to master - should be in the may 2016 release.  Thanks
Comment 23 Julian Maurice 2016-03-07 08:33:51 UTC
Patches pushed to stable, will be in 3.22.5
Comment 24 Frédéric Demians 2016-03-15 08:57:23 UTC
This patch has been pushed to 3.20.x, will be in 3.20.10.