From dcda4c2af0adcf9b7c15bf6cb6feb29508b67a68 Mon Sep 17 00:00:00 2001 From: "Cameron E. Tidd" 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' -%] Execute SQL reports ([% name | html %]) + [%- CASE 'manage_report_limits' -%] + Manage Report Limits + ([% name | html %]) [%- CASE 'add_reserves' -%] Add course reserves ([% name | html %]) 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 @@ [% END %] + [% IF ( reports ) %] +
  • + Reports + [% PROCESS subtabs %] +
  • + [% ELSE %] +
  • + Reports +
  • + [% END %] + [% IF ( searching ) %]
  • Searching 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 %] +
    +

    Access denied

    +

    This report is restricted and cannot be accessed by the current user.

    + [% IF id %]

    Report ID: [% id | html %]

    [% END %] +

    Return to saved reports

    +
    + [% END %] + [% IF ( start ) %]

    Guided reports

    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.

    @@ -236,7 +246,7 @@

    Saved reports

    [% IF ( savedreports ) %] - [% IF ( filters.date || filters.author || filters.keyword ) %] + [% IF ( filters.date || filters.author || filters.keyword || branch_limit_active ) %]

    Filtered by: @@ -249,6 +259,9 @@ [% IF ( filters.keyword ) %] Keyword: [% filters.keyword | html %] [% END %] + [% IF branch_limit_active %] + Show reports: My branch reports + [% END %] Clear

    @@ -309,6 +322,9 @@ [% ELSE %] Update [% END %] + [% IF ( can_manage_report_limits ) %] + Library limits + [% END %] Actions @@ -376,6 +392,13 @@ > [% END %] + [% IF ( can_manage_report_limits ) %] + + [% IF savedreport.library_limits %] + [% savedreport.library_limits.join(', ') %] + [% END %] + + [% END %]
    [%# There should be no space between these two buttons, it would render badly %] @@ -455,7 +478,7 @@ New SQL report - Cancel filter + Cancel filter [% END %] [% ELSE %]

    There are no saved reports.

    @@ -850,6 +873,7 @@ + @@ -1434,7 +1458,18 @@ Required
    - + [% IF ( Koha.Preference('LimitReportsByBranch') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] +
    + Library limitation: +
    + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %]
    @@ -1548,7 +1583,18 @@
    Required
    - + [% IF ( Koha.Preference('LimitReportsByBranch') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] +
    + Library limitation: +
    + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %]
    @@ -1616,6 +1662,26 @@
  • +
  • + [% IF Koha.Preference('LimitReportsByBranch') %] + [% IF ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) %] + + + [% END %] + [% END %] +
  • 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 <>', + } + )->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