Test plan:
1. Navigate to Reports > Create from SQL.
2. Create a new report with a name and the following SQL query:
SELECT 'test', 1, '2'
3. Run the report.
4. Under the Download dropdown, select "Open Document Spreadsheet".
5. Verify a ".ods" file is downloaded.
6. Open the downloaded spreadsheet.
7. Verify the two number-like columns are handled as such. This depends
on your choice of spreadsheet software:
- In Excel, the numbers should not have a leading single quote in the
edit-bar when selected.
- In Google Sheets, the numbers should be right-aligned, and
selecting both of them should display their sum in the bottom
toolbar.
8. Run `prove t/Koha/Util/OpenDocument.t` and verify all tests were
successful.
Sponsored-by: Open Fifth
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Before this patch, control fields were not modifiable using this subroutine.
Control fields use character positions instead of subfields. When modifying
control fields, this patch makes the subroutine's parameter "subfield" to indicate
a character position. An example
Koha::SimpleMARC::update_field({
record => $record,
field => '008',
subfield => '2',
values => [09],
});
changes field 008 starting from character position 2 to contain values "09".
This replaces char pos 2 with "0", and char pos 3 with "9".
To test:
1. prove t/SimpleMARC.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Anneli Österman <anneli.osterman@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Add explicit `use JSON::PP ()` to Elasticsearch.pm and the test file
to make the dependency on JSON::PP::false visible rather than relying
on YAML::XS loading it as a side effect.
Extend the get_elasticsearch_mappings() test subtest to assert that
date_detection and numeric_detection are disabled at the mapping root.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When ElasticsearchMARCFormat is set to ARRAY, the marc_data_array
field uses dynamic:true mapping. Elasticsearch auto-detects field
types based on the first document indexed. If a MARC subfield value
looks like a date (e.g. 2024-01-15 in 952$e), ES locks that
subfield to type date. Subsequent records with non-date values in
the same subfield fail to index with a document_parsing_exception.
This patch sets date_detection and numeric_detection to false at
the mapping root level, so ES treats all dynamically mapped fields
as text instead of guessing types.
Test plan:
1. $ ktd --search-engine es8 --name test-es --proxy up -d
$ ktd --name test-es --wait-ready 180
$ ktd --name test-es --shell
2. k$ koha-mysql kohadev -e "UPDATE systempreferences
SET value='ARRAY'
WHERE variable='ElasticsearchMARCFormat';"
k$ echo 'flush_all' | nc -q1 memcached 11211
k$ koha-mysql kohadev -e "DELETE FROM biblio_metadata;
DELETE FROM items; DELETE FROM biblioitems;
DELETE FROM biblio;"
k$ rebuild_elasticsearch.pl -d -r --biblios
3. Add a record with date-like 952$e and index it:
k$ koha-shell kohadev -p -c 'perl -MC4::Biblio
-MMARC::Record -MMARC::Field -MKoha::Item -e "
my \$r = MARC::Record->new();
\$r->append_fields(
MARC::Field->new(q{245},q{1},q{0},a=>q{Record one}),
MARC::Field->new(q{942},q{ },q{ },c=>q{BK}));
my (\$bn) = C4::Biblio::AddBiblio(\$r, q{});
Koha::Item->new({biblionumber=>\$bn,
biblioitemnumber=>\$bn, homebranch=>q{CPL},
holdingbranch=>q{CPL}, itype=>q{BK},
booksellerid=>q{2024-01-15}})->store;
print qq{Added biblio \$bn\n};"'
k$ rebuild_elasticsearch.pl --biblios -bn <biblionumber>
4. Check the mapping for 952$e:
k$ curl -s es:9200/koha_kohadev_biblios/_mapping/field/\
marc_data_array.fields.952.subfields.e
=> FAIL: type is "date" (should be "text")
5. Add a record with non-date 952$e and index it:
k$ koha-shell kohadev -p -c 'perl -MC4::Biblio
-MMARC::Record -MMARC::Field -MKoha::Item -e "
my \$r = MARC::Record->new();
\$r->append_fields(
MARC::Field->new(q{245},q{1},q{0},a=>q{Record two}),
MARC::Field->new(q{942},q{ },q{ },c=>q{BK}));
my (\$bn) = C4::Biblio::AddBiblio(\$r, q{});
Koha::Item->new({biblionumber=>\$bn,
biblioitemnumber=>\$bn, homebranch=>q{CPL},
holdingbranch=>q{CPL}, itype=>q{BK},
booksellerid=>q{My Vendor Name}})->store;
print qq{Added biblio \$bn\n};"'
k$ rebuild_elasticsearch.pl --biblios -bn <biblionumber> -v -v -v
=> FAIL: document_parsing_exception - failed to parse date field
[My Vendor Name]
6. Apply this patch, repeat from step 2
7. Repeat step 4:
=> SUCCESS: type is "text"
8. Repeat step 5:
=> SUCCESS: No indexing errors
9. Sign off :-D
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This is a quite ugly solution, but that may answer end-user needs.
It add the option the group items by status on the bibliographic record
detail page.
However it will have VERY bad impact on performances for record with a
lot of items, as we are going to determine the status of each items of
the record.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Testing plan:
- Set EmailPurchaseSuggestions to BranchEmailAddress
- Go to create suggestion on the staff side /suggestion/suggestion.pl?op=add_form
- Create a suggestion with Library: Any
- 500 error: inbound_email_address is not a method of undef
- Apply patch & repeat
- Suggestion created
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The indexation step has been removed by
commit d2820f57d6
Bug 34788: (QA follow-up)
Koha/ERM/EHoldings/Title.pm
- C4::Biblio::ModBiblio( $record, $self->biblio_id, '' );
+ C4::Biblio::ModBiblio( $record, $self->biblio_id, '', { skip_record_index => 1 } );
From the commit message:
2) The background job now uses skip_record_index to avoid queuing indexing jobs for every new biblio and instead queues one job at the end
it's then missing when we add or edit a local title.
Test plan:
1 - Enable ERMModule
2 - Go to ERM
3 - eHoldings->Titles
4 - New title
5 - Populate the form
6 - Check the create bibliographic record box
7 - Save
=> Confirm that the biblio is now indexed
8. Edit the title, modify a value, tick the box, save
=> Confirm that the biblio is now indexed with the new value
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Rather than excluding the entire about.tt files from codespell scanning,
annotate only the specific lines containing the proper noun "Provence"
(SAN-Ouest Provence, a French library co-operative and Koha sponsor) with
an inline TT comment that codespell recognises as a per-line ignore.
This lets codespell continue to check the rest of each file for genuine
spelling mistakes, while still catching accidental "Provence" for
"Province" in any other template not carrying this annotation.
Replaces the file-level exceptions added to Koha::Devel::Files in
favour of the more surgical inline approach.
Test plan:
1. Apply patch
2. Verify "Provence" on the annotated lines produces no codespell output:
codespell -d --ignore-words .codespell-ignore \
koha-tmpl/intranet-tmpl/prog/en/modules/about.tt
=> no output
3. Verify the rest of about.tt is still checked by introducing a
deliberate typo elsewhere in the file and confirming codespell flags it
4. prove xt/author/codespell.t
=> All tests pass
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
The ci-results repo stores all failures from a single test run as one
flat JSON array, regardless of whether they came from the pl, tt, or js
filetype pass. On the next incremental run, get_files_to_test('pl') reads
that entire list -- including any .tt failures -- and calls
remove_exceptions with the 'pl' context. Since .tt files only appear in
the tt exception list, they survive the filter and get codespell'd in the
pl pass where nothing is configured to exclude them.
The fix filters stored failures by filetype before applying exceptions,
so each pass only re-checks failures that belong to its own type.
Test plan:
1. Apply all patches
2. Add a file to the tt codespell exceptions in Koha::Devel::Files that
contains a word codespell would flag
3. Introduce a deliberate failure in another file so the ci-results repo
records the excepted .tt file as a previous failure
4. On the next incremental run, confirm the excepted .tt file is not
re-tested in the pl or js pass
5. prove xt/author/codespell.t
=> Tests pass; no false positives from cross-filetype re-checks
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Correct genuine spelling errors identified by the codespell 2.4.x
full scan after the KTD container OS upgrade. Changes are in
comments, POD, strings, and template text across 55 files.
"SAN Ouest Provence" (the French library co-operative and Koha
sponsor) is preserved as a proper noun. Perl copyright lines
receive an inline # codespell:ignore annotation; about.tt files
(contributor attribution pages) are added to the Koha::Devel::Files
codespell exception list so that future uses of "Provence" as a
typo for "Province" are still caught in all other files.
Notable user-visible fixes:
- "submiting" -> "submitting" in authority and bibliographic merge screens
- "deleteable" -> "deletable" in patron exception messages
- "Acquistions" -> "Acquisitions" in reports dictionary
Test plan:
1. Apply patches
2. In the staff interface, go to Cataloguing > Merge records and
confirm the submit button reads "Submitting" (not "Submiting")
3. In the staff interface, go to Authorities > Merge and confirm
the same correction
4. Attempt to delete a patron with outstanding issues and confirm
the error message reads "deletable" (not "deleteable")
5. prove xt/author/codespell.t
=> All tests pass
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Three issues caused the EdifactModal_spec.ts test suite to fail:
1. insertSampleEdifactMessages inserted a vendor with listprice='USD' and
invoiceprice='USD', but those columns have FK constraints against the
currency table and USD is not pre-populated in KTD's default install.
Drop the currency columns from the INSERT so they default to NULL.
2. Koha::Edifact::File::vendor() and basket() called
Koha::Acquisition::Bookseller and Koha::Acquisition::Basket without
use-ing them. The edifactmsgs.pl table embeds both on every API call,
so a missing module caused a 500 and an empty table.
3. showMessage() already appended &format=json before delegating to
showMessageFromUrl(), which then appended it a second time. Remove the
redundant copy from showMessage().
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The existing 'days: 289 hours, 2 holidays' test asserted 8-1=7 but its own
description named 2 holidays. The expected value matched the buggy behaviour
(loop exiting before the holiday end date) rather than the correct result.
Update to 8-2=6 to reflect the fix.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In Koha::Calendar::days_between, the holiday-subtraction loop uses a comparison
that includes time. If the end_dt is a holiday and its time is earlier than
the start_dt time (common in renewals), the loop terminates before processing
the end_dt, leading to an extra fine day.
This patch truncates both dates to the beginning of the day after cloning
and setting the time zone, ensuring the loop correctly processes all
calendar days between the two dates.
Test Plan:
1. Set 'finesCalendar' syspref to 'noFinesWhenClosed'.
2. Set 'CalculateFinesOnReturn' syspref to 'Yes'.
3. Set 'finesMode' syspref to 'Production'.
4. Define a special holiday for today in the library calendar.
5. Identify an overdue item for a patron. Manually set its 'date_due' in the
database to several days ago with a time of 23:59:00.
6. Renew the item today at any time BEFORE 23:59:00 (e.g., 10:00 AM).
7. Observe that an extra day of fine is charged because today (the holiday)
was not subtracted.
8. Apply this patch.
9. Repeat the test and observe that the holiday is now correctly subtracted.
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
An exception is not implemented as progress_request is called in
different circulation contexts (hold is placed, checkout happens,
etc) and those actions should not be dependent on the outcome of
progress_request, i.e. if progress_request fails due to schema
validation failure, the circulation action should happen as before.
prove t/db_dependent/Koha/ILL/ISO18626/Request.t
prove t/db_dependent/api/v1/iso18626/request.t
cypress run --spec t/cypress/integration/ILL/SupplyingRequests.ts
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This set of patches adds a new is_fast_add column to biblio_framework.
The Biblio.pm subroutine can_be_edited now checks if the framework is_fast_add.
In Admin > MARC bibliographic frameworks admin users can add or edit
frameworks to be used as Fast add frameworks.
Users with only fast_cataloging permission will be able to add records
using those frameworks in the Fast Cataloging module.
Test plan:
Apply patches, restart_all and updatedatabase
1. Go to Admin > MARC Bibliographic frameworks
2. Edit/create a new framework and check the 'Use as Fast add Framework' box
3. Also edit a framework and uncheck the 'Use as Fast add Framework' box
4. Notice a 'Fast add' badge is displayed in the table
5. Create a staff user with only fast_cataloging permissions
6. Log in as that user and go to Cataloging > Fast Cataloging
7. Confirm you can add a Fast add record and items
8. Use the 'Settings' drop down to Change Fast add framework
9. Confirm you see the frameworks that you set as Fast add earlier
10. Confirm you can add the record and items
11. Log in as a superlibrarian and confirm that you can add records and items
using standard frameworks and frameworks set as fast add
12. Set some biblio frameworks as Fast Add frameworks
13. Login as a user with fast_cataloging permissions only
14. Create and edit different types of Fast Add records
15. Add items and edit items
16. Confirm that you can only edit bibs and items
for bibs where the framework is set as a Fast add framework
and the item is from your home library.
17. Log in as a superlibrarian
18. Confirm you can still create and edit records with the Default
and other frameworks
19. Change a record's framework from a Fast add to Default
20. Login as the user with only fast_cataloging confirm that you can
no longer edit that record or its items
Sponsored-by: CLAMS
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Miranda Nero <mnero@oslri.net>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Instead of waiting a real 1 second
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
This commit applies changes to V1.pm to be able to utilize the same XML methods.
We're also updating tests to reflect this new functionality.
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
In preparation to implement 'hold for supply ILL'
This DRYs the existing functionality, meaning the new similar functionality will only be added once in the Controller::Catalogue
IMPORTANT:
The following command must only return occurrences in
Koha::Controller::Catalogue before this is pushed to main:
git grep "\->cookie(\"holdfor\")"
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Convert JSON response to XML if header content_type: application/xml
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Lisette Scheer <lisette@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Building on bug 41918, it would be good to limit the number of reports
a given user can run at a time. This control is config-file based as
it is tied to the server resources provided by the administrator.
Adds a new koha-conf.xml option total_running_reports_per_user_limit
(0 = disabled). When the configured limit is reached, the user is told
"You have too many reports running at the moment. Please wait for some
of them to finish before running this report again."
Note on implementation: the original patch on bz (attachment 198034)
performed a second pre-flight Koha::Reports->running call in
reports/guided_reports.pl. The bug 41918 QA follow-ups already pushed
the duplicate-running guard into Koha::Report::prep_report so it would
fire across the staff UI, svc/report and opac/svc/report. This patch
extends that same model-layer architecture rather than reintroducing
controller-layer enforcement:
- New Koha::Exceptions::Report::TotalRunning subclass of
Koha::Exceptions::Report.
- prep_report now performs a second check after the duplicate one and
throws TotalRunning when the user has total_running_reports_per_user_limit
reports of any kind already in flight.
- All three callers (guided_reports.pl, svc/report, opac/svc/report)
catch the new exception alongside DuplicateRunning, mapping it to a
new template error key total_running_reports_limit_exceeded for the
staff UI and a JSON {error: total_running_reports_limit_exceeded}
with HTTP 429 for the JSON endpoints.
Test Plan:
1) Set total_running_reports_per_user_limit to 3 in your koha-conf.xml.
2) Create 3 long-running reports like:
SELECT COUNT(*) FROM items i1 JOIN items i2 JOIN items i3 JOIN items i4 JOIN items i5 JOIN items i6
3) Run each report in turn by opening a new tab, browsing to the
reports module, and choosing to run that report.
4) Note the first three continue to run, but a fourth attempt gives
the new "too many reports running" alert in the staff UI.
5) Repeat against /cgi-bin/koha/svc/report?id=N and confirm an HTTP
429 response with JSON {error: total_running_reports_limit_exceeded}.
6) Set total_running_reports_per_user_limit back to 0 and confirm
normal behaviour resumes.
7) prove t/db_dependent/Koha/Reports.t -> 13 tests pass, including a
new subtest for TotalRunning.
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Koha::Reports->running queries information_schema.processlist. Without
the PROCESS privilege MariaDB/MySQL already restrict the view to the
caller's own threads, so the previous code worked correctly on stock
deployments. However, if a site has granted broader privileges (eg. a
shared MariaDB server where the Koha DB user happens to also have
PROCESS, or a custom role) the query would happily count threads
belonging to other database users - another Koha instance, a reporting
tool, or any unrelated application sharing the server.
Add an explicit
user = SUBSTRING_INDEX(CURRENT_USER(), '@', 1)
clause so the query is always scoped to the connection's MySQL user,
regardless of what privileges that user has been granted. The
processlist.user column holds just the username with no host, so we
strip the '@host' off CURRENT_USER() before comparing.
The new clause has no bind parameters - it is a constant per
connection - so the existing parameterised-bind assertions in the
running subtest still hold. A small additional assertion verifies the
clause is present in the SQL text.
Test plan:
1) prove t/db_dependent/Koha/Reports.t (now 12 tests, all pass).
2) Optional: in a multi-tenant MariaDB, grant PROCESS to the Koha DB
user, start a long query as a different DB user with the marker
/* { saved_sql.id: 1 } { user_id: 1 } */ in its info, then call
Koha::Reports->running. Confirm the foreign thread is NOT counted.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
The previous patches enforced duplicate_running_reports_per_user_limit
only in reports/guided_reports.pl. Two other code paths run saved SQL
reports for an authenticated user and bypassed the guard entirely:
- svc/report (staff JSON endpoint, eg. used for chart widgets and any
caller hitting /cgi-bin/koha/svc/report?id=N directly)
- opac/svc/report (OPAC JSON endpoint)
A user who hit their staff-UI limit could still spawn additional copies
of the same long-running report via either svc endpoint, defeating the
guard.
Push the check into Koha::Report::prep_report itself, which is the one
function all three callers go through. When the limit is reached
prep_report now throws a new Koha::Exceptions::Report::DuplicateRunning
(carrying report_id, user_id and the configured limit). The guided
reports CGI catches it and re-renders the existing "report is already
running" alert; both svc/report endpoints catch it and return HTTP 429
Too Many Requests with a JSON { error: "duplicate_running_report" }
payload.
Notes:
- The pre-flight Koha::Reports->running call is removed from the
guided-reports CGI; the model now owns that policy and the
controller is just rendering. The TOCTOU window remains identical
in size and is the same intentional trade-off as before.
- Limit=0 (the default) and absent userenv both bypass the check, so
CLI cronjobs (misc/cronjobs/runreport.pl) and direct DB users are
unaffected.
Test plan:
1) prove t/db_dependent/Koha/Reports.t - now includes a new subtest
covering limit=0 (lives), under limit (lives) and at limit (throws
Koha::Exceptions::Report::DuplicateRunning).
2) Set duplicate_running_reports_per_user_limit=1 in koha-conf.xml,
restart_all.
3) Open a long-running report in two staff tabs - second tab shows
the "report is already running" alert (unchanged behaviour).
4) While one run is in flight, hit /cgi-bin/koha/svc/report?id=N as
the same user - now returns HTTP 429 with the JSON error payload
(previously: ran a second copy of the query).
5) Same check on /cgi-bin/koha/opac/svc/report?id=N.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Two small documentation improvements following on from the duplicate-
running guard:
- Koha::Report::prep_report now spells out the tracking-comment format
in its POD. Koha::Reports->running parses this comment, and external
log/audit tooling that consumes Koha SQL relied on the previous
format (/* saved_sql.id: <id> */). Calling out the new format
explicitly makes the public contract visible to anyone reading the
POD or grepping for it, and warns future contributors that changing
the wrapper will break running() too.
- reports/guided_reports.pl gets an inline comment explaining the
TOCTOU window between the count check and the actual report
execution. The window is intentional - the guard exists to stop
accidental double-clicks and two-tab submissions, not to be a
serializing lock - so worst case is one extra concurrent run, which
is acceptable and cheaper than the alternative.
No behaviour change.
Test plan:
1) prove t/db_dependent/Koha/Reports.t still passes.
2) perldoc Koha::Report -> prep_report section now mentions the
saved_sql.id / user_id wrapper format.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Adds a 'running' subtest covering the new method:
- Returns a Koha::Reports resultset (object identity, not opaque ids).
- Returns an empty resultset when nothing is in flight, and when the
user_id / report_id filter does not match any process.
- Parses saved_sql.id markers out of synthetic processlist rows and
resolves them to real Koha::Report objects, ignoring rows without
the marker.
- Sends parameterised LIKE binds for user_id (proves the WHERE clause
is built without string interpolation).
- Falls back to an empty resultset and does not propagate the
exception when the underlying selectall_arrayref call dies, eg.
when the database user lacks PROCESS privilege.
To make the failure path mockable without trying to mock DBI::db
(which is not loadable as a separate file), the dbh selectall is
extracted into a tiny Koha::Reports::_processlist_rows helper so
Test::MockModule can replace it cleanly. The helper is otherwise
behaviour-preserving.
Test plan:
1) prove t/db_dependent/Koha/Reports.t -> 11 tests, all pass.
2) Existing prep_report subtest still passes.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Koha::Reports->running queries information_schema.processlist. A MySQL
user without the PROCESS privilege still sees its own threads, but if
the privilege check fails entirely (or the engine returns an unexpected
error) the previous code would let the DBI exception bubble up and
abort the calling report run.
Wrap the selectall in eval and on failure log via Koha::Logger and
treat the result as empty. This means a misconfigured database
degrades to "no enforcement" rather than to "no reports can be run",
matching the principle that the duplicate-running guard is a safeguard,
not a critical path.
The POD now documents this fallback so future readers know the empty
resultset has two possible meanings: nothing is currently running, or
we could not look. The Koha::Logger->warn ensures the latter is still
observable in the configured log destination.
Test plan:
1) prove t/db_dependent/Koha/Reports.t still passes.
2) Optional: revoke SELECT on information_schema.processlist for the
koha DB user, run a report, observe a warning in the logs and the
report still executes (with no duplicate enforcement).
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
- Fix POD synopsis: invalid syntax and wrong field (userid vs
borrowernumber) replaced with a correct example and parameter list.
- Build the WHERE clause with parameterised bindings via
$schema->storage->dbh, removing string interpolation of
$user_id / $report_id into LIKE patterns and the regex match.
IDs are formatted with %d so non-integer input becomes 0 and cannot
inject regex/LIKE metacharacters.
- Return a Koha::Reports resultset instead of a list of MySQL process
ids. The method name now matches what it returns and callers can use
the standard ->count / iteration interface. The single in-tree caller
in reports/guided_reports.pl is updated; the template error key is
simplified from duplicate_running_report_ids to a boolean
duplicate_running_report (the ids were never displayed).
Test plan:
1) Apply the previous Bug 41918 patches and this follow-up.
2) Set duplicate_running_reports_per_user_limit to 1 in koha-conf.xml.
3) Restart all the things.
4) Open a long-running saved report in two tabs and run it from both.
5) Verify the second tab shows "This report is already running..."
6) Set the limit to 0 and confirm both tabs run normally.
7) prove t/db_dependent/Reports.t still passes.
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Alex Carver [Acerock7] <alex@rcls.org>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Signed-off-by: Bob Bennhoff <bbennhoff@clicweb.org>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Test plan (ktd)
0) Update the database (ktd, ignore if sandbox):
$ updatedatabase
1) Enable 'RealTimeHoldsQueue' and 'DisplayAddHoldGroups' system preferences
2) restart_all # This is required or the worker won't pick up the new sys pref values
3) Place at least 2 holds for a patron on different items/biblios, without grouping them:
<staff_url>/cgi-bin/koha/members/moremember.pl?borrowernumber=19
4) Build the holds queue once to get a baseline:
$ perl misc/cronjobs/holds/build_holds_queue.pl --force
5) Visit the holds queue and confirm the holds appear without a group indicator:
<staff_url>/cgi-bin/koha/circ/view_holdsqueue.pl?branchlimit=&itemtypeslimit=&run_report=1
6) Create a hold group for those holds via the patron page
<staff_url>/cgi-bin/koha/circ/circulation.pl?borrowernumber=<patron_borrowernumber>
7) Without running build_holds_queue.pl, refresh the holds queue, confirm the 'part of a hold group' indicator now appears
8) Ungroup the holds, refresh the holds queue, confirm the 'part of a hold group' is no longer showing
9) Run the unit tests:
$ prove t/db_dependent/Reserves/HoldGroup.t
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Replace the empty =head3 stub with a proper synopsis explaining the URL,
the supported filtering / ordering / embed surface, and the permission
requirement. Aligns the controller with the documentation density of
other Koha REST controllers and removes the "Missing POD" QA noise.
Test plan:
1. perl -MKoha::REST::V1::ActionLogs -e '' (still loads)
2. perl -c Koha/REST/V1/ActionLogs.pm (still parses)
3. perldoc Koha/REST/V1/ActionLogs.pm (renders the new POD)
Signed-off-by: Lucas Gass <lucas@bywatersolutions.com>
Olivia Reynolds
Pedro Amorim
Andrew Fuerste Henry
Lari Taskula
Martin Renvoize
Tomás Cohen Arazi
Jonathan Druart
Eric Phetteplace
apirak
Lucas Gass
Brendan Lawlor
Kyle M Hall