Bug 8436

Summary: Add branch limit on checkexpiration.pl
Product: Koha Reporter: claire.hernandez <claire.hernandez>
Component: SerialsAssignee: Julian Maurice <julian.maurice>
Status: CLOSED FIXED QA Contact:
Severity: enhancement    
Priority: P5 - low CC: chris, colin.campbell, gitbot, gmcharlt, jesse, jonathan.druart, julian.maurice, katrin.fischer, koha, kyle, paola.rossi, tomascohen
Version: master   
Hardware: All   
OS: All   
Change sponsored?: --- Patch complexity: ---
Documentation contact: Documentation submission:
Text to go in the release notes:
Version(s) released in:
Bug Depends on: 8435    
Bug Blocks: 15171    
Attachments: Bug 8436: Add limit on branch in checkexpiration.pl
Bug 8436: Add limit on branch in checkexpiration.pl
Bug 8436: Add limit on branch in checkexpiration.pl
[SIGNED OFF] Bug 8436: Add limit on branch in checkexpiration.pl
Bug 8436: Replace GetBranches with GetBranchesLoop
Bug 8436: Use C4::Context->IsSuperLibrarian()
[PASSED QA] Bug 8436: Add limit on branch in checkexpiration.pl
[PASSED QA] Bug 8436: Replace GetBranches with GetBranchesLoop
[PASSED QA] Bug 8436: Use C4::Context->IsSuperLibrarian()
Bug 8436: QA Follow-up: Activate branch filter, if IndependentBranches is off
Bug 8436: QA Follow-up: Add Library to result list
[PASSED QA] Bug 8436: QA Follow-up: Activate branch filter, if IndependentBranches is off
[PASSED QA] Bug 8436: QA Follow-up: Add Library to result list

Description claire.hernandez@biblibre.com 2012-07-13 08:14:10 UTC
This limit is available only if user is superlibrarian or has the permission 'superserials'. Otherwise, only subscriptions that are linked to the working branch are shown.

(BibLibre stetienne/ft/MT6725)
Comment 1 Julian Maurice 2012-08-22 12:39:57 UTC Comment hidden (obsolete)
Comment 2 Julian Maurice 2012-08-22 12:40:42 UTC
Note that this patch depends on Bug 8435
Comment 3 Julian Maurice 2012-08-22 13:07:03 UTC Comment hidden (obsolete)
Comment 4 Galen Charlton 2013-05-31 03:32:15 UTC
Patch does not apply; please rebase and resubmit.
Comment 5 Jonathan Druart 2013-06-27 14:05:50 UTC Comment hidden (obsolete)
Comment 6 I'm just a bot 2013-09-29 04:58:07 UTC
Patch applied cleanly, go forth and signoff
Comment 7 I'm just a bot 2013-09-29 04:58:33 UTC
Patch applied cleanly, go forth and signoff
Comment 8 I'm just a bot 2013-10-24 19:34:22 UTC
Patch applied cleanly, go forth and signoff
Comment 9 Chris Cormack 2013-12-23 20:11:34 UTC
Can I get a test plan on this please so I can signoff
Comment 10 Jesse Maseto 2014-03-12 11:47:34 UTC
Julian,

Can you send a testing plan for us to follow.

-Jesse
Comment 11 Koha Team University Lyon 3 2014-06-19 12:32:20 UTC
difficult to test this patch before solving bug 12452
Comment 12 Paola Rossi 2015-01-13 16:33:05 UTC Comment hidden (obsolete)
Comment 13 Katrin Fischer 2015-01-30 07:17:03 UTC
Julian, could you take a look at the search for the expiration date? I think I can see bug 12452. 

I have a serial subscription with expiration date on 10/09/2014.
If I use the filter on the left side of the subscription search with 01/10/2014 the subscription shows up (as it should).

But when I repeat the same search on the 'check expiration' page, the subscription is not found.

Using iso formatted dates dd/mm/yyyy.
Comment 14 Jonathan Druart 2015-01-30 08:24:45 UTC
(In reply to Katrin Fischer from comment #13)

I answered on bug 12452.
Comment 15 Kyle M Hall 2015-01-30 12:44:14 UTC
Can you please use C4::Branch::GetBranchesLoop rather than GetBranches with bespoke selection logic?
Comment 16 Jonathan Druart 2015-02-20 09:59:05 UTC Comment hidden (obsolete)
Comment 17 Katrin Fischer 2015-03-22 23:08:28 UTC
Comment on attachment 35166 [details] [review]
[SIGNED OFF] Bug 8436: Add limit on branch in checkexpiration.pl

Review of attachment 35166 [details] [review]:
-----------------------------------------------------------------

::: serials/checkexpiration.pl
@@ +106,5 @@
>  }
> +
> +my $branchname;
> +my @branches_loop;
> +if (   $flags->{superlibrarian}

I think we should be using C4::Context->IsSuperLibrarian() here - please check!
Comment 18 Julian Maurice 2015-08-25 11:16:51 UTC Comment hidden (obsolete)
Comment 19 Katrin Fischer 2015-10-20 15:04:47 UTC
Created attachment 43646 [details] [review]
[PASSED QA] Bug 8436: Add limit on branch in checkexpiration.pl

This limit is only available for superlibrarians and patrons that have
superserials permission. Other patrons will only see subscriptions of
their branch.

Signed-off-by: Paola Rossi <paola.rossi@cineca.it>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 20 Katrin Fischer 2015-10-20 15:04:51 UTC
Created attachment 43647 [details] [review]
[PASSED QA] Bug 8436: Replace GetBranches with GetBranchesLoop

The branch selection logic still exists in GetBranchesLoop, it should be
used here.

Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 21 Katrin Fischer 2015-10-20 15:04:54 UTC
Created attachment 43648 [details] [review]
[PASSED QA] Bug 8436: Use C4::Context->IsSuperLibrarian()

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 22 Katrin Fischer 2015-10-20 15:04:59 UTC Comment hidden (obsolete)
Comment 23 Katrin Fischer 2015-10-20 15:05:03 UTC Comment hidden (obsolete)
Comment 24 Katrin Fischer 2015-10-20 15:10:12 UTC
I am guilty of 2 small follow-ups here - Julian or some other QA person - could you take a look?
Comment 25 Kyle M Hall 2015-10-22 11:17:11 UTC
Created attachment 43750 [details] [review]
[PASSED QA] Bug 8436: QA Follow-up: Activate branch filter, if IndependentBranches is off

The superserials permission is only in use in combination with
IndependentBranches - so it makes sense to also show the branch
filter when IndependentBranches is off.

Permission description:
 superserials Manage subscriptions from any branch
 (only applies when IndependentBranches is used)

Also adds a missing check for superlibraian.

Note: the duplicated permission check in the code is not really nice
- would be good to rewrite this to be more clean.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 26 Kyle M Hall 2015-10-22 11:17:21 UTC
Created attachment 43751 [details] [review]
[PASSED QA] Bug 8436: QA Follow-up: Add Library to result list

If we offer a library filter, we should also show the
library in the result list, so that search options
and result list match.

Otherwise, for libraries with multiple branches, there
is no way to see, which is the subscription of a
specific branch in the list.

- Add a Library column in the result list
- Rename branch to library in the template

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Comment 27 Tomás Cohen Arazi 2015-11-04 14:30:47 UTC
Patches pushed to master.

Thanks Julian!