Bug 12098

Summary: User can edit some subcriptions he should be prevented to
Product: Koha Reporter: Frédéric Demians <f.demians>
Component: SerialsAssignee: Bugs List <koha-bugs>
Status: CLOSED FIXED QA Contact: Testopia <testopia>
Severity: minor    
Priority: P5 - low CC: colin.campbell, fridolin.somers, gmcharlt, jonathan.druart, kyle, mtompset
Version: Main   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: Small patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Circulation function:
Bug Depends on: 12080    
Bug Blocks:    
Attachments: Refactor can_*_subscription in C4::Serials
Fix C4::Serials::can_edit_subscription
Bug 12080: Fix C4::Serials::can_edit_subscription
Bug 12080: Refactor can_*_subscription in C4::Serials
[PASSED QA] Bug 12098: Fix C4::Serials::can_edit_subscription
[PASSED QA] Bug 12098: Refactor can_*_subscription in C4::Serials

Description Frédéric Demians 2014-04-16 19:12:42 UTC
If the user doesn't have the superserials permission, or isn't superlibrarian, you can still edit a subscription accessing the subscription detail page through the serial collection page (for records with multiple subscriptions) or accessing the detail page directly.
Comment 1 Mark Tompsett 2014-04-16 22:37:30 UTC
Perhaps we should move JD's patches from bug 12080 here? I don't like the thought that his patches are hidden by being obsoleted on another bug.
Comment 2 Frédéric Demians 2014-04-17 10:33:39 UTC
(In reply to M. Tompsett from comment #1)
> Perhaps we should move JD's patches from bug 12080 here? I don't like the
> thought that his patches are hidden by being obsoleted on another bug.

Jonathan have them on his repo, and he will re-post them here. That's my understanding. If you prefer, or would like to test them now, I can attach his patches here as they are. Just tell.
Comment 3 Katrin Fischer 2014-04-17 10:43:30 UTC
Jonathan said he would be gone for a week - so reattaching the patches here would be nice.
[+] Comment 4 Frédéric Demians 2014-04-17 13:41:49 UTC Comment hidden (obsolete)
[+] Comment 5 Frédéric Demians 2014-04-17 13:42:30 UTC Comment hidden (obsolete)
[+] Comment 6 Kyle M Hall (khall) 2014-04-18 11:37:22 UTC Comment hidden (obsolete)
[+] Comment 7 Kyle M Hall (khall) 2014-04-18 11:38:36 UTC Comment hidden (obsolete)
Comment 8 Katrin Fischer 2014-04-18 12:20:00 UTC
After applying all the patches for 12048 - 12080 - 12098 t/db_dependent/Bookseller.t fails for me. I can imagine the test might be wrong now or needs to be rewritten a bit. I will continue testing for now.
Comment 9 Katrin Fischer 2014-04-18 13:46:13 UTC
Created attachment 27290 [details] [review]
[PASSED QA] Bug 12098: Fix C4::Serials::can_edit_subscription

This patch adds some unit tests for the can_edit_subscription and add a
new can_show_subscription subroutines.

http://bugs.koha-community.org/show_bug.cgi?id=12098

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Notes on second patch.
Comment 10 Katrin Fischer 2014-04-18 13:46:22 UTC
Created attachment 27291 [details] [review]
[PASSED QA] Bug 12098: Refactor can_*_subscription in C4::Serials

http://bugs.koha-community.org/show_bug.cgi?id=12098

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Tested on top of patches for 12048 and 12080.

Subscription search
- superlibrarian, IndyBranches on/off - always sees all subscriptions
- superserials, IndyBranches on/off - always sees all subscriptions
- no superserials, IndyBranches on - only sees own subscriptions
Note: Subscriptions without branches will only show, when all subscriptions
      are visible. In a future enh it might be good to enforce setting a
      branch, when IndyBranches is used.
- no superserials, IndyBranches off - always sees all subscriptions

Subscription editing
- superlibrarian, IndyBranches on/off - can edit all subscriptions
- superserials, IndyBranches on/off - can edit all subscriptions
- no superserials, IndyBranches on - can only edit own subscriptons and
  subscriptions without branch
  NOTE: it would make sense to also allow Edit > Edit as new (duplicate)
  here, so one can copy the subscription from another branch to modify
  it for the own branch.

Passes tests in t, xt and QA script, also newly provided unit tests.
Comment 11 Galen Charlton 2014-04-18 20:53:57 UTC
Pushed to master, along with an important follow-up that puts the C4::Serials::can_show_subscription routine into use (as otherwise, it would be a routine that had zero actual affect).

BUT NOTE:

    Note that there is user-visible change: if a subscription has a
    blank library, all users with serials permissions will be able
    to view and/or edit it.  It remains to be determined whether
    we *want* such subscriptions to exist, or if they should only
    be tied to specific libraries.

If this change in behavior isn't desirable, please open a new bug.

Thanks, Jonathan!
Comment 12 Fridolin Somers 2014-08-07 08:32:16 UTC
Pushed to 3.14.x, will be in 3.14.10