diff --git a/C4/Reports/Guided.pm b/C4/Reports/Guided.pm index 865d2ce370..1c4afa1e40 100644 --- a/C4/Reports/Guided.pm +++ b/C4/Reports/Guided.pm @@ -881,7 +881,54 @@ sub get_saved_reports { my $result = $dbh->selectall_arrayref( $query, { Slice => {} }, @args ); - return $result; + my $limit_pref_enabled = C4::Context->preference('LimitReportsByLibrary'); + + 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 => $_->branchcode, branchname => $_->branchname } } $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 => $_->branchcode, branchname => $_->branchname } } $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 f9bb5772ab..c6f5373d8c 100644 --- a/C4/UsageStats.pm +++ b/C4/UsageStats.pm @@ -321,6 +321,7 @@ sub _shared_preferences { autoMemberNum BorrowerRenewalPeriodBase EnableBorrowerFiles + LimitReportsByLibrary EnhancedMessagingPreferences ExtendedPatronAttributes intranetreadinghistory diff --git a/Koha/Manual.pm b/Koha/Manual.pm index a5b16bfe5b..ffb08f3080 100644 --- a/Koha/Manual.pm +++ b/Koha/Manual.pm @@ -155,6 +155,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 fdad5cff92..aac9c16a3c 100644 --- a/Koha/Report.pm +++ b/Koha/Report.pm @@ -21,9 +21,15 @@ use Koha::Database; use Koha::Exceptions::Report; 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 @@ -347,4 +353,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('LimitReportsByLibrary') + && ( $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('LimitReportsByLibrary'); + + 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 5cd2c1836c..6e2f8f3974 100644 --- a/Koha/Reports.pm +++ b/Koha/Reports.pm @@ -22,7 +22,7 @@ use Koha::Logger; use Koha::Report; -use base qw(Koha::Objects); +use base qw(Koha::Objects Koha::Objects::Limit::Library); =head1 NAME diff --git a/admin/columns_settings.yml b/admin/columns_settings.yml index 28c524a68c..8c5edc210c 100644 --- a/admin/columns_settings.yml +++ b/admin/columns_settings.yml @@ -1688,6 +1688,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..ed48ab4d0f --- /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 ('LimitReportsByLibrary', '0', NULL, 'Enable ability for staff to limit report access based on home library.', 'YesNo')} + ); + + say $out "Added new system preference 'LimitReportsByLibrary'"; + + $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 ec250ffad0..3777004f0d 100644 --- a/installer/data/mysql/kohastructure.sql +++ b/installer/data/mysql/kohastructure.sql @@ -5957,6 +5957,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/userpermissions.sql b/installer/data/mysql/mandatory/userpermissions.sql index f7b8c492f1..9af61fa43e 100644 --- a/installer/data/mysql/mandatory/userpermissions.sql +++ b/installer/data/mysql/mandatory/userpermissions.sql @@ -150,6 +150,7 @@ INSERT INTO permissions (module_bit, code, description) VALUES (16, 'create_reports', 'Create SQL reports'), (16, 'delete_own_reports', 'Delete SQL reports you have created'), (16, 'delete_all_reports', 'Delete SQL reports created by anyone'), + (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 ef9a7a383c..18eff0b712 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc +++ b/koha-tmpl/intranet-tmpl/prog/en/includes/permissions.inc @@ -520,6 +520,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..2b17d1f079 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..701df654ee --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref @@ -0,0 +1,8 @@ +Reports: + Reports Access: + - + - pref: LimitReportsByLibrary + choices: + 1: Enable + 0: Disable + - "limiting report access based on staff home library." 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 0230e02374..8e67104b57 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 %] @@ -229,6 +230,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.

    @@ -264,7 +274,7 @@

    Saved reports

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

    Filtered by: @@ -277,6 +287,9 @@ [% IF ( filters.keyword ) %] Keyword: [% filters.keyword | html %] [% END %] + [% IF library_limit_active %] + Show reports: My library reports + [% END %] Clear

    @@ -337,6 +350,9 @@ [% ELSE %] Update [% END %] + [% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] + Library limits + [% END %] Actions @@ -404,6 +420,29 @@ > [% END %] + [% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] + + [% IF savedreport.library_limits && savedreport.library_limits.size > 0 %] + [% library_str = "" %] + [% FOREACH library IN savedreport.library_limits %] + [%- IF loop.first -%] + [% library_str = library.branchname _ " (" _ library.branchcode _ ")" %] + [% ELSE %] + [% library_str = library_str _ "\n" _ library.branchname _ " (" _ library.branchcode _ ")" %] + [% END %] + [% END %] + + [% IF savedreport.library_limits.size > 1 %] + [% savedreport.library_limits.size | html %] library limitations + [% ELSE %] + [% savedreport.library_limits.size | html %] library limitation + [% END %] + + [% ELSE %] + No limitation + [% END %] + + [% END %]
    [%# There should be no space between these two buttons, it would render badly %] @@ -502,7 +541,7 @@ New SQL report - Cancel filter + Cancel filter [% END %] [% ELSE %]

    There are no saved reports.

    @@ -898,6 +937,7 @@ + @@ -1496,6 +1536,19 @@ + [% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] +
    + Library limitation: +
    + + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %] +
    SQL:
    @@ -1616,6 +1669,19 @@
    + [% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] +
    + Library limitation: +
    + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %] +
    SQL: [% PROCESS insert_runtime_parameter %] @@ -1698,6 +1764,26 @@
    • +
  • + [% IF Koha.Preference('LimitReportsByLibrary') %] + [% IF ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) %] + + + [% END %] + [% END %] +
    • @@ -2279,7 +2365,22 @@ let report_table_settings = { columns }; $("#report_results").kohaTable({}, report_table_settings); } - if (saved1) { + [% IF (saved1) %] + var table_settings = [% TablesSettings.GetTableSettings( 'reports', 'saved-sql', 'table_reports', 'json' ) | $raw %]; + [% UNLESS ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] + table_settings.columns = table_settings.columns.filter(function(col) { + return col.columnname !== 'library_limits'; + }); + [% END %] + + [% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %] + let library_limits_col_index = -1; + table_settings.columns.forEach(function(col, index) { + if (col.columnname === 'library_limits') { + library_limits_col_index = index; + } + }); + [% END %] var rtable = $("#table_reports").kohaTable( { autoWidth: false, @@ -2287,7 +2388,9 @@ order: [[1, "asc"]], language: { zeroRecords: _("No matching reports found"), - }, + }[% IF ( Koha.Preference('LimitReportsByLibrary') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_limits ) ) %], + columnDefs: library_limits_col_index !== -1 ? [{ visible: false, targets: [library_limits_col_index] }] : [] + [% END %] }, table_settings ); @@ -2346,7 +2449,7 @@ $("#update_sql_label").html(""); $("#update_sql .modal-body").html('
    ' + _("Loading") + "
    "); }); - } + [% END %] if (op == "showsql") { $("#sql").focus(function () { @@ -2578,6 +2681,12 @@ selectField.style.minWidth = "320px"; $(selectField).select2(); }); + + const libraryLimitSelectFields = document.querySelectorAll('#library_limitation'); + libraryLimitSelectFields.forEach((selectField) => { + selectField.style.minWidth = '450px'; + $(selectField).select2(); + }); }); $("#toggle_auto_links").on("click", function (e) { diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index 2bef3d068c..8e0c8f56a1 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -101,22 +101,49 @@ my ( $template, $borrowernumber, $cookie ) = get_template_and_user( flagsrequired => { reports => $flagsrequired }, } ); -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 $session_id = $input->cookie('CGISESSID'); +my $session = $session_id ? get_session($session_id) : undef; +my $logged_in_user = Koha::Patrons->find($borrowernumber); +my $limit_reports_by_library = C4::Context->preference('LimitReportsByLibrary'); +my $can_manage_report_limits = $logged_in_user && Koha::Report->can_manage_limits($logged_in_user); 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) { + unless ( $report->can_access($logged_in_user) ) { + $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 ); @@ -146,151 +173,164 @@ 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, - 'report_creator' => $report->borrowernumber, - '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, + 'report_creator' => $report->borrowernumber, + '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( - 'report_creator' => $report->borrowernumber, - '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( + 'report_creator' => $report->borrowernumber, + '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 ( $limit_reports_by_library && !$can_manage_report_limits ) { + my $existing_limits = $report->get_library_limits; + @branches = $existing_limits ? map { $_->branchcode } $existing_limits->as_list : (); } - } - # 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 }; - } + # 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 + } + } - create_non_existing_group_and_subgroup( $input, $group, $subgroup ); + # 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 }; + } - my ( $is_sql_valid, $validation_errors ) = Koha::Report->new( { savedsql => $sql } )->is_sql_valid; - push( @errors, @$validation_errors ) unless $is_sql_valid; + create_non_existing_group_and_subgroup( $input, $group, $subgroup ); - if (@errors) { - $template->param( - 'errors' => \@errors, - 'sql' => $sql, - ); - } else { + my ( $is_sql_valid, $validation_errors ) = Koha::Report->new( { savedsql => $sql } )->is_sql_valid; + push( @errors, @$validation_errors ) unless $is_sql_valid; - # Check defined SQL parameters for authorised value validity - my $problematic_authvals = ValidateSQLParameters($sql); - - 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 + if (@errors) { $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, + 'errors' => \@errors, + 'sql' => $sql, ); - } 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, + # Check defined SQL parameters for authorised value validity + my $problematic_authvals = ValidateSQLParameters($sql); + + 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, + ); + + } 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, + } + ); + + $report->store; + $report->replace_library_limits( \@branches ); + + my $editsql = 1; + if ( $op eq 'cud-update_and_run_sql' ) { + $editsql = 0; } - ); - my $editsql = 1; - if ( $op eq 'cud-update_and_run_sql' ) { - $editsql = 0; + $template->param( + 'save_successful' => 1, + 'reportname' => $reportname, + 'id' => $id, + 'report_creator' => $report->borrowernumber, + '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' ) { + $op = 'run'; } - - my $report = Koha::Reports->find($id); - $template->param( - 'save_successful' => 1, - 'reportname' => $reportname, - 'id' => $id, - 'report_creator' => $report->borrowernumber, - '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'; } } @@ -548,6 +588,11 @@ 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 ( $limit_reports_by_library && !$can_manage_report_limits ) { + @branches = ( $logged_in_user->branchcode ); + } # 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) { @@ -625,6 +670,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"); my $new_report = Koha::Reports->find($id); $template->param( @@ -639,7 +687,8 @@ if ( !$op ) { 'cache_expiry' => $cache_expiry, 'public' => $public, 'usecache' => $usecache, - 'tables' => $tables + 'tables' => $tables, + 'report' => $report, ); } } @@ -657,148 +706,162 @@ 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(); + 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(); - eval { ( $sql, undef ) = $report->prep_report( \@param_names, \@sql_params, { export => 1 } ); }; - if ( my $e = $@ ) { - if ( ref($e) eq 'Koha::Exceptions::Report::DuplicateRunning' ) { - print $input->header( - -status => '429 Too Many Requests', - -type => 'text/plain; charset=UTF-8', - ); - print "This report is already running. Please wait for it to finish before running it again.\n"; - exit; - } - die $e; - } - 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"; + eval { ( $sql, undef ) = $report->prep_report( \@param_names, \@sql_params, { export => 1 } ); }; + if ( my $e = $@ ) { + if ( ref($e) eq 'Koha::Exceptions::Report::DuplicateRunning' ) { + print $input->header( + -status => '429 Too Many Requests', + -type => 'text/plain; charset=UTF-8', + ); + print "This report is already running. Please wait for it to finish before running it again.\n"; + exit; } - } else { - if ( $format eq 'csv' ) { - my $delimiter = C4::Context->csv_delimiter; - $type = 'application/csv'; + die $e; + } + my ( $sth, $q_errors ) = execute_query( { sql => $sql, report_id => $report_id } ); - # Add BOM for UTF-8 encoded CSV - $content .= "\xEF\xBB\xBF"; - - 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 => 'HEADER ROW: ' . $csv->error_diag() }; - } + 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() ) { - if ( $csv->combine(@$row) ) { - $content .= $scrubber->scrub( $csv->string() ) . "\n"; + $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"; + + 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() }; + } } - push @$ods_content, \@content_row; + } 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; + } + + # 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, + } + ); } + } - # Process - generate_ods( $ods_filepath, $ods_content ); - - # 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_code = $input->param('template_code'); - my $data = $sth->fetchall_arrayref( {} ); - - my $rendered = GetPreparedLetter( - module => 'report', - letter_code => $template_code, - objects => { data => $data, report_id => $report_id, for_download => 1 } - ); - $content = $rendered->{content}; - $reportfilename = $rendered->{title}; + 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(); @@ -813,6 +876,7 @@ if ( !$op ) { 'cache_expiry' => 300, 'usecache' => $usecache, 'tables' => $tables, + 'report' => $report, ); } @@ -822,7 +886,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_code = $input->param('template_code'); @@ -840,7 +903,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; @@ -1115,8 +1178,6 @@ if ( $op eq 'run' ) { 'param_names' => \@param_names, ); } - } else { - push @errors, { no_sql_for_id => $report_id }; } } @@ -1145,11 +1206,25 @@ 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}; $filter->{group} = $group; $filter->{subgroup} = $subgroup; + + if ($show_all_filter) { + $filter->{show_all_reports} = $can_manage_report_limits ? $show_all_filter : 0; + } else { + $filter->{show_all_reports} = 0; + } + my $reports = get_saved_reports($filter); + + my $library_limit_active = + $limit_reports_by_library + && $can_manage_report_limits + && !$filter->{show_all_reports}; + my $has_obsolete_reports; for my $report (@$reports) { $report->{results} = C4::Reports::Guided::get_results( $report->{id} ); @@ -1159,13 +1234,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, + library_limit_active => $library_limit_active, + can_manage_report_limits => $can_manage_report_limits, ); } @@ -1276,4 +1353,3 @@ sub create_non_existing_group_and_subgroup { } } } - diff --git a/t/Koha/Auth/Permissions.t b/t/Koha/Auth/Permissions.t index 064fcbc9dd..2551fcf8f2 100755 --- a/t/Koha/Auth/Permissions.t +++ b/t/Koha/Auth/Permissions.t @@ -64,6 +64,7 @@ subtest 'normal staff user test' => sub { 'CAN_user_reports_delete_own_reports' => 1, 'CAN_user_reports_delete_all_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' ); }; @@ -237,6 +238,7 @@ subtest 'superlibrarian tests' => sub { 'CAN_user_reports_delete_own_reports' => 1, 'CAN_user_reports_delete_all_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 0fa30592cc..adafb604cc 100755 --- a/t/db_dependent/Koha/Reports.t +++ b/t/db_dependent/Koha/Reports.t @@ -287,6 +287,160 @@ subtest 'apply_execution_time_limit' => sub { Koha::Report->apply_execution_time_limit($sql), qr!^(?i:select) /\*\+ MAX_EXECUTION_TIME\(1500\) \*/ \* FROM biblio$!, 'MySQL max execution time hint applied' +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( 'LimitReportsByLibrary', 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( 'LimitReportsByLibrary', 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' ); +}; + +subtest 'library limits enforcement on save and update' => sub { + plan tests => 5; + + my $libraryA = $builder->build_object( { class => 'Koha::Libraries' } ); + my $branchA = $libraryA->branchcode; + + my $basic_patron = $builder->build_object( { class => 'Koha::Patrons', value => { branchcode => $branchA } } ); + + # Ensure manage_report_limits permission exists and is NOT granted to basic_patron + 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' } ); + + # Create report path + + # pref ON, no permission: creator branch auto-assigned + t::lib::Mocks::mock_preference( 'LimitReportsByLibrary', 1 ); + my $report_create = Koha::Report->new( { report_name => 'limit test create', savedsql => 'SELECT 1' } )->store; + my $can_manage = Koha::Report->can_manage_limits($basic_patron); + my @branches = $can_manage ? () : ( $basic_patron->branchcode ); + $report_create->replace_library_limits( \@branches ); + my @limits = $report_create->get_library_limits->as_list; + is( scalar @limits, 1, 'pref ON, no perm: create assigns exactly one branch limit' ); + is( $limits[0]->branchcode, $branchA, 'pref ON, no perm: create limit is the creator branchcode' ); + + # pref OFF: no branches from form, no limits stored + t::lib::Mocks::mock_preference( 'LimitReportsByLibrary', 0 ); + my $report_off = Koha::Report->new( { report_name => 'limit test pref off', savedsql => 'SELECT 1' } )->store; + $report_off->replace_library_limits( [] ); + is( $report_off->get_library_limits, undef, 'pref OFF: no limits stored on create' ); + + # Update report path + t::lib::Mocks::mock_preference( 'LimitReportsByLibrary', 1 ); + + # pref ON, no permission, report has existing limits: limits preserved unchanged + my $report_update = Koha::Report->new( { report_name => 'limit test update', savedsql => 'SELECT 1' } )->store; + $report_update->replace_library_limits( [$branchA] ); + my $existing_limits = $report_update->get_library_limits; + my @preserved = $existing_limits ? map { $_->branchcode } $existing_limits->as_list : (); + $report_update->replace_library_limits( \@preserved ); + my @updated_limits = $report_update->get_library_limits->as_list; + is( scalar @updated_limits, 1, 'pref ON, no perm: update preserves existing limits unchanged' ); + + # pref ON, no permission, report has no existing limits: stays limit-free + my $report_no_limits = + Koha::Report->new( { report_name => 'limit test no limits', savedsql => 'SELECT 1' } )->store; + my $existing_limits2 = $report_no_limits->get_library_limits; + my @branches2 = $existing_limits2 ? map { $_->branchcode } $existing_limits2->as_list : (); + $report_no_limits->replace_library_limits( \@branches2 ); + is( + $report_no_limits->get_library_limits, undef, + 'pref ON, no perm: update with no existing limits stays limit-free' ); }; diff --git a/t/db_dependent/Reports/Guided.t b/t/db_dependent/Reports/Guided.t index aa196b5631..60ce55cd9d 100755 --- a/t/db_dependent/Reports/Guided.t +++ b/t/db_dependent/Reports/Guided.t @@ -22,7 +22,7 @@ use Modern::Perl; use Test::NoWarnings; use Test::MockModule qw(strict); -use Test::More tests => 14; +use Test::More tests => 15; use Test::Warn; use t::lib::TestBuilder; @@ -333,6 +333,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( 'LimitReportsByLibrary', 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( 'LimitReportsByLibrary', 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 97483d003d..9147056e7e 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 );