Bugzilla – Attachment 188445 Details for
Bug 16631
Limit listing of reports using patron library
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
0002-Bug-16631-Institute-library-limits.patch
0002-Bug-16631-Institute-library-limits.patch (text/plain), 69.36 KB, created by
Casey Conlin
on 2025-10-25 19:37:12 UTC
(
hide
)
Description:
0002-Bug-16631-Institute-library-limits.patch
Filename:
MIME Type:
Creator:
Casey Conlin
Created:
2025-10-25 19:37:12 UTC
Size:
69.36 KB
patch
obsolete
>From dcda4c2af0adcf9b7c15bf6cb6feb29508b67a68 Mon Sep 17 00:00:00 2001 >From: "Cameron E. Tidd" <ctidd@citlink.net> >Date: Sat, 26 Jul 2025 02:22:06 +0000 >Subject: [PATCH 2/2] Bug 16631: Institute library limits This patch implements > the library_limit concept for Report/Reports. Staff with the correct > permission and superlibrarians can set libaries to which the report should be > visible, if the system preference is enabled. > >--- > C4/Reports/Guided.pm | 47 +- > C4/UsageStats.pm | 1 + > Koha/Manual.pm | 1 + > Koha/Report.pm | 61 ++- > Koha/Reports.pm | 8 +- > admin/columns_settings.yml | 3 + > installer/data/mysql/atomicupdate/bug_16631.pl | 39 ++ > installer/data/mysql/kohastructure.sql | 13 + > installer/data/mysql/mandatory/sysprefs.sql | 1 + > installer/data/mysql/mandatory/userpermissions.sql | 1 + > .../intranet-tmpl/prog/en/includes/permissions.inc | 3 + > .../intranet-tmpl/prog/en/includes/prefs-menu.inc | 11 + > .../prog/en/modules/admin/preferences/reports.pref | 8 + > .../en/modules/reports/guided_reports_start.tt | 74 ++- > reports/guided_reports.pl | 555 ++++++++++++--------- > t/Koha/Auth/Permissions.t | 2 + > t/db_dependent/Koha/Reports.t | 104 +++- > t/db_dependent/Reports/Guided.t | 87 +++- > tools/scheduler.pl | 41 +- > 19 files changed, 792 insertions(+), 268 deletions(-) > create mode 100755 installer/data/mysql/atomicupdate/bug_16631.pl > create mode 100644 koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref > >diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm >index 94e271cef7..3168d60c73 100644 >--- a/C4/Reports/Guided.pm >+++ b/C4/Reports/Guided.pm >@@ -881,7 +881,52 @@ sub get_saved_reports { > > my $result = $dbh->selectall_arrayref( $query, { Slice => {} }, @args ); > >- return $result; >+ my $limit_pref_enabled = C4::Context->preference('LimitReportsByBranch'); >+ >+ return $result unless $limit_pref_enabled; >+ >+ # Preference enabled AND showing all: attach library limits metadata, but do not filter >+ if ( $filter->{show_all_reports} ) { >+ foreach my $report (@$result) { >+ my $report_obj = Koha::Reports->find( $report->{id} ); >+ if ($report_obj) { >+ my $limits_rs = $report_obj->get_library_limits; >+ if ($limits_rs) { >+ my @branches = map { $_->branchcode } $limits_rs->as_list; >+ $report->{library_limits} = \@branches; >+ } else { >+ $report->{library_limits} = []; >+ } >+ } else { >+ $report->{library_limits} = []; >+ } >+ } >+ return $result; >+ } >+ >+ # Preference enabled AND not showing all: apply library limits filtering >+ my $limited_reports = Koha::Reports->search_with_library_limits( {}, {}, C4::Context::mybranch() ); >+ my %allowed = map { $_->{id} => 1 } @{ $limited_reports->unblessed }; >+ my @limited = grep { $allowed{ $_->{id} } } @$result; >+ >+ # Attach library limits data for the filtered list >+ foreach my $report (@limited) { >+ my $report_obj = Koha::Reports->find( $report->{id} ); >+ if ($report_obj) { >+ my $limits_rs = $report_obj->get_library_limits; >+ if ($limits_rs) { >+ my @branches = map { $_->branchcode } $limits_rs->as_list; >+ $report->{library_limits} = \@branches; >+ } else { >+ $report->{library_limits} = []; >+ } >+ } else { >+ $report->{library_limits} = []; >+ } >+ } >+ >+ return \@limited; >+ > } > > =head2 get_column_type($column) >diff --git a/C4/UsageStats.pm b/C4/UsageStats.pm >index 4022741ac0..9f198a7750 100644 >--- a/C4/UsageStats.pm >+++ b/C4/UsageStats.pm >@@ -314,6 +314,7 @@ sub _shared_preferences { > autoMemberNum > BorrowerRenewalPeriodBase > EnableBorrowerFiles >+ EnableFilteringReports > EnhancedMessagingPreferences > ExtendedPatronAttributes > intranetreadinghistory >diff --git a/Koha/Manual.pm b/Koha/Manual.pm >index fe5cc2c5b3..e811c64867 100644 >--- a/Koha/Manual.pm >+++ b/Koha/Manual.pm >@@ -117,6 +117,7 @@ our $mapping = { > 'admin/preferences#logs' => '/logspreferences.html', > 'admin/preferences#opac' => '/opacpreferences.html', > 'admin/preferences#patrons' => '/patronspreferences.html', >+ 'admin/preferences#reports' => '/reportspreferences.html', > 'admin/preferences#searching' => '/searchingpreferences.html', > 'admin/preferences#serials' => '/serialspreferences.html', > 'admin/preferences#staff_interface' => '/staffclientpreferences.html', >diff --git a/Koha/Report.pm b/Koha/Report.pm >index 54511cdc4c..a4b6ff7782 100644 >--- a/Koha/Report.pm >+++ b/Koha/Report.pm >@@ -20,9 +20,15 @@ use Modern::Perl; > use Koha::Database; > use Koha::Reports; > >+use Koha::Object; >+use Koha::Object::Limit::Library; >+use Koha::Patrons; >+use C4::Context; >+ >+use base qw(Koha::Object Koha::Object::Limit::Library); >+ > #use Koha::DateUtils qw( dt_from_string output_pref ); > >-use base qw(Koha::Object); > # > # FIXME We could only return an error code instead of the arrayref > # Only 1 error is returned >@@ -268,4 +274,57 @@ sub _type { > return 'SavedSql'; > } > >+=head3 _library_limits >+ >+Configurable library limits >+ >+=cut >+ >+sub _library_limits { >+ return { >+ class => "ReportsBranch", >+ id => "report_id", >+ library => "branchcode", >+ }; >+} >+ >+=head3 can_manage_limits >+ >+ my $can = Koha::Report->can_manage_limits($patron); >+ >+Returns true if branch limiting is enabled and the patron can manage report limits (superlibrarian or permission reports => manage_report_limits). >+ >+=cut >+ >+sub can_manage_limits { >+ my ( $class, $patron ) = @_; >+ return C4::Context->preference('LimitReportsByBranch') >+ && ( $patron->is_superlibrarian || $patron->has_permission( { reports => 'manage_report_limits' } ) ); >+} >+ >+=head3 can_access >+ >+ my $allowed = $report->can_access($patron); >+ >+Returns true if the patron may access this report considering branch limits, or if limits are disabled. >+ >+=cut >+ >+sub can_access { >+ my ( $self, $patron ) = @_; >+ >+ return 1 unless C4::Context->preference('LimitReportsByBranch'); >+ >+ return 1 if __PACKAGE__->can_manage_limits($patron); >+ >+ my $limits_rs = $self->get_library_limits; >+ return 1 unless $limits_rs; >+ >+ my $user_branch = C4::Context::mybranch(); >+ return 0 unless $user_branch; >+ >+ my %allowed = map { $_->branchcode => 1 } $limits_rs->as_list; >+ return $allowed{$user_branch} ? 1 : 0; >+} >+ > 1; >diff --git a/Koha/Reports.pm b/Koha/Reports.pm >index 7d99232b9b..360ee4d9cf 100644 >--- a/Koha/Reports.pm >+++ b/Koha/Reports.pm >@@ -21,7 +21,7 @@ use Koha::Database; > > use Koha::Report; > >-use base qw(Koha::Objects); >+use base qw(Koha::Objects Koha::Objects::Limit::Library); > > =head1 NAME > >@@ -33,12 +33,6 @@ Koha::Reports - Koha Report Object set class > > =cut > >-=head3 _type >- >-Returns name of corresponding DBIC resultset >- >-=cut >- > sub _type { > return 'SavedSql'; > } >diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml >index 380a48d3ba..ef5d06b72b 100644 >--- a/admin/columns_settings.yml >+++ b/admin/columns_settings.yml >@@ -1675,6 +1675,9 @@ modules: > columnname: saved_results > - > columnname: update >+ - >+ columnname: library_limits >+ is_hidden: 1 > - > columnname: actions > cannot_be_toggled: 1 >diff --git a/installer/data/mysql/atomicupdate/bug_16631.pl b/installer/data/mysql/atomicupdate/bug_16631.pl >new file mode 100755 >index 0000000000..6488c8e5c2 >--- /dev/null >+++ b/installer/data/mysql/atomicupdate/bug_16631.pl >@@ -0,0 +1,39 @@ >+use Modern::Perl; >+use Koha::Installer::Output qw(say_warning say_success say_info); >+ >+return { >+ bug_number => "16631", >+ description => "Add library limits to reports", >+ up => sub { >+ my ($args) = @_; >+ my ( $dbh, $out ) = @$args{qw(dbh out)}; >+ >+ $dbh->do( >+ q{CREATE TABLE IF NOT EXISTS `reports_branches` ( >+ `report_id` int(11) NOT NULL, >+ `branchcode` varchar(10) NOT NULL, >+ KEY `report_id` (`report_id`), >+ KEY `branchcode` (`branchcode`), >+ CONSTRAINT `reports_branches_ibfk_1` FOREIGN KEY (`report_id`) REFERENCES `saved_sql` (`id`) ON DELETE CASCADE, >+ CONSTRAINT `reports_branches_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE >+ ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci} >+ ); >+ >+ say $out "Added new table 'reports_branches'"; >+ >+ $dbh->do( >+ q{INSERT IGNORE INTO systempreferences >+ (variable,value,options,explanation,type) VALUES ('LimitReportsByBranch', '0', NULL, 'Enable ability for staff to limit report access based on home library.', 'YesNo')} >+ ); >+ >+ say $out "Added new system preference 'LimitReportsByBranch'"; >+ >+ $dbh->do( >+ q{INSERT IGNORE INTO permissions >+ (module_bit, code, description) VALUES (16, 'manage_report_limits', 'Manage report limits')} >+ ); >+ >+ say $out "Added new permission 'manage_report_limits'"; >+ >+ }, >+}; >diff --git a/installer/data/mysql/kohastructure.sql b/installer/data/mysql/kohastructure.sql >index e7448b7fca..dd06d8a58a 100644 >--- a/installer/data/mysql/kohastructure.sql >+++ b/installer/data/mysql/kohastructure.sql >@@ -5772,6 +5772,19 @@ CREATE TABLE `saved_reports` ( > ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; > /*!40101 SET character_set_client = @saved_cs_client */; > >+DROP TABLE IF EXISTS `reports_branches`; >+/*!40101 SET @saved_cs_client = @@character_set_client */; >+/*!40101 SET character_set_client = utf8mb4 */; >+CREATE TABLE `reports_branches` ( >+ `report_id` int(11) NOT NULL, >+ `branchcode` varchar(10) NOT NULL, >+ KEY `report_id` (`report_id`), >+ KEY `branchcode` (`branchcode`), >+ CONSTRAINT `reports_branches_ibfk_1` FOREIGN KEY (`report_id`) REFERENCES `saved_sql` (`id`) ON DELETE CASCADE, >+ CONSTRAINT `reports_branches_ibfk_2` FOREIGN KEY (`branchcode`) REFERENCES `branches` (`branchcode`) ON DELETE CASCADE >+) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci; >+/*!40101 SET character_set_client = @saved_cs_client */; >+ > -- > -- Table structure for table `saved_sql` > -- >diff --git a/installer/data/mysql/mandatory/sysprefs.sql b/installer/data/mysql/mandatory/sysprefs.sql >index 76954d2022..9f32ba048e 100644 >--- a/installer/data/mysql/mandatory/sysprefs.sql >+++ b/installer/data/mysql/mandatory/sysprefs.sql >@@ -385,6 +385,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` > ('LibraryThingForLibrariesTabbedView','0','','Put LibraryThingForLibraries Content in Tabs.','YesNo'), > ('LibrisKey', '', 'This key must be obtained at http://api.libris.kb.se/. It is unique for the IP of the server.', NULL, 'Free'), > ('LibrisURL', 'http://api.libris.kb.se/bibspell/', 'This it the base URL for the Libris spellchecking API.',NULL,'Free'), >+('LimitReportsByBranch', '0', NULL, 'Enable ability for staff to limit report access based on home library.', 'YesNo'), > ('LinkerConsiderDiacritics', '0', NULL, 'Linker should consider diacritics', 'YesNo'), > ('LinkerConsiderThesaurus','0',NULL,'If ON the authority linker will only search for 6XX authorities from the same source as the heading','YesNo'), > ('LinkerKeepStale','0',NULL,'If ON the authority linker will keep existing authority links for headings where it is unable to find a match.','YesNo'), >diff --git a/installer/data/mysql/mandatory/userpermissions.sql b/installer/data/mysql/mandatory/userpermissions.sql >index 1ff3f33d1e..23f8a47cbd 100644 >--- a/installer/data/mysql/mandatory/userpermissions.sql >+++ b/installer/data/mysql/mandatory/userpermissions.sql >@@ -141,6 +141,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES > (16, 'execute_reports', 'Execute SQL reports'), > (16, 'create_reports', 'Create SQL reports'), > (16, 'delete_reports', 'Delete SQL reports'), >+ (16, 'manage_report_limits', 'Manage report limits'), > (18, 'manage_courses', 'Add, edit and delete courses'), > (18, 'add_reserves', 'Add course reserves'), > (18, 'delete_reserves', 'Remove course reserves'), >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >index 9ee9543772..2ca47d3661 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc >@@ -487,6 +487,9 @@ > [%- CASE 'execute_reports' -%] > <span class="sub_permission execute_reports_subpermission"> Execute SQL reports </span> > <span class="permissioncode">([% name | html %])</span> >+ [%- CASE 'manage_report_limits' -%] >+ <span class="sub_permission manage_report_limits_subpermission"> Manage Report Limits </span> >+ <span class="permissioncode">([% name | html %])</span> > [%- CASE 'add_reserves' -%] > <span class="sub_permission add_reserves_subpermission"> Add course reserves </span> > <span class="permissioncode">([% name | html %])</span> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/includes/prefs-menu.inc b/koha-tmpl/intranet-tmpl/prog/en/includes/prefs-menu.inc >index 937e174112..001f4bcd97 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/includes/prefs-menu.inc >+++ b/koha-tmpl/intranet-tmpl/prog/en/includes/prefs-menu.inc >@@ -165,6 +165,17 @@ > </li> > [% END %] > >+ [% IF ( reports ) %] >+ <li class="active"> >+ <a title="Reports" href="/cgi-bin/koha/admin/preferences.pl?tab=reports">Reports</a> >+ [% PROCESS subtabs %] >+ </li> >+ [% ELSE %] >+ <li> >+ <a title="Reports" href="/cgi-bin/koha/admin/preferences.pl?tab=reports">Reports</a> >+ </li> >+ [% END %] >+ > [% IF ( searching ) %] > <li class="active"> > <a title="Searching" href="/cgi-bin/koha/admin/preferences.pl?tab=searching">Searching</a> >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref >new file mode 100644 >index 0000000000..6dbdd47e37 >--- /dev/null >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref >@@ -0,0 +1,8 @@ >+Reports: >+ Reports Access: >+ - >+ - pref: LimitReportsByBranch >+ choices: >+ 1: Enable >+ 0: Disable >+ - "limiting report access based on staff home library." >\ No newline at end of file >diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt >index b954e48a9d..cd3806bf00 100644 >--- a/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt >+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/reports/guided_reports_start.tt >@@ -3,6 +3,7 @@ > [% USE AuthorisedValues %] > [% USE KohaDates %] > [% USE Koha %] >+[% USE Branches %] > [% USE TablesSettings %] > [% USE HtmlScrubber %] > [% USE JSON.Escape %] >@@ -201,6 +202,15 @@ > > [% INCLUDE "reports-toolbar.inc" %] > >+ [% IF access_denied %] >+ <div> >+ <h1>Access denied</h1> >+ <p>This report is restricted and cannot be accessed by the current user.</p> >+ [% IF id %]<p>Report ID: [% id | html %]</p>[% END %] >+ <p><a class="btn btn-secondary" href="/cgi-bin/koha/reports/guided_reports.pl?op=list">Return to saved reports</a></p> >+ </div> >+ [% END %] >+ > [% IF ( start ) %] > <h1>Guided reports</h1> > <p>Use the guided reports engine to create non standard reports. This feature aims to provide some middle ground between the built in canned reports and writing custom SQL reports.</p> >@@ -236,7 +246,7 @@ > <h1>Saved reports</h1> > [% IF ( savedreports ) %] > >- [% IF ( filters.date || filters.author || filters.keyword ) %] >+ [% IF ( filters.date || filters.author || filters.keyword || branch_limit_active ) %] > <p > >Filtered by: > <span class="filter"> >@@ -249,6 +259,9 @@ > [% IF ( filters.keyword ) %] > <span class="filter_keyword"><strong>Keyword:</strong> [% filters.keyword | html %]</span> > [% END %] >+ [% IF branch_limit_active %] >+ <span class="filter_show_all"><strong>Show reports:</strong> My branch reports</span> >+ [% END %] > <a class="clear_filter" href="/cgi-bin/koha/reports/guided_reports.pl?op=list&clear_filters=1"><i class="fa fa-times" aria-hidden="true"></i> Clear</a> > </span> > </p> >@@ -309,6 +322,9 @@ > [% ELSE %] > <th class="NoVisible">Update</th> > [% END %] >+ [% IF ( can_manage_report_limits ) %] >+ <th>Library limits</th> >+ [% END %] > <th class="no-sort no-export">Actions</th> > </tr> > </thead> >@@ -376,6 +392,13 @@ > > > [% END %] > </td> >+ [% IF ( can_manage_report_limits ) %] >+ <td> >+ [% IF savedreport.library_limits %] >+ [% savedreport.library_limits.join(', ') %] >+ [% END %] >+ </td> >+ [% END %] > <td> > <div class="btn-group dropup"> > [%# There should be no space between these two buttons, it would render badly %] >@@ -455,7 +478,7 @@ > > <a href="/cgi-bin/koha/reports/guided_reports.pl?op=add_form_sql" class="new btn btn-default"><i class="fa fa-plus" aria-hidden="true"></i> New SQL report</a> > >- <a href="/cgi-bin/koha/reports/guided_reports.pl?op=list&filter_set=1&filter_keywork=" class="deny btn btn-default"><i class="fa fa-fw fa-times" aria-hidden="true"></i> Cancel filter</a> >+ <a href="/cgi-bin/koha/reports/guided_reports.pl?op=list&filter_set=1&clear_filters=1" class="deny btn btn-default"><i class="fa fa-fw fa-times" aria-hidden="true"></i> Cancel filter</a> > [% END %] > [% ELSE %] > <h4>There are no saved reports. </h4> >@@ -850,6 +873,7 @@ > <input type="hidden" name="reportname" value="[% reportname | html %]" /> > <input type="hidden" name="group" value="[% group | html %]" /> > <input type="hidden" name="subgroup" value="[% subgroup | html %]" /> >+ <input type="hidden" name="branches" value="[% branches | html %]" /> > <input type="hidden" name="notes" value="[% notes | scrub_html type => 'note' | $raw %]" /> > <input type="hidden" name="cache_expiry" value="[% cache_expiry | html %]" /> > <input type="hidden" name="cache_expiry_units" value="[% cache_expiry_units | html %]" /> >@@ -1434,7 +1458,18 @@ > <span class="required">Required</span> > </div> > </fieldset> >- >+ [% IF ( Koha.Preference('LimitReportsByBranch') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] >+ <fieldset class="rows"> >+ <legend>Library limitation:</legend> >+ <div >+ ><label for="library_limitation">Library limitation: </label> >+ <select id="library_limitation" name="branches" multiple size="10"> >+ [% PROCESS options_for_libraries libraries => Branches.all( selected => report.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %] >+ </select> >+ <div class="hint">Limits the use of this report to the selected libraries.</div> >+ </div> >+ </fieldset> >+ [% END %] > <fieldset class="action"> > <input type="hidden" name="op" value="cud-save" /> > <input type="submit" name="submit" class="btn btn-primary" value="Save report" /> >@@ -1548,7 +1583,18 @@ > <br /> > <span class="required" style="margin-left:30px;">Required</span> > </fieldset> >- >+ [% IF ( Koha.Preference('LimitReportsByBranch') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] >+ <fieldset class="rows"> >+ <legend>Library limitation:</legend> >+ <div >+ ><label for="library_limitation">Library limitation: </label> >+ <select id="library_limitation" name="branches" multiple size="10"> >+ [% PROCESS options_for_libraries libraries => Branches.all( selected => report.get_library_limits, unfiltered => 1, do_not_select_my_library => 1 ) %] >+ </select> >+ <div class="hint">Limits the use of this report to the selected libraries.</div> >+ </div> >+ </fieldset> >+ [% END %] > <fieldset class="action"> > <button class="btn btn-primary" type="submit" name="op" value="cud-update_sql">Update SQL</button> > <button class="btn btn-default" type="submit" name="op" value="cud-update_and_run_sql">Update and run SQL</button> >@@ -1616,6 +1662,26 @@ > <label for="filter_keyword">Keyword:</label> > <input type="text" id="filter_keyword" name="filter_keyword" value="[% filters.keyword | html %]" size="16" /> > </li> >+ <li> >+ [% IF Koha.Preference('LimitReportsByBranch') %] >+ [% IF ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) %] >+ <label for="filter_show_all_reports">Show reports:</label> >+ <select name="filter_show_all_reports" id="filter_show_all_reports"> >+ [% IF filters.show_all_reports == "1" %] >+ <option value="1" selected="selected">Show all reports</option> >+ [% ELSE %] >+ <option value="1">Show all reports</option> >+ [% END %] >+ >+ [% IF filters.show_all_reports == 0 %] >+ <option value="0" selected="selected">Show my branch reports</option> >+ [% ELSE %] >+ <option value="0">Show my branch reports</option> >+ [% END %] >+ </select> >+ [% END %] >+ [% END %] >+ </li> > </ol> > </fieldset> > <!-- /.brief --> >diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl >index fe47501ee7..11fd45e515 100755 >--- a/reports/guided_reports.pl >+++ b/reports/guided_reports.pl >@@ -83,19 +83,44 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > my $session_id = $input->cookie('CGISESSID'); > my $session = $session_id ? get_session($session_id) : undef; > >-$template->param( templates => Koha::Notice::Templates->search( { module => 'report' } ) ); >- > my $filter; > if ( $input->param("filter_set") or $input->param('clear_filters') ) { > $filter = {}; >- $filter->{$_} = $input->param("filter_$_") foreach qw/date author keyword group subgroup/; >+ $filter->{$_} = $input->param("filter_$_") foreach qw/date author keyword show_all_reports group subgroup/; >+ >+ if ( $input->param('clear_filters') ) { >+ $filter->{show_all_reports} = 1; >+ } >+ > $session->param( 'report_filter', $filter ) if $session; > $template->param( 'filter_set' => 1 ); > } elsif ( $session and not $input->param('clear_filters') ) { > $filter = $session->param('report_filter'); > } > >-my @errors = (); >+my @errors = (); >+my %OP_REQUIRES_REPORT = map { $_ => 1 } qw(show edit_form duplicate cud-update_sql cud-update_and_run_sql export run); >+my $report_id = $input->param('id'); >+my $report; >+my $access_blocked; >+if ( $OP_REQUIRES_REPORT{$op} && $report_id ) { >+ $report = Koha::Reports->find($report_id); >+ if ($report) { >+ my $patron = Koha::Patrons->find($borrowernumber); >+ unless ( $report->can_access($patron) ) { >+ $template->param( access_denied => 1, denied_op => $op, id => $report_id ); >+ $access_blocked = 1; >+ } >+ } else { >+ push @errors, { no_sql_for_id => $report_id }; >+ } >+} >+ >+if ($access_blocked) { >+ output_html_with_http_headers $input, $cookie, $template->output; >+ exit; >+} >+ > if ( !$op ) { > $template->param( 'start' => 1 ); > >@@ -116,147 +141,156 @@ if ( !$op ) { > $op = 'list'; > > } elsif ( $op eq 'show' ) { >- >- my $id = $input->param('id'); >- my $report = Koha::Reports->find($id); >- $template->param( >- 'id' => $id, >- 'reportname' => $report->report_name, >- 'notes' => $report->notes, >- 'sql' => $report->savedsql, >- 'showsql' => 1, >- 'mana_success' => scalar $input->param('mana_success'), >- 'mana_id' => $report->{mana_id}, >- 'mana_comments' => $report->{comments} >- ); >+ if ($report) { >+ $template->param( >+ 'id' => $report_id, >+ 'reportname' => $report->report_name, >+ 'notes' => $report->notes, >+ 'sql' => $report->savedsql, >+ 'showsql' => 1, >+ 'mana_success' => scalar $input->param('mana_success'), >+ 'mana_id' => $report->{mana_id}, >+ 'mana_comments' => $report->{comments} >+ ); >+ } > > } elsif ( $op eq 'edit_form' ) { >- my $id = $input->param('id'); >- my $report = Koha::Reports->find($id); >- my $group = $report->report_group; >- my $subgroup = $report->report_subgroup; >- my $tables = get_tables(); >- $template->param( >- 'sql' => $report->savedsql, >- 'reportname' => $report->report_name, >- 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >- 'notes' => $report->notes, >- 'id' => $id, >- 'cache_expiry' => $report->cache_expiry, >- 'public' => $report->public, >- 'usecache' => $usecache, >- 'editsql' => 1, >- 'mana_id' => $report->{mana_id}, >- 'mana_comments' => $report->{comments}, >- 'tables' => $tables >- ); >+ if ($report) { >+ my $group = $report->report_group; >+ my $subgroup = $report->report_subgroup; >+ my $tables = get_tables(); >+ $template->param( >+ 'sql' => $report->savedsql, >+ 'reportname' => $report->report_name, >+ 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >+ 'notes' => $report->notes, >+ 'id' => $report_id, >+ 'cache_expiry' => $report->cache_expiry, >+ 'public' => $report->public, >+ 'usecache' => $usecache, >+ 'editsql' => 1, >+ 'mana_id' => $report->{mana_id}, >+ 'mana_comments' => $report->{comments}, >+ 'tables' => $tables, >+ 'report' => $report, >+ ); >+ } > > } elsif ( $op eq 'cud-update_sql' || $op eq 'cud-update_and_run_sql' ) { >- my $id = $input->param('id'); >- my $sql = $input->param('sql'); >- my $reportname = $input->param('reportname'); >- my $group = $input->param('group'); >- my $subgroup = $input->param('subgroup'); >- my $notes = $input->param('notes'); >- my $cache_expiry = $input->param('cache_expiry'); >- my $cache_expiry_units = $input->param('cache_expiry_units'); >- my $public = $input->param('public'); >- my $save_anyway = $input->param('save_anyway'); >- my @errors; >- my $tables = get_tables(); >+ if ($report) { >+ my $id = $report_id; >+ my $sql = $input->param('sql'); >+ my $reportname = $input->param('reportname'); >+ my $group = $input->param('group'); >+ my $subgroup = $input->param('subgroup'); >+ my $notes = $input->param('notes'); >+ my $cache_expiry = $input->param('cache_expiry'); >+ my $cache_expiry_units = $input->param('cache_expiry_units'); >+ my $public = $input->param('public'); >+ my $save_anyway = $input->param('save_anyway'); >+ my @errors; >+ my $tables = get_tables(); >+ my @branches = grep { $_ ne q{} } $input->multi_param('branches'); >+ >+ # if we have the units, then we came from creating a report from SQL and thus need to handle converting units >+ if ($cache_expiry_units) { >+ if ( $cache_expiry_units eq "minutes" ) { >+ $cache_expiry *= 60; >+ } elsif ( $cache_expiry_units eq "hours" ) { >+ $cache_expiry *= 3600; # 60 * 60 >+ } elsif ( $cache_expiry_units eq "days" ) { >+ $cache_expiry *= 86400; # 60 * 60 * 24 >+ } >+ } > >- # if we have the units, then we came from creating a report from SQL and thus need to handle converting units >- if ($cache_expiry_units) { >- if ( $cache_expiry_units eq "minutes" ) { >- $cache_expiry *= 60; >- } elsif ( $cache_expiry_units eq "hours" ) { >- $cache_expiry *= 3600; # 60 * 60 >- } elsif ( $cache_expiry_units eq "days" ) { >- $cache_expiry *= 86400; # 60 * 60 * 24 >+ # check $cache_expiry isn't too large, Memcached::set requires it to be less than 30 days or it will be treated as if it were an absolute time stamp >+ if ( $cache_expiry >= 2592000 ) { >+ push @errors, { cache_expiry => $cache_expiry }; > } >- } > >- # check $cache_expiry isn't too large, Memcached::set requires it to be less than 30 days or it will be treated as if it were an absolute time stamp >- if ( $cache_expiry >= 2592000 ) { >- push @errors, { cache_expiry => $cache_expiry }; >- } >+ create_non_existing_group_and_subgroup( $input, $group, $subgroup ); > >- create_non_existing_group_and_subgroup( $input, $group, $subgroup ); >+ my ( $is_sql_valid, $validation_errors ) = Koha::Report->new( { savedsql => $sql } )->is_sql_valid; >+ push( @errors, @$validation_errors ) unless $is_sql_valid; > >- my ( $is_sql_valid, $validation_errors ) = Koha::Report->new( { savedsql => $sql } )->is_sql_valid; >- push( @errors, @$validation_errors ) unless $is_sql_valid; >+ if (@errors) { >+ $template->param( >+ 'errors' => \@errors, >+ 'sql' => $sql, >+ ); >+ } else { > >- if (@errors) { >- $template->param( >- 'errors' => \@errors, >- 'sql' => $sql, >- ); >- } else { >+ # Check defined SQL parameters for authorised value validity >+ my $problematic_authvals = ValidateSQLParameters($sql); > >- # Check defined SQL parameters for authorised value validity >- my $problematic_authvals = ValidateSQLParameters($sql); >+ if ( scalar @$problematic_authvals > 0 && not $save_anyway ) { > >- if ( scalar @$problematic_authvals > 0 && not $save_anyway ) { >+ # There's at least one problematic parameter, report to the >+ # GUI and provide all user input for further actions >+ $template->param( >+ 'id' => $id, >+ 'sql' => $sql, >+ 'reportname' => $reportname, >+ 'group' => $group, >+ 'subgroup' => $subgroup, >+ 'notes' => $notes, >+ 'public' => $public, >+ 'problematic_authvals' => $problematic_authvals, >+ 'warn_authval_problem' => 1, >+ 'phase_update' => 1, >+ 'branches' => @branches, >+ 'report' => $report, >+ ); > >- # There's at least one problematic parameter, report to the >- # GUI and provide all user input for further actions >- $template->param( >- 'id' => $id, >- 'sql' => $sql, >- 'reportname' => $reportname, >- 'group' => $group, >- 'subgroup' => $subgroup, >- 'notes' => $notes, >- 'public' => $public, >- 'problematic_authvals' => $problematic_authvals, >- 'warn_authval_problem' => 1, >- 'phase_update' => 1, >- ); >+ } else { > >- } else { >+ # No params problem found or asked to save anyway >+ update_sql( >+ $id, >+ { >+ sql => $sql, >+ name => $reportname, >+ group => $group, >+ subgroup => $subgroup, >+ notes => $notes, >+ public => $public, >+ cache_expiry => $cache_expiry, >+ } >+ ); > >- # No params problem found or asked to save anyway >- update_sql( >- $id, >- { >- sql => $sql, >- name => $reportname, >- group => $group, >- subgroup => $subgroup, >- notes => $notes, >- public => $public, >- cache_expiry => $cache_expiry, >+ $report->store; >+ $report->replace_library_limits( \@branches ); >+ >+ my $editsql = 1; >+ if ( $op eq 'cud-update_and_run_sql' ) { >+ $editsql = 0; > } >- ); > >- my $editsql = 1; >+ $template->param( >+ 'save_successful' => 1, >+ 'reportname' => $reportname, >+ 'id' => $id, >+ 'editsql' => $editsql, >+ 'sql' => $sql, >+ 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >+ 'notes' => $notes, >+ 'cache_expiry' => $cache_expiry, >+ 'public' => $public, >+ 'usecache' => $usecache, >+ 'tables' => $tables, >+ 'report' => $report >+ ); >+ logaction( "REPORTS", "MODIFY", $id, "$reportname | $sql" ) if C4::Context->preference("ReportsLog"); >+ } >+ if ($usecache) { >+ $template->param( >+ cache_expiry => $cache_expiry, >+ cache_expiry_units => $cache_expiry_units, >+ ); >+ } > if ( $op eq 'cud-update_and_run_sql' ) { >- $editsql = 0; >+ $op = 'run'; > } >- >- $template->param( >- 'save_successful' => 1, >- 'reportname' => $reportname, >- 'id' => $id, >- 'editsql' => $editsql, >- 'sql' => $sql, >- 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >- 'notes' => $notes, >- 'cache_expiry' => $cache_expiry, >- 'public' => $public, >- 'usecache' => $usecache, >- 'tables' => $tables >- ); >- logaction( "REPORTS", "MODIFY", $id, "$reportname | $sql" ) if C4::Context->preference("ReportsLog"); >- } >- if ($usecache) { >- $template->param( >- cache_expiry => $cache_expiry, >- cache_expiry_units => $cache_expiry_units, >- ); >- } >- if ( $op eq 'cud-update_and_run_sql' ) { >- $op = 'run'; > } > } > >@@ -514,6 +548,7 @@ if ( !$op ) { > my $public = $input->param('public'); > my $save_anyway = $input->param('save_anyway'); > my $tables = get_tables(); >+ my @branches = grep { $_ ne q{} } $input->multi_param('branches'); > > # if we have the units, then we came from creating a report from SQL and thus need to handle converting units > if ($cache_expiry_units) { >@@ -591,6 +626,9 @@ if ( !$op ) { > public => $public, > } > ); >+ my $report = Koha::Reports->find($id); >+ $report->replace_library_limits( \@branches ); >+ > logaction( "REPORTS", "ADD", $id, "$name | $sql" ) if C4::Context->preference("ReportsLog"); > $template->param( > 'save_successful' => 1, >@@ -603,7 +641,8 @@ if ( !$op ) { > 'cache_expiry' => $cache_expiry, > 'public' => $public, > 'usecache' => $usecache, >- 'tables' => $tables >+ 'tables' => $tables, >+ 'report' => $report, > ); > } > } >@@ -621,145 +660,150 @@ if ( !$op ) { > } elsif ( $op eq 'export' ) { > > # export results to tab separated text or CSV >- my $report_id = $input->param('id'); >- my $report = Koha::Reports->find($report_id); >- my $sql = $report->savedsql; >- my @param_names = $input->multi_param('param_name'); >- my @sql_params = $input->multi_param('sql_params'); >- my $format = $input->param('format'); >- my $reportname = $input->param('reportname'); >- my $reportfilename = >- $reportname ? "$report_id-$reportname-reportresults.$format" : "$report_id-reportresults.$format"; >- my $scrubber = C4::Scrubber->new(); >- >- ( $sql, undef ) = $report->prep_report( \@param_names, \@sql_params, { export => 1 } ); >- my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); >- unless ( $q_errors and @$q_errors ) { >- my ( $type, $content ); >- if ( $format eq 'tab' ) { >- $type = 'application/octet-stream'; >- $content .= join( "\t", header_cell_values($sth) ) . "\n"; >- $content = $scrubber->scrub( Encode::decode( 'UTF-8', $content ) ); >- while ( my $row = $sth->fetchrow_arrayref() ) { >- $content .= $scrubber->scrub( join( "\t", map { $_ // '' } @$row ) ) . "\n"; >- } >- } else { >- if ( $format eq 'csv' ) { >- my $delimiter = C4::Context->csv_delimiter; >- $type = 'application/csv'; >- >- # Add BOM for UTF-8 encoded CSV >- $content .= "\xEF\xBB\xBF"; >+ if ($report) { >+ my $sql = $report->savedsql; >+ my @param_names = $input->multi_param('param_name'); >+ my @sql_params = $input->multi_param('sql_params'); >+ my $format = $input->param('format'); >+ my $reportname = $input->param('reportname'); >+ my $reportfilename = >+ $reportname ? "$report_id-$reportname-reportresults.$format" : "$report_id-reportresults.$format"; >+ my $scrubber = C4::Scrubber->new(); >+ >+ ( $sql, undef ) = $report->prep_report( \@param_names, \@sql_params, { export => 1 } ); >+ my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); >+ unless ( $q_errors and @$q_errors ) { >+ my ( $type, $content ); >+ if ( $format eq 'tab' ) { >+ $type = 'application/octet-stream'; >+ $content .= join( "\t", header_cell_values($sth) ) . "\n"; >+ $content = $scrubber->scrub( Encode::decode( 'UTF-8', $content ) ); >+ while ( my $row = $sth->fetchrow_arrayref() ) { >+ $content .= $scrubber->scrub( join( "\t", map { $_ // '' } @$row ) ) . "\n"; >+ } >+ } else { >+ if ( $format eq 'csv' ) { >+ my $delimiter = C4::Context->csv_delimiter; >+ $type = 'application/csv'; > >- my $csv = >- Text::CSV::Encoded->new( { encoding_out => 'UTF-8', sep_char => $delimiter, formula => 'empty' } ); >- $csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag(); >- if ( $csv->combine( header_cell_values($sth) ) ) { >- $content .= $scrubber->scrub( Encode::decode( 'UTF-8', $csv->string() ) ) . "\n"; >+ # Add BOM for UTF-8 encoded CSV >+ $content .= "\xEF\xBB\xBF"; > >- } else { >- push @$q_errors, { combine => 'HEADER ROW: ' . $csv->error_diag() }; >- } >- while ( my $row = $sth->fetchrow_arrayref() ) { >- if ( $csv->combine(@$row) ) { >- $content .= $scrubber->scrub( $csv->string() ) . "\n"; >+ my $csv = >+ Text::CSV::Encoded->new( >+ { encoding_out => 'UTF-8', sep_char => $delimiter, formula => 'empty' } ); >+ $csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag(); >+ if ( $csv->combine( header_cell_values($sth) ) ) { >+ $content .= $scrubber->scrub( Encode::decode( 'UTF-8', $csv->string() ) ) . "\n"; > > } else { >- push @$q_errors, { combine => $csv->error_diag() }; >+ push @$q_errors, { combine => 'HEADER ROW: ' . $csv->error_diag() }; > } >- } >- } elsif ( $format eq 'ods' && C4::Context->preference('ReportsExportFormatODS') ) { >- $type = 'application/vnd.oasis.opendocument.spreadsheet'; >- my $ods_fh = File::Temp->new( UNLINK => 0 ); >- my $ods_filepath = $ods_fh->filename; >- my $ods_content; >- >- # First line is headers >- my @headers = header_cell_values($sth); >- push @$ods_content, \@headers; >- >- # Other line in Unicode >- my $sql_rows = $sth->fetchall_arrayref(); >- foreach my $sql_row (@$sql_rows) { >- my @content_row; >- foreach my $sql_cell (@$sql_row) { >- push @content_row, $scrubber->scrub( Encode::encode( 'UTF8', $sql_cell ) ); >+ while ( my $row = $sth->fetchrow_arrayref() ) { >+ if ( $csv->combine(@$row) ) { >+ $content .= $scrubber->scrub( $csv->string() ) . "\n"; >+ >+ } else { >+ push @$q_errors, { combine => $csv->error_diag() }; >+ } >+ } >+ } elsif ( $format eq 'ods' && C4::Context->preference('ReportsExportFormatODS') ) { >+ $type = 'application/vnd.oasis.opendocument.spreadsheet'; >+ my $ods_fh = File::Temp->new( UNLINK => 0 ); >+ my $ods_filepath = $ods_fh->filename; >+ my $ods_content; >+ >+ # First line is headers >+ my @headers = header_cell_values($sth); >+ push @$ods_content, \@headers; >+ >+ # Other line in Unicode >+ my $sql_rows = $sth->fetchall_arrayref(); >+ foreach my $sql_row (@$sql_rows) { >+ my @content_row; >+ foreach my $sql_cell (@$sql_row) { >+ push @content_row, $scrubber->scrub( Encode::encode( 'UTF8', $sql_cell ) ); > >+ } >+ push @$ods_content, \@content_row; > } >- push @$ods_content, \@content_row; >- } > >- # Process >- generate_ods( $ods_filepath, $ods_content ); >+ # Process >+ generate_ods( $ods_filepath, $ods_content ); >+ >+ # Output >+ binmode(STDOUT); >+ open $ods_fh, '<', $ods_filepath; >+ print $input->header( >+ -type => $type, >+ -attachment => $reportfilename >+ ); >+ print $_ while <$ods_fh>; >+ unlink $ods_filepath; >+ } elsif ( $format eq 'template' ) { >+ my $template_id = $input->param('template'); >+ my $notice_template = Koha::Notice::Templates->find($template_id); >+ my $data = $sth->fetchall_arrayref( {} ); >+ $content = process_tt( >+ $notice_template->content, >+ { >+ data => $data, >+ report_id => $report_id, >+ for_download => 1, >+ } >+ ); >+ $reportfilename = process_tt( >+ $notice_template->title, >+ { >+ data => $data, >+ report_id => $report_id, >+ } >+ ); >+ } >+ } > >- # Output >- binmode(STDOUT); >- open $ods_fh, '<', $ods_filepath; >+ unless ( $format eq 'ods' ) { > print $input->header( > -type => $type, > -attachment => $reportfilename > ); >- print $_ while <$ods_fh>; >- unlink $ods_filepath; >- } elsif ( $format eq 'template' ) { >- my $template_id = $input->param('template'); >- my $notice_template = Koha::Notice::Templates->find($template_id); >- my $data = $sth->fetchall_arrayref( {} ); >- $content = process_tt( >- $notice_template->content, >- { >- data => $data, >- report_id => $report_id, >- for_download => 1, >- } >- ); >- $reportfilename = process_tt( >- $notice_template->title, >- { >- data => $data, >- report_id => $report_id, >- } >- ); >+ print $content; > } >- } > >- unless ( $format eq 'ods' ) { >- print $input->header( >- -type => $type, >- -attachment => $reportfilename >- ); >- print $content; >+ foreach my $err ( @$q_errors, @errors ) { >+ print "# ERROR: " . ( map { $_ . ": " . $err->{$_} } keys %$err ) . "\n"; >+ } # here we print all the non-fatal errors at the end. Not super smooth, but better than nothing. >+ exit; > } >- >- foreach my $err ( @$q_errors, @errors ) { >- print "# ERROR: " . ( map { $_ . ": " . $err->{$_} } keys %$err ) . "\n"; >- } # here we print all the non-fatal errors at the end. Not super smooth, but better than nothing. >- exit; >+ $template->param( >+ 'sql' => $sql, >+ 'execute' => 1, >+ 'name' => 'Error exporting report!', >+ 'notes' => '', >+ 'errors' => $q_errors, >+ ); > } >- $template->param( >- 'sql' => $sql, >- 'execute' => 1, >- 'name' => 'Error exporting report!', >- 'notes' => '', >- 'errors' => $q_errors, >- ); > > } elsif ( $op eq 'add_form_sql' || $op eq 'duplicate' ) { > >- my ( $group, $subgroup, $sql, $reportname, $notes ); >+ my ( $group, $subgroup, $sql, $reportname, $notes, @branches, $report ); > if ( $input->param('sql') ) { > $group = $input->param('report_group'); > $subgroup = $input->param('report_subgroup'); > $sql = $input->param('sql') // ''; > $reportname = $input->param('reportname') // ''; > $notes = $input->param('notes') // ''; >+ @branches = grep { $_ ne q{} } $input->multi_param('branches'); >+ > } elsif ( my $report_id = $input->param('id') ) { >- my $report = Koha::Reports->find($report_id); >+ $report = Koha::Reports->find($report_id); > $group = $report->report_group; > $subgroup = $report->report_subgroup; > $sql = $report->savedsql // ''; > $reportname = $report->report_name // ''; > $notes = $report->notes // ''; >+ @branches = grep { $_ ne q{} } $input->multi_param('branches'); >+ > } > > my $tables = get_tables(); >@@ -774,6 +818,7 @@ if ( !$op ) { > 'cache_expiry' => 300, > 'usecache' => $usecache, > 'tables' => $tables, >+ 'report' => $report, > > ); > } >@@ -783,7 +828,6 @@ if ( $op eq 'run' ) { > # execute a saved report > my $limit = $input->param('limit') || 20; > my $offset = 0; >- my $report_id = $input->param('id'); > my @sql_params = $input->multi_param('sql_params'); > my @param_names = $input->multi_param('param_name'); > my $template_id = $input->param('template'); >@@ -800,7 +844,7 @@ if ( $op eq 'run' ) { > ); > > my ( $sql, $original_sql, $type, $name, $notes ); >- if ( my $report = Koha::Reports->find($report_id) ) { >+ if ($report) { > $sql = $original_sql = $report->savedsql; > $name = $report->report_name; > $notes = $report->notes; >@@ -1047,8 +1091,6 @@ if ( $op eq 'run' ) { > 'param_names' => \@param_names, > ); > } >- } else { >- push @errors, { no_sql_for_id => $report_id }; > } > } > >@@ -1077,11 +1119,27 @@ if ( $op eq 'list' || $op eq 'convert' ) { > > # use a saved report > # get list of reports and display them >- my $group = $input->param('group'); >- my $subgroup = $input->param('subgroup'); >+ my $group = $input->param('group'); >+ my $subgroup = $input->param('subgroup'); >+ my $show_all_filter = $filter->{show_all_reports}; >+ my $logged_in_user = Koha::Patrons->find($borrowernumber); >+ my $can_manage_limits = Koha::Report->can_manage_limits($logged_in_user); > $filter->{group} = $group; > $filter->{subgroup} = $subgroup; >+ >+ if ($show_all_filter) { >+ $filter->{show_all_reports} = $can_manage_limits ? $show_all_filter : 0; >+ } else { >+ $filter->{show_all_reports} = 0; >+ } >+ > my $reports = get_saved_reports($filter); >+ >+ my $branch_limit_active = >+ C4::Context->preference('LimitReportsByBranch') >+ && $can_manage_limits >+ && !$filter->{show_all_reports}; >+ > my $has_obsolete_reports; > for my $report (@$reports) { > $report->{results} = C4::Reports::Guided::get_results( $report->{id} ); >@@ -1091,13 +1149,15 @@ if ( $op eq 'list' || $op eq 'convert' ) { > } > } > $template->param( >- 'manamsg' => $input->param('manamsg') || '', >- 'saved1' => 1, >- 'savedreports' => $reports, >- 'usecache' => $usecache, >- 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >- filters => $filter, >- has_obsolete_reports => $has_obsolete_reports, >+ 'manamsg' => $input->param('manamsg') || '', >+ 'saved1' => 1, >+ 'savedreports' => $reports, >+ 'usecache' => $usecache, >+ 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), >+ filters => $filter, >+ has_obsolete_reports => $has_obsolete_reports, >+ branch_limit_active => $branch_limit_active, >+ can_manage_report_limits => $can_manage_limits, > ); > } > >@@ -1208,4 +1268,3 @@ sub create_non_existing_group_and_subgroup { > } > } > } >- >diff --git a/t/Koha/Auth/Permissions.t b/t/Koha/Auth/Permissions.t >index 0fc4ba108a..6ac523dc8c 100755 >--- a/t/Koha/Auth/Permissions.t >+++ b/t/Koha/Auth/Permissions.t >@@ -63,6 +63,7 @@ subtest 'normal staff user test' => sub { > 'CAN_user_reports_create_reports' => 1, > 'CAN_user_reports_delete_reports' => 1, > 'CAN_user_reports_execute_reports' => 1, >+ 'CAN_user_reports_manage_report_limits' => 1, > }; > is_deeply( $authz, $expected, 'Expected permissions generated for normal staff user' ); > }; >@@ -228,6 +229,7 @@ subtest 'superlibrarian tests' => sub { > 'CAN_user_reports_create_reports' => 1, > 'CAN_user_reports_delete_reports' => 1, > 'CAN_user_reports_execute_reports' => 1, >+ 'CAN_user_reports_manage_report_limits' => 1, > 'CAN_user_reports' => 1, > 'CAN_user_reserveforothers_modify_holds_priority' => 1, > 'CAN_user_reserveforothers_place_holds' => 1, >diff --git a/t/db_dependent/Koha/Reports.t b/t/db_dependent/Koha/Reports.t >index 205b1df60a..051392fc21 100755 >--- a/t/db_dependent/Koha/Reports.t >+++ b/t/db_dependent/Koha/Reports.t >@@ -18,7 +18,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 9; >+use Test::More tests => 11; > > use Koha::Report; > use Koha::Reports; >@@ -185,4 +185,106 @@ subtest '_might_add_limit' => sub { > ); > }; > >+subtest 'reports_branches are added and removed from report_branches table' => sub { >+ plan tests => 4; >+ >+ my $updated_nb_of_reports = Koha::Reports->search->count; >+ my $report = Koha::Report->new( >+ { >+ report_name => 'report_name_for_test_1', >+ savedsql => 'SELECT * FROM items WHERE itemnumber IN <<Test|list>>', >+ } >+ )->store; >+ >+ my $id = $report->id; >+ my $library1 = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $library2 = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $library3 = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my @branches = ( $library1->branchcode, $library2->branchcode, $library3->branchcode ); >+ >+ $report->replace_library_limits( \@branches ); >+ >+ my @branches_loop = $report->get_library_limits->as_list; >+ is( scalar @branches_loop, 3, '3 branches added to report_branches table' ); >+ >+ $report->replace_library_limits( [ $library1->branchcode, $library2->branchcode ] ); >+ >+ @branches_loop = $report->get_library_limits->as_list; >+ is( scalar @branches_loop, 2, '1 branch removed from report_branches table' ); >+ >+ $report->delete; >+ is( Koha::Reports->search->count, $updated_nb_of_reports, 'Report deleted, count is back to original' ); >+ is( >+ $schema->resultset('ReportsBranch')->search( { report_id => $id } )->count, >+ 0, >+ 'No branches left in reports_branches table after report deletion' >+ ); >+}; >+ >+subtest 'can_manage_limits and can_access' => sub { >+ plan tests => 21; >+ >+ my $libraryA = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $libraryB = $builder->build_object( { class => 'Koha::Libraries' } ); >+ my $branchA = $libraryA->branchcode; >+ my $branchB = $libraryB->branchcode; >+ >+ my $super_patron = >+ $builder->build_object( { class => 'Koha::Patrons', value => { flags => 1, branchcode => $branchA } } ); >+ my $mgr_patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $branchA } } ); >+ my $basic_patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $branchA } } ); >+ >+ # Grant reports => manage_report_limits to manager patron (create permission if missing) >+ my $perm = $schema->resultset('Permission')->find( { module_bit => 16, code => 'manage_report_limits' } ) >+ // $schema->resultset('Permission') >+ ->create( { module_bit => 16, code => 'manage_report_limits', description => 'Manage report limits' } ); >+ $schema->resultset('UserPermission') >+ ->create( { borrowernumber => $mgr_patron->borrowernumber, module_bit => 16, code => 'manage_report_limits' } ); >+ >+ # Helper to create reports >+ my $r_no = Koha::Report->new( { report_name => 'No limits', savedsql => 'SELECT 1' } )->store; >+ my $r_A = Koha::Report->new( { report_name => 'Limit A', savedsql => 'SELECT 1' } )->store; >+ my $r_B = Koha::Report->new( { report_name => 'Limit B', savedsql => 'SELECT 1' } )->store; >+ my $r_AB = Koha::Report->new( { report_name => 'Limit AB', savedsql => 'SELECT 1' } )->store; >+ >+ $r_A->replace_library_limits( [$branchA] ); >+ $r_B->replace_library_limits( [$branchB] ); >+ $r_AB->replace_library_limits( [ $branchA, $branchB ] ); >+ >+ # Preference ON, branch A >+ t::lib::Mocks::mock_preference( 'LimitReportsByBranch', 1 ); >+ t::lib::Mocks::mock_userenv( { branchcode => $branchA } ); >+ >+ ok( Koha::Report->can_manage_limits($super_patron), 'pref ON: super manages limits' ); >+ ok( Koha::Report->can_manage_limits($mgr_patron), 'pref ON: manager manages limits' ); >+ ok( !Koha::Report->can_manage_limits($basic_patron), 'pref ON: basic cannot manage limits' ); >+ >+ # Basic patron (branch A) >+ ok( $r_no->can_access($basic_patron), 'pref ON: no limits accessible' ); >+ ok( $r_A->can_access($basic_patron), 'pref ON: limited includes branch accessible' ); >+ ok( !$r_B->can_access($basic_patron), 'pref ON: limited excludes branch denied' ); >+ ok( $r_AB->can_access($basic_patron), 'pref ON: multi includes branch accessible' ); >+ >+ # Manager bypass (branch A) >+ ok( $r_A->can_access($mgr_patron), 'pref ON: manager sees limited A' ); >+ ok( $r_B->can_access($mgr_patron), 'pref ON: manager sees limited B' ); >+ ok( $r_AB->can_access($mgr_patron), 'pref ON: manager sees multi AB' ); >+ ok( $r_no->can_access($mgr_patron), 'pref ON: manager sees no limits' ); >+ >+ # Superlibrarian bypass (branch A) >+ ok( $r_A->can_access($super_patron), 'pref ON: super sees limited A' ); >+ ok( $r_B->can_access($super_patron), 'pref ON: super sees limited B' ); >+ ok( $r_AB->can_access($super_patron), 'pref ON: super sees multi AB' ); >+ >+ # Preference OFF (everything accessible, manage disabled) >+ t::lib::Mocks::mock_preference( 'LimitReportsByBranch', 0 ); >+ ok( !Koha::Report->can_manage_limits($super_patron), 'pref OFF: super cannot manage limits' ); >+ ok( !Koha::Report->can_manage_limits($mgr_patron), 'pref OFF: manager cannot manage limits' ); >+ ok( !Koha::Report->can_manage_limits($basic_patron), 'pref OFF: basic cannot manage limits' ); >+ ok( $r_B->can_access($basic_patron), 'pref OFF: limited excludes branch still accessible' ); >+ ok( $r_A->can_access($basic_patron), 'pref OFF: limited includes branch accessible' ); >+ ok( $r_AB->can_access($basic_patron), 'pref OFF: multi-limit accessible' ); >+ ok( $r_no->can_access($basic_patron), 'pref OFF: no limits accessible' ); >+}; >+ > $schema->storage->txn_rollback; >diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t >index fdb0213dd6..c8d3185f1c 100755 >--- a/t/db_dependent/Reports/Guided.t >+++ b/t/db_dependent/Reports/Guided.t >@@ -21,7 +21,7 @@ > use Modern::Perl; > > use Test::NoWarnings; >-use Test::More tests => 14; >+use Test::More tests => 15; > use Test::Warn; > > use t::lib::TestBuilder; >@@ -324,6 +324,91 @@ subtest 'get_saved_reports' => sub { > ); > }; > >+subtest 'get_saved_reports branch limits' => sub { >+ my $schema = Koha::Database->new->schema; >+ my $dbh = C4::Context->dbh; >+ >+ # Skip gracefully if the feature table is not present (e.g. older schema) >+ my $sth = $dbh->prepare(q{SHOW TABLES LIKE 'reports_library_limits'}); >+ $sth->execute; >+ my ($exists) = $sth->fetchrow_array; >+ unless ($exists) { >+ plan skip_all => 'reports_library_limits table not present; skipping branch limits tests'; >+ return; >+ } >+ >+ plan tests => 13; >+ >+ # Clean related tables >+ $dbh->do(q|DELETE FROM saved_sql|); >+ $dbh->do(q|DELETE FROM saved_reports|); >+ $dbh->do(q|DELETE FROM reports_library_limits|); >+ >+ # Build two libraries (branches) >+ my $lib_a = $builder->build( { source => 'Branch', value => 'Library A' } ); >+ my $lib_b = $builder->build( { source => 'Branch', value => 'Library B' } ); >+ >+ # Mock userenv to lib_a >+ t::lib::Mocks::mock_userenv( { branchcode => $lib_a->{branchcode} } ); >+ >+ # Build three reports >+ my @ids; >+ foreach my $n ( 1 .. 3 ) { >+ push @ids, save_report( >+ { >+ borrowernumber => $builder->build( { source => 'Borrower' } )->{borrowernumber}, >+ sql => "SELECT $n", >+ name => "Report $n", >+ area => 'CIRC', >+ group => 'G', >+ subgroup => 'SG', >+ type => 'T', >+ notes => undef, >+ cache_expiry => undef, >+ public => 0, >+ } >+ ); >+ } >+ >+ my ( $r1, $r2, $r3 ) = map { Koha::Reports->find($_) } @ids; >+ >+ # Apply library limits to first two reports (r1 => lib_a only, r2 => lib_b only) >+ $r1->replace_library_limits( [ $lib_a->{branchcode} ] ); >+ $r2->replace_library_limits( [ $lib_b->{branchcode} ] ); >+ >+ # r3 has no limits >+ >+ # Preference OFF: should ignore limits, return all three, no library_limits metadata >+ t::lib::Mocks::mock_preference( 'LimitReportsByBranch', 0 ); >+ my $all_pref_off = get_saved_reports(); >+ is( scalar(@$all_pref_off), 3, 'Pref OFF returns all reports' ); >+ ok( !defined $all_pref_off->[0]{library_limits}, 'Pref OFF does not attach metadata' ); >+ >+ # Preference ON, filtering applied (userenv branch is lib_a) >+ t::lib::Mocks::mock_preference( 'LimitReportsByBranch', 1 ); >+ my $filtered = get_saved_reports(); >+ is( scalar(@$filtered), 2, 'Pref ON filters reports by user branch and unrestricted reports' ); >+ my @filtered_ids = sort map { $_->{id} } @$filtered; >+ my @expected_ids = sort ( $r1->id, $r3->id ); # r1 limited to lib_a + r3 unrestricted >+ is_deeply( \@filtered_ids, \@expected_ids, 'Filtered list contains branch-limited and unrestricted reports' ); >+ foreach my $r (@$filtered) { >+ ok( defined $r->{library_limits}, 'Metadata attached on pref ON filtered path' ); >+ } >+ my ($meta_r1) = grep { $_->{id} == $r1->id } @$filtered; >+ is_deeply( $meta_r1->{library_limits}, [ $lib_a->{branchcode} ], 'r1 metadata lists its single limit' ); >+ my ($meta_r3) = grep { $_->{id} == $r3->id } @$filtered; >+ is_deeply( $meta_r3->{library_limits}, [], 'Unrestricted r3 has empty limits array' ); >+ >+ # Preference ON + show_all_reports flag: should return all, with metadata for all >+ my $show_all = get_saved_reports( { show_all_reports => 1 } ); >+ is( scalar(@$show_all), 3, 'show_all_reports returns all reports when pref ON' ); >+ foreach my $r (@$show_all) { >+ ok( defined $r->{library_limits}, 'Metadata attached for each report in show_all path' ); >+ } >+ my ($show_r2) = grep { $_->{id} == $r2->id } @$show_all; >+ is_deeply( $show_r2->{library_limits}, [ $lib_b->{branchcode} ], 'r2 metadata lists its limit under show_all' ); >+}; >+ > subtest 'Ensure last_run is populated' => sub { > plan tests => 3; > >diff --git a/tools/scheduler.pl b/tools/scheduler.pl >index 37d5a06883..16c1a89816 100755 >--- a/tools/scheduler.pl >+++ b/tools/scheduler.pl >@@ -27,6 +27,8 @@ use C4::Output qw( output_html_with_http_headers ); > use Koha::DateUtils qw( dt_from_string ); > use Koha::Reports; > use Koha::Email; >+use Koha::Patrons; >+use Koha::Report; > > my $input = CGI->new; > my $base; >@@ -48,8 +50,20 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( > } > ); > >-my $op = $input->param('op') // q{}; >-my $id = $input->param('id'); >+my $op = $input->param('op') // q{}; >+my $id = $input->param('id'); >+my $selected_report_id = $input->param('report'); >+my $selected_report = $selected_report_id ? Koha::Reports->find($selected_report_id) : undef; >+if ( $op eq 'cud-add' && $selected_report_id ) { >+ my $logged_in_user = Koha::Patrons->find($borrowernumber); >+ if ( !$selected_report ) { >+ $template->param( no_sql_for_id => $selected_report_id, access_denied => 1 ); >+ $op = ''; >+ } elsif ( !$selected_report->can_access($logged_in_user) ) { >+ $template->param( access_denied => 1, id => $selected_report_id ); >+ $op = ''; >+ } >+} > > if ( $op eq 'cud-add' ) { > >@@ -111,14 +125,31 @@ foreach my $job ( values %$jobs ) { > > @jobloop = sort { $a->{TIME} cmp $b->{TIME} } @jobloop; > >-my $reports = get_saved_reports(); >+my $logged_in_user = Koha::Patrons->find($borrowernumber); >+my $can_manage_limits = Koha::Report->can_manage_limits($logged_in_user); >+ >+my $reports = >+ $can_manage_limits >+ ? get_saved_reports( { show_all_reports => 1 } ) >+ : get_saved_reports(); >+ >+my @final_reports; >+if ($can_manage_limits) { >+ @final_reports = @$reports; >+} else { >+ @final_reports = grep { >+ my $r_obj = Koha::Reports->find( $_->{id} ); >+ $r_obj && $r_obj->can_access($logged_in_user); >+ } @$reports; >+} >+ > if ( defined $id ) { >- foreach my $report (@$reports) { >+ foreach my $report (@final_reports) { > $report->{'selected'} = 1 if $report->{'id'} eq $id; > } > } > >-$template->param( 'savedreports' => $reports ); >+$template->param( 'savedreports' => \@final_reports ); > $template->param( JOBS => \@jobloop ); > my $time = localtime(time); > $template->param( 'time' => $time ); >-- >2.15.0 >
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 16631
:
184422
|
184508
|
184509
|
184510
|
184511
|
184512
|
184513
|
184514
|
184515
|
184516
|
184517
|
184518
|
184519
|
184520
|
184521
|
184522
|
184525
|
184526
|
184657
|
184674
|
184686
|
184687
|
184688
|
184689
|
184690
|
184691
|
184692
|
184693
|
184694
|
184695
|
184845
|
184846
|
184847
|
184848
|
184859
|
184861
|
185627
|
185628
|
185779
|
185780
|
185781
|
185782
|
185788
|
185789
|
185790
|
186299
|
186300
|
186335
|
186393
|
186632
|
186633
|
188444
| 188445