From f39b3bcf70e39557f4c7ba2210599e558e08a7d3 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/UsageStats.pm | 1 + Koha/Manual.pm | 1 + Koha/Report.pm | 20 ++++- Koha/Reports.pm | 27 +++++- installer/data/mysql/atomicupdate/bug_16631.pl | 31 +++++++ 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 | 38 ++++++++- reports/guided_reports.pl | 97 ++++++++++++++++------ t/Koha/Auth/Permissions.t | 2 + t/db_dependent/Koha/Reports.t | 38 ++++++++- 15 files changed, 263 insertions(+), 29 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/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..8c292a16a8 100644 --- a/Koha/Report.pm +++ b/Koha/Report.pm @@ -20,9 +20,13 @@ use Modern::Perl; use Koha::Database; use Koha::Reports; +use Koha::Object; +use Koha::Object::Limit::Library; + +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 +272,18 @@ sub _type { return 'SavedSql'; } +=head3 _library_limits + +Configurable library limits + +=cut + +sub _library_limits { + return { + class => "ReportsBranch", + id => "report_id", + library => "branchcode", + }; +} + 1; diff --git a/Koha/Reports.pm b/Koha/Reports.pm index 7d99232b9b..2803c8c02c 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,6 +33,31 @@ Koha::Reports - Koha Report Object set class =cut +=head3 search_with_localization + +=cut + +sub search_with_localization { + my ( $self, $params, $attributes ) = @_; + + my $language = C4::Languages::getlanguage(); + $Koha::Schema::Result::Itemtype::LANGUAGE = $language; + $attributes->{order_by} = 'translated_description' unless exists $attributes->{order_by}; + $attributes->{join} = 'localization'; + $attributes->{'+select'} = [ + { + coalesce => [qw( localization.translation me.description )], + -as => 'translated_description' + } + ]; + if ( defined $params->{branchcode} ) { + my $branchcode = delete $params->{branchcode}; + $self->search_with_library_limits( $params, $attributes, $branchcode ); + } else { + $self->SUPER::search( $params, $attributes ); + } +} + =head3 _type Returns name of corresponding DBIC resultset diff --git a/installer/data/mysql/atomicupdate/bug_16631.pl b/installer/data/mysql/atomicupdate/bug_16631.pl new file mode 100755 index 0000000000..c2656e161b --- /dev/null +++ b/installer/data/mysql/atomicupdate/bug_16631.pl @@ -0,0 +1,31 @@ +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 `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 ('EnableFilteringReports', '0', NULL, 'Enable ability for staff to filter report access based on home library.', 'YesNo')} + ); + + say $out "Added new system preference 'EnableFilteringReports'"; + }, +}; 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..fb4d560b1a 100644 --- a/installer/data/mysql/mandatory/sysprefs.sql +++ b/installer/data/mysql/mandatory/sysprefs.sql @@ -253,6 +253,7 @@ INSERT INTO systempreferences ( `variable`, `value`, `options`, `explanation`, ` ('EnableAdvancedCatalogingEditor','0','','Enable the Rancor advanced cataloging editor','YesNo'), ('EnableBorrowerFiles','0',NULL,'If enabled, allows librarians to upload and attach arbitrary files to a borrower record.','YesNo'), ('EnableExpiredPasswordReset', '0', NULL, 'Enable ability for patrons with expired password to reset their password directly', 'YesNo'), +('EnableFilteringReports', '0', NULL, 'Enable ability for staff to filter report access based on home library.', 'YesNo'), ('EnableItemGroupHolds','0','','Enable item groups holds feature','YesNo'), ('EnableItemGroups','0','','Enable the item groups feature','YesNo'), ('EnableOpacSearchHistory','1','YesNo','Enable or disable opac search history',''), diff --git a/installer/data/mysql/mandatory/userpermissions.sql b/installer/data/mysql/mandatory/userpermissions.sql index 1ff3f33d1e..33aac5b308 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_filters', 'Manage report filters'), (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..dbbb8430df 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_filters' -%] + Manage Report Filters + ([% 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..5d5a60c6c1 --- /dev/null +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/reports.pref @@ -0,0 +1,8 @@ +Reports: + Reports Access: + - + - pref: EnableFilteringReports + choices: + 1: Enable + 0: Disable + - "filtering 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..37940a3032 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 %] @@ -234,6 +235,16 @@ [% IF ( saved1 ) %]

    Saved reports

    + [% IF ( CAN_user_superlibrarian || CAN_user_reports_manage_report_filters ) %] +
    + + + +
    + [% END %] [% IF ( savedreports ) %] [% IF ( filters.date || filters.author || filters.keyword ) %] @@ -850,6 +861,7 @@ + @@ -1434,7 +1446,18 @@ Required - + [% IF ( Koha.Preference('EnableFilteringReports') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_filters ) ) %] +
    + Library limitation: +
    + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %]
    @@ -1548,7 +1571,18 @@
    Required
    - + [% IF ( Koha.Preference('EnableFilteringReports') && ( CAN_user_superlibrarian || CAN_user_reports_manage_report_filters ) ) %] +
    + Library limitation: +
    + +
    Limits the use of this report to the selected libraries.
    +
    +
    + [% END %]
    diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl index fe47501ee7..8e52f5c9f3 100755 --- a/reports/guided_reports.pl +++ b/reports/guided_reports.pl @@ -95,6 +95,9 @@ if ( $input->param("filter_set") or $input->param('clear_filters') ) { $filter = $session->param('report_filter'); } +my $show_all = $input->param('show_all'); +$template->param( show_all => $show_all ); + my @errors = (); if ( !$op ) { $template->param( 'start' => 1 ); @@ -127,7 +130,7 @@ if ( !$op ) { 'showsql' => 1, 'mana_success' => scalar $input->param('mana_success'), 'mana_id' => $report->{mana_id}, - 'mana_comments' => $report->{comments} + 'mana_comments' => $report->{comments}, ); } elsif ( $op eq 'edit_form' ) { @@ -148,11 +151,13 @@ if ( !$op ) { 'editsql' => 1, 'mana_id' => $report->{mana_id}, 'mana_comments' => $report->{comments}, - 'tables' => $tables + 'tables' => $tables, + 'report' => $report, ); } elsif ( $op eq 'cud-update_sql' || $op eq 'cud-update_and_run_sql' ) { my $id = $input->param('id'); + my $report = Koha::Reports->find($id); my $sql = $input->param('sql'); my $reportname = $input->param('reportname'); my $group = $input->param('group'); @@ -163,7 +168,8 @@ if ( !$op ) { my $public = $input->param('public'); my $save_anyway = $input->param('save_anyway'); my @errors; - my $tables = get_tables(); + 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) { @@ -211,6 +217,8 @@ if ( !$op ) { 'problematic_authvals' => $problematic_authvals, 'warn_authval_problem' => 1, 'phase_update' => 1, + 'branches' => @branches, + 'report' => $report, ); } else { @@ -225,10 +233,13 @@ if ( !$op ) { subgroup => $subgroup, notes => $notes, public => $public, - cache_expiry => $cache_expiry, + cache_expiry => $cache_expiry } ); + $report->store; + $report->replace_library_limits( \@branches ); + my $editsql = 1; if ( $op eq 'cud-update_and_run_sql' ) { $editsql = 0; @@ -245,7 +256,8 @@ if ( !$op ) { 'cache_expiry' => $cache_expiry, 'public' => $public, 'usecache' => $usecache, - 'tables' => $tables + 'tables' => $tables, + 'report' => $report ); logaction( "REPORTS", "MODIFY", $id, "$reportname | $sql" ) if C4::Context->preference("ReportsLog"); } @@ -514,6 +526,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 +604,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 +619,8 @@ if ( !$op ) { 'cache_expiry' => $cache_expiry, 'public' => $public, 'usecache' => $usecache, - 'tables' => $tables + 'tables' => $tables, + 'report' => $report, ); } } @@ -746,20 +763,24 @@ if ( !$op ) { } 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 +795,7 @@ if ( !$op ) { 'cache_expiry' => 300, 'usecache' => $usecache, 'tables' => $tables, + 'report' => $report, ); } @@ -1081,24 +1103,51 @@ if ( $op eq 'list' || $op eq 'convert' ) { my $subgroup = $input->param('subgroup'); $filter->{group} = $group; $filter->{subgroup} = $subgroup; - my $reports = get_saved_reports($filter); - my $has_obsolete_reports; - for my $report (@$reports) { - $report->{results} = C4::Reports::Guided::get_results( $report->{id} ); - if ( $report->{savedsql} =~ m|biblioitems| and $report->{savedsql} =~ m|marcxml| ) { - $report->{seems_obsolete} = 1; - $has_obsolete_reports++; + + my $pref_enable_filtering_reports = C4::Context->preference("EnableFilteringReports"); + if ( !$show_all && $pref_enable_filtering_reports == "1" ) { + my $reports_with_library_limits_results = + Koha::Reports->search_with_library_limits( {}, {}, C4::Context::mybranch() ); + my $reports_list = $reports_with_library_limits_results->unblessed; + my $has_obsolete_reports; + while ( my $report = $reports_with_library_limits_results->next ) { + $report->{results} = C4::Reports::Guided::get_results( $report->{id} ); + if ( $report->{savedsql} =~ m|biblioitems| and $report->{savedsql} =~ m|marcxml| ) { + $report->{seems_obsolete} = 1; + $has_obsolete_reports++; + } + $template->param( + 'manamsg' => $input->param('manamsg') || '', + 'saved1' => 1, + 'savedreports' => $reports_list, + 'usecache' => $usecache, + 'groups_with_subgroups' => groups_with_subgroups( $group, $subgroup ), + filters => $filter, + has_obsolete_reports => $has_obsolete_reports, + ); + } + } else { + + my $reports = get_saved_reports($filter); + my $has_obsolete_reports; + + for my $report (@$reports) { + $report->{results} = C4::Reports::Guided::get_results( $report->{id} ); + if ( $report->{savedsql} =~ m|biblioitems| and $report->{savedsql} =~ m|marcxml| ) { + $report->{seems_obsolete} = 1; + $has_obsolete_reports++; + } } + $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, + ); } - $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, - ); } # pass $sth, get back an array of names for the column headers diff --git a/t/Koha/Auth/Permissions.t b/t/Koha/Auth/Permissions.t index 0fc4ba108a..c2a36774cd 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_filters' => 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_filters' => 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..9778357d87 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 => 10; use Koha::Report; use Koha::Reports; @@ -185,4 +185,40 @@ 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' + ); +}; + $schema->storage->txn_rollback; -- 2.15.0