Bug 8436 - Add branch limit on checkexpiration.pl
Summary: Add branch limit on checkexpiration.pl
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Serials (show other bugs)
Version: master
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Julian Maurice
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on: 8435
Blocks: 15171
  Show dependency treegraph
 
Reported: 2012-07-13 08:14 UTC by claire.hernandez@biblibre.com
Modified: 2017-06-14 22:05 UTC (History)
12 users (show)

See Also:
Change sponsored?: ---
Patch complexity: ---
Bot Control: ---
When did the bot last check this: 2013-10-25 00:00:00
Who signed the patch off:
Text to go in the release notes:


Attachments
Bug 8436: Add limit on branch in checkexpiration.pl (4.94 KB, patch)
2012-08-22 12:39 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8436: Add limit on branch in checkexpiration.pl (4.99 KB, patch)
2012-08-22 13:07 UTC, Julian Maurice
Details | Diff | Splinter Review
Bug 8436: Add limit on branch in checkexpiration.pl (4.97 KB, patch)
2013-06-27 14:05 UTC, Jonathan Druart
Details | Diff | Splinter Review
[SIGNED OFF] Bug 8436: Add limit on branch in checkexpiration.pl (5.14 KB, patch)
2015-01-13 16:33 UTC, Paola Rossi
Details | Diff | Splinter Review
Bug 8436: Replace GetBranches with GetBranchesLoop (1.68 KB, patch)
2015-02-20 09:59 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 8436: Use C4::Context->IsSuperLibrarian() (770 bytes, patch)
2015-08-25 11:16 UTC, Julian Maurice
Details | Diff | Splinter Review
[PASSED QA] Bug 8436: Add limit on branch in checkexpiration.pl (5.09 KB, patch)
2015-10-20 15:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 8436: Replace GetBranches with GetBranchesLoop (1.75 KB, patch)
2015-10-20 15:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 8436: Use C4::Context->IsSuperLibrarian() (839 bytes, patch)
2015-10-20 15:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 8436: QA Follow-up: Activate branch filter, if IndependentBranches is off (1.90 KB, patch)
2015-10-20 15:04 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 8436: QA Follow-up: Add Library to result list (2.55 KB, patch)
2015-10-20 15:05 UTC, Katrin Fischer
Details | Diff | Splinter Review
[PASSED QA] Bug 8436: QA Follow-up: Activate branch filter, if IndependentBranches is off (1.97 KB, patch)
2015-10-22 11:17 UTC, Kyle M Hall
Details | Diff | Splinter Review
[PASSED QA] Bug 8436: QA Follow-up: Add Library to result list (2.60 KB, patch)
2015-10-22 11:17 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 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 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!