As OverDriveEnabled is declared via SET in the template and not a system preference and after all not used in that script it is removed.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
OpacHighlightedWords was set in quotation marks and resulted in either "1" or "0".
As "0" and "1" always evaluates to true in JS the following error was thrown when OpacHighlightedWords was set to Don't highlight: Uncaught TypeError: $(...).highlight is not a function
To test:
a) Set system preference OpacHighlightedWords to Don't highlight
b) search a title in the opac with open dev console and check that no error (like above) is thrown.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Three issues introduced by the OPAC port of opac-results.tt:
- The tt-allowed block sets OverDriveEnabled via
`Koha.Preference("OverDriveEnabled")`, but OverDriveEnabled is a
TT-side SET variable (computed from OverDriveLibraryID +
OverDriveClientKey + OverDriveClientSecret), not a system
preference, so Koha.prefs.OverDriveEnabled is always "".
The code at line 621 then reads an unprefixed `OverDriveEnabled`
identifier which is not declared -- the OverDrive result block
never fires.
- The neighbouring block `if (OpenLibrarySearch) { ... }` reads an
unprefixed identifier for a value that is only exposed on
Koha.prefs, so the OpenLibrary result block also never fires.
- Both blocks build a throbber <img> with
`src=\`${interface}/${theme}/images/spinner-small.gif\`` inside a
single-quoted JS string; the backticks are literal characters
there, so the rendered src ends up as that literal template
text, not a usable URL.
Replace the OverDriveEnabled addPrefs entry with a plain
`const OverDriveEnabled = [% OverDriveEnabled ? 1 : 0 | html %];`
that reads the TT SET variable, prefix OpenLibrarySearch with
Koha.prefs, and rebuild the throbber src via normal string
concatenation.
To test:
- With OverDrive configured (OPACOverDrive, OverDriveLibraryID,
OverDriveClientKey, OverDriveClientSecret), run a search in the
OPAC: the OverDrive results panel shows the spinner and then
the API results.
- With OpenLibrarySearch enabled, the OpenLibrary panel does the
same.
- In both cases the spinner image loads (no broken-image icon).
- Disable both prefs -> neither panel is rendered, no console errors.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In the new tt-allowed block, the NovelistSelect fallback declarations
redeclare `NovelistSelectProfile` instead of also exposing
`NovelistSelectPassword`:
var NovelistSelectProfile = "[% NovelistSelectProfile | html %]";
var NovelistSelectProfile = "[% NovelistSelectPassword | html %]";
So `NovelistSelectProfile` ends up holding the password, and
`NovelistSelectPassword` is never declared. The later
`novSelect.loadContentForQuery(..., NovelistSelectProfile,
NovelistSelectPassword, ...)` call then throws ReferenceError and
NovelistSelect content never loads on biblio detail pages.
Rename the second declaration to `NovelistSelectPassword` to match
the original template behaviour.
To test:
- Configure the NovelistSelect sysprefs (NovelistSelectEnabled,
NovelistSelectProfile, NovelistSelectPassword) in the OPAC. Real
NovelistSelect content only loads with valid EBSCO subscription
credentials -- with dummy values the JS path still runs but the
tab stays empty; that is still sufficient to verify this fix.
- Open an OPAC biblio detail page with a known ISBN.
- In the browser console, run
console.log(NovelistSelectProfile, NovelistSelectPassword)
and confirm both values are printed (before the fix,
NovelistSelectPassword is undefined).
- With valid subscription credentials, NovelistSelect content loads
in its tab/panel instead of throwing a ReferenceError in the
console.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The port declares `var print = [% print ? 1 : 0 | html %];` at the
top of the tt-allowed block, then later calls `window.print()` inside
`if (print) { ... }`. Declaring `var print` at top-level overwrites
the built-in `window.print` with 0 or 1, so the subsequent
`window.print()` call throws "print is not a function" and the print
view never fires its actual print dialog.
Rename the flag to `should_print` and update the single `if (print)`
check accordingly.
To test:
- Open a public list with items in the OPAC.
- Click "Print this list" (or load the page with op=print).
- The browser print dialog opens and the window closes afterwards.
- No "print is not a function" error in the console.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The OPAC port moved the dateformat pref into Koha.addPrefs and dropped
the local `var dateformat_pref = ...;` declaration, but four usages of
`dateformat_pref` remain in the same file (the debug alert, two
`switch` statements, and the get_dateformat_str call in validate_date).
At runtime the switches throw ReferenceError, so flatpickr ends up
with the default 'Y-m-d' format and date validation via validate_date
crashes before showing the invalid-date alert.
Replace all remaining occurrences with Koha.prefs.dateformat, which is
populated by the new addPrefs block at the top of the include.
To test:
- Set dateformat syspref to each of us / metric / dmydot / iso.
- Open any OPAC page using a flatpickr input (e.g. hold date, patron
dob on the registration form).
- Confirm the widget renders dates in the expected format and
entering an invalid string triggers the validate_date alert.
- No ReferenceError in the browser console on page load.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.
This patch focuses on the OPAC module.
Non-trivial changes, we modify the whole OPAC in a single patch.
Testing this patch should focus on:
* date formatting
* the cover images on the search result and shelves views
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Removes a left over TT variable from a JS script block without data-tt-allowed="true" attribute. Auto tidy
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The port quoted [% quantityrec | html %] into a string const, but
the value is used as `$(this).val() < quantityrec` where both sides
are now strings and JS does a lexicographic compare ("10" < "5" is
true), giving the wrong branch when the user enters a multi-digit
quantity.
Render quantityrec as a JS number (with a 0 fallback) so the
comparison stays numeric, matching the original template output.
To test:
- Modify an existing order with quantityrec, e.g. 5 items received.
- Change quantity to 10 (higher than received): "can't add new item"
alert fires, value reverts.
- Change quantity to 2 (lower): "don't forget to delete" alert fires.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.
This patch focuses on the acquisition module.
The 2 main changes are in the following files:
* basket.tt
It's mostly about table settings and the visibility of columns depending
on syspref:
- "supplier report" / EDIFACT
- "modify" / if basket is not closed
- "cancel" / CancelOrdersInClosedBaskets
* neworderempty.tt
trivial changes, no error or warning in the console should appear.
Confirm than createing a new order works as expected.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
- There was a mix of "patronphone" and "patron-phone" and I fixed it to
standardize on "patron-phone" (and patron-email) which I find easier
to read.
- I made similar changes to the branchtransfers template
- Corrected the TT filter on mailto links from "html" to "uri"
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch modifies both hold-found-modal and hold-found1. It adds a
class to the patron phone number. For patron email, it changes the
existing id to a class, expands the selector to include the bullet
point in hold-found-modal, and standardizes the name (changing from
"#boreamil' to ".patronemail").
To test:
1 - have a patron with both a phone number and an email address
2 - create a hold for your patron
3 - check an item in to fill your hold, triggering the hold-found-modal
4 - inspect the patron contact info, confirm there is no class on the
phone number
5 - confirm there is an id on the patron email but it does not include
the bullet point and is inconsistent with the names of other classes
in the modal
6 - click Confirm Hold
7 - check your item in again, triggering the hold-found1 modal
8 - confirm there is no class on the patron phone number
9 - confirm there is an id on the patron email with an inconsistent
name
10 - revert the waiting status of your hold
11 - apply patch, restart_all
12 - check your item in again, triggering hold-found-modal
13 - confirm the patron phone now has a class, .patronphone
14 - confirm the patron email now has a class, .patronemail, that the
class includes the bullet point, and that the #boremail id has been
removed
15 - click Confirm Hold
16 - check your item in again, triggering hold-found1 modal
17 - confirm the patron phone now has a class, .patronphone
18 - confirm the patron email now has a class, .patronemail, and that
the #boremail id has been removed
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Currently, if you put either <select multiple> or <select size="10"> in
an <aside><fieldset> sidebar, you get a height of "one fat line" rather
than the bigger box you would expect.
Test plan:
1. Without the patch, load Reports - Use saved
2. Open Firefox developer tools to the Inspector tab
3. With the "Pick an element from the page" tool, click on the word
"Filter" in the Reports page's sidebar
4. Just above that line in the HTML display is a <fieldset
class="brief"> - right click that and choose Edit As HTML
5. After the fieldset element, paste
<select multiple><option>one</option><option>two</option></select><select size="10"></select>
6. Click outside the editor to save your changes
7. Note that both new selects are only one line high
8. Apply patch, run yarn build, Shift+reload to clear your cache
9. Repeat steps 3 through 6
10. Note that both selects are more than one line high
11. Set the preference EnableAdvancedCatalogingEditor to Enable
12. Cataloging - Advanced editor
13. In the left sidebar, verify that the Clipboard is still 10 lines high,
not one fat line like it was before bug 36589
Sponsored-by: Chetco Community Public Library
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch adds additional fields (table subscription, searchable) to the subscription search form on serials home.
To test (ktd):
a) create some additional fields in Admin -> Additional fields -> Serials subscriptions. Make sure the 'Searchable' checkbox is checked.
b) go to serials home: More -> Serials
c) Check that the form is missing the additional fields just defined.
d) apply patch and call restart_all
e) reload serials home and check that the search form is now extended with the additional fields
Signed-off-by: Michaela Sieber <michaela.sieber@kit.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This patch updates several more administration templates with new markup
and CSS for layout forms using CSS grids.
To test, apply the patch rebuild the staff interface CSS.
Check the following pages, confirming that the "New" and
"Edit" versions of the form look correct in each case:
- Administration -> Item search fields
- Administration -> OAI sets
* Updated OAI set mappings form to add maxlength attributes to field
and subfield inputs.
- Administration -> OAI repositories
- Administration -> SMS cellular providers
- Administration -> SMTP servers
Sponsored-by: Athens County Public Libraries
Signed-off-by: Felicie <felicie.thiery@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Presetting biblionumber to null (when adding a new record) will lead to a query parameter biblionumber=null when performing a z39.50 search followed by importing a result. This patch will default biblionumber to "".
Testplan:
a) go to More -> Cataloging -> New Record -> Z39.50/SRU search
b) search for a title f.e. "Programming Perl"
c) import one of the results
d) check that the opened URL has a param biblionumber=null and the addbiblio page displays an error: The record you are trying to edit doesn't exist.
Add a new record or do a catalog search.
e) apply patch and redo steps a) - d) and check that query param biblonumber is 0 and addbiblio loads correct carrying the imported data.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The original UNLESS/ELSE block kept MARC tag links visible by
default and hid them only when the hide_marc syspref was enabled:
[% UNLESS Koha.Preference("hide_marc") %]
toggleMARCTagLinks(true)
[% ELSE %]
toggleMARCTagLinks(false);
[% END %]
The JS port inverted the test without swapping the branches, so
when hide_marc was off the tag links were hidden and when it was
on they were shown -- the opposite of the pref's intent.
Flip the condition back (!Koha.prefs.hide_marc).
To test:
- Set hide_marc = 0: open addbiblio.pl, MARC tag numbers and
subfield codes are visible on first load.
- Set hide_marc = 1 and clear the marctags cookie (devtools ->
Application -> Cookies): reload addbiblio.pl, tag numbers and
subfield codes are hidden on first load.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
GetMandatoryFieldZ3950 (in cataloguing/addbiblio.pl and
authorities/authorities.pl) populates subfield_loo.z3950_mandatory
with the z3950 query-field name expected by z3950_search.pl
(isbn, title, author, issn, lccn, ...). The pre-port JS copied
this value into mandatories_label and used it as the URL parameter
name in GetZ3950Terms.
The BIG_LOOP rewrite stored subfield_loo.marc_lib as `label`
instead. marc_lib is the human-readable field label ("Title
statement"), so GetZ3950Terms started emitting URLs like
&Title+statement=... which z3950_search.pl ignores, and the
z3950 lookup from the editor stopped matching any subfield.
Use subfield_loo.z3950_mandatory for the label in
z3950_mandatory_subfields on both templates.
To test:
- Open addbiblio.pl and fill the ISBN / title / author subfields.
- Click "Z39.50/SRU search".
- In the popup form (or the network request), the pre-filled
fields should correspond to the entered values (title -> Title,
isbn -> ISBN, ...). Before this patch they were empty.
- Same flow from authorities/authorities.pl on a matching
authority type.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The BIG_LOOP rewrite introduced two regressions in the mandatory /
important field collector at the top of addbiblio.tt:
- `important_fields.push([ tag: ..., id: ..., ... ])` wraps the
object in brackets instead of braces. `[a: 1]` is not a valid
array literal, so the parser aborts the whole script tag and
AreFieldsNotOk / PopupZ3950 never run for any record whose
framework marks fields as "important".
- The `id` interpolation joins index and random with a literal `$`
(`"[% index %]$[% random %]"`), but the DOM ids rendered
elsewhere in the template (and looked up by AreFieldsNotOk via
`divid = "tag_" + tag + "_" + id`) concatenate them with no
separator.
Use `{...}` for important_fields and drop the `$` so the collector
shape and ids match the rest of the template.
To test:
- Open addbiblio.pl for a framework that has mandatory and
important fields (e.g. default MARC21).
- Submit a new record with mandatory subfields empty: the error
panel lists them and focuses the first empty input.
- Submit with only important subfields empty: the warning panel
(not blocking) lists them under "The following important
subfields aren't filled".
- Browser console has no SyntaxError on page load.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.
This patch focuses on the edition of biblio and authority records.
We rethink how (the ugly) BIG_LOOP works:
* push all mandatory and important fields in a JS array
* use objects/hashes instead of array to store/access tag, id, index and
tab (vs arr[0], arr[1], ...)
For testing you need to focus on the mandatory and important fields and
subfields. Confirm that there is no change in behaviour introduced by
this patch.
Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
This fixes the hiding of additional fields in mana search by generating an array of additional field ids instead of calling To.json(additional_fields_for_subscription) which led to an Template process error.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
In order to tidy all the script tags of the .tt files we isolate the
interpolation of Template::Toolkit variables in a separate script tag.
This patch focuses on the serials module.
The 3 main changes are in the following files:
* routing.tt
* serials-search.tt
* subscription-detail.tt
Changes are trivial.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jan Kissig <bibliothek@th-wildau.de>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The news RSS link on the OPAC appears to be invalid when the news
content contains HTML. Users cannot subscribe to the feed using
standard RSS readers or email clients.
To reproduce:
1- Go to Tools -> HTML customizations -> News
2- Click New Entry with the following settings:
2-1- Display location: OPAC
2-2- Publication date: today
2-3- Title: "News test"
2-4- Content: click on <> and paste some HTML code, for example:
<span style="color: #ba372a; font-size: 12pt;">Le Prêt
entre bibliothèques est mis en pause.
Désolés de
l'inconvénients.<br />Merci de votre
compréhension.</span>
3- Save the entry.
4- Go to the OPAC and copy the "RSS feed for system-wide library news"
link.
5- Execute: curl -v
http://opac/cgi-bin/koha/opac-news-rss.pl?branchcode= and copy the
XML output to https://validator.w3.org/feed/ to check the feed.
==> The feed is not valid.
6- Try to create a subscription to this link in an email client (e.g.,
Thunderbird).
==> It does not work, giving the error "Feed URL is not valid".
7- Apply the patch.
8- Repeat steps 5 and 6.
==> The feed is now a valid RSS.
9- Repeat step 7.
==> You can now subscribe to the RSS feed using standard readers
and email clients.
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Laura_Escamilla <laura.escamilla@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When running the script bulkmarcimport.pl, the number of MARC records reported at the end of the execution does not match the actual number of records processed from the input file.
This patch fixes 3 issues
- broken (invalid) records from batch get now counted (record_number)
- when using the number param (n) to limit the number of records to be processed valid and invalid records get counted
- when using th offset param (o) the scipt errored when broken records were part of the offset.
To test:
1. Download the MARC file with the broken record
2. Run the following import commands and compare the counted import numbers:
a) all records
perl misc/migration_tools/bulkmarcimport.pl -b -v -m MARCXML --file 2_records_1st_broken.xml
1 MARC records done in 0.0421221256256104 seconds
b) limit to 1 record in total
perl misc/migration_tools/bulkmarcimport.pl -b -v -n 1 -m MARCXML --file 2_records_1st_broken.xml
1 MARC records done in 0.0420489311218262 seconds
c) limit to 1 record offset 1 record
perl misc/migration_tools/bulkmarcimport.pl -b -v -n 1 -o 1 -m MARCXML --file 2_records_1st_broken.xml
:2: parser error : Opening and ending tag mismatch: leader2 line 2 and leader
<leader2>00216nam a22001097a 4500</leader>
3. Apply the patch
4. Repeat steps in 2 and compare the results
a) 2 MARC records done in 0.0405838489532471 seconds
b) 1 MARC records done in 0.00948596000671387 seconds *
c) 1 MARC records done in 0.0294840335845947 seconds
*though this one was skipped
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
When running the script bulkmarcimport.pl, the number of MARC records reported at the end of the execution does not match the actual number of records processed from the input file.
To test:
1. Download the joined MARC file containing 5 records.
2. Run the import command:
./misc/migration_tools/bulkmarcimport.pl -b --file=marc_file.mrc
3. Check the final script report
==> 11 MARC records done in 0.566349983215332 seconds
Expected output should show 5 records
4. Apply the patch
5. Repeat step 2, 3
==> The correct number is shown
Signed-off-by: Janusz Kaczmarek <januszop@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Update the pending offline circulation actions page to link each item
barcode directly to the item details, rather than the biblio record, and
open the links in a new tab.
Add a column to show the last seen date of the items being processed,
with a warning if the item's last seen date is more recent than the
offline circulation action date.
To test:
1. Upload a Koha Offline Circulation (koc) file
(the file attached to this bug can be used if you are using a
sandbox or KTD with the standard sample data)
i. Go to Circulation > Upload offline circulation file
ii. Upload the file
iii. Click "Add to offline circulation queue"
2. Go to Circulation > Pending offline circulation actions
3. Click on the item barcodes in the table
--> Note that each barcode links to the biblio record, and navigates
away from the page within the same tab.
4. Apply patch and restart_all
5. Return to (or refresh) the pending offline circulation actions page
and click on the item barcodes again
--> The barcodes now open the item details page in a new tab
--> There is a new column for the item last seen date
6. Without processing the offline circulation actions, check in one of
the items listed in the table
7. Return to the Pending offline circulation actions page
--> Confirm that the last seen date of the item you just checked in is
highlighted in red with a warning icon, since its last seen date is
newer than the offline transaction date
8. Hover over the warning icon
--> Confirm that a tooltip appears with an explanation of why the date
is in red
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>
This patch adds an option -exact_days, to filter objects by an exact
number of days.
To test:
1 - Apply test patch
2 - Run tests -> they fail
3 - Apply patch
4 - Run tests -> they don't fail
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
t/db_dependent/Log.t
- Add subtest verifying cronlogaction() prefixes loginfo with \$0
(the running script path), appends the info parameter, defaults the
action to 'Run', and respects custom action values.
t/Koha/Script.t
- Assert that \$Koha::Script::_cron is false when the module is loaded
without the -cron flag, confirming INIT/END blocks will not log.
t/Koha/Script_cron.t
- Assert that \$Koha::Script::_cron is true when the module is loaded
with the -cron flag, confirming INIT/END blocks will log.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
The INIT/END blocks in Koha::Script fire for every consumer of the
module, including non-cron admin and migration tools. Logging those as
CRONJOBS when CronjobLog is enabled would pollute the action log with
entries for tools that are not scheduled jobs.
Introduce a package-level $_cron flag that is set by import() when the
-cron argument is passed (import() runs at compile time, before INIT
blocks execute, so the flag is in the correct state when INIT fires).
Guard both the INIT and END blocks behind this flag so only scripts that
explicitly declare themselves as cron scripts receive automatic logging.
Also update the import() POD to document the auto-logging behaviour.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
caller(1) was introduced to skip past Koha/Script.pm when cronlogaction
is called from the INIT/END blocks, but it breaks any direct caller that
invokes cronlogaction from within a Try::Tiny try/catch closure or a
callback sub - in those contexts caller(1) resolves to Try/Tiny.pm (or
the calling module) rather than the script itself.
\$0 is always the path of the running script regardless of call depth,
INIT/END phase, or enclosing closure, making it the correct choice here.
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Since we are calling from within another module we need more caller information to
log the correct script
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Add Koha::Script -cron to restrict_patrons_with_failed_notices.pl
and sftp_file.pl which used cronlogaction directly without importing
Koha::Script. Remove their now-redundant manual calls.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Remove start/end cronlogaction calls and unused C4::Log imports
from scripts that use Koha::Script, as these are now handled
automatically by the INIT/END blocks.
Scripts with mid-execution informational logging (plugins_nightly,
harvest_oai, fines, process_message_queue) retain their C4::Log
import and non-start/end cronlogaction calls.
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Add INIT and END blocks to Koha::Script that automatically call
cronlogaction on script start (with command line options) and
completion. This removes the need for scripts to manually handle
cron log entries.
This will mean that any script importing Koha::Script will be logged,
but I think the addition is beneficial.
To test:
1 - Ensure CronjobLog is enabled
2 - perl misc/cronjobs/fines.pl --test
3 - Confirm start and end are logged in action logs:
http://localhost:8081/cgi-bin/koha/tools/viewlog.pl?src=&user=&modules=CRONJOBS&actions=&object=&info=&interfaces=&from=&to=&output=screen&basename=Export&report_name=&do_it=1
4 - Apply patch
5 - Repeat 2
6 - Reload the logs, confirm the new logs look the same
7 - Sign off
Signed-off-by: Tomás Cohen Arazi <tomascohen@theke.io>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
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.
Test plan:
Confirm Initial Settings
1. Log in to Koha staff with username: koha and password: koha and set your library as Centerville if it's not already.
2. Visit the Saved reports page, and create and save three reports: Report 1, Report 2, and Report 3. The SQL query for each report doesn't matter, it should just be something valid.
3. Visit the Patrons page and create two staff users: StaffReportLimitUser and StaffNoLimitPermission. Set their home libraries to Centerville and Fairfield, respectively.
4. Under Allow access to the reports module, ensure both users have report management permissions for Create, Delete, Execute.
Verify Patch Settings
5. Apply the patch and restart services. If using Koha Testing Docker (KTD): from the Koha repo directory, apply the patch (e.g. `git bz apply 16631`, or check out the patched branch with `git fetch && git checkout <branch>`), then run `ktd --shell` to enter the KTD shell and run `restart_all`. If the patched branch is already checked out, just enter the KTD shell and run `restart_all`.
6. Click Patrons in the top navbar to go to the Patrons page.
7. Search for "StaffReportLimitUser" in the patron search field.
8. Click StaffReportLimitUser to edit the patron record for this user.
9. Click More -> Set permissions to access the permissions screens for the StaffReportLimitUser.
10. In the row for Allow access to the reports module (reports), click Show details to view relevant permissions.
11. Confirm Manage Report Limits (manage_report_limits) is an option under Allow access to the reports module (reports).
12. Click the checkbox for Manage Report Limits to give this user this permission, and click Save.
13. Go to the System Preferences page (Click the More dropdown from the top nav bar, click Administration, click System Preferences).
14. Verify Reports is an option in the left side navigation bar, and that when clicked, you see the LimitReportsByBranch option.
15. Enable LimitReportsByBranch as described in step 14.
16. Go back to Saved reports as in Step 2.
17. Click Edit in the row menu for Report 1.
18. At the bottom of the Report 1 edit page, you should see a Library limitation select box.
Verify the Library Limitation Settings are Persisted
19. Choose Centerville from the select box and click Update SQL.
20. Reload the page, or navigate away from the page and go back to the edit screen for Report 1.
21. Verify that Centerville is still in the Library limitation select box.
22. Add Fairfield in the Library limitation select box and click Update and run SQL.
23. Go back to editing Report 1.
24. Confirm the Library limitation select box has both Centerville and Fairfield selected.
25. Duplicate Report 1.
26. Confirm the Library limitation select box has both Centerville and Fairfield selected.
27. Click Save Report.
28. Confirm the Library limitation select box has both Centerville and Fairfield selected.
Verify Reports are Limited Based on Library Limitations
29. Edit Report 1 to add Centerville in the Library limitation select box as the only choice and click Update SQL.
30. Edit Report 2 to add Fairfield in the Library limitation select box as the only choice and click Update SQL.
31. Report 3 should have no library limitations, so it should be visible to all libraries.
32. Go to Saved reports as in Step 2.
33. Confirm you do not see Report 2, and only Report 1 and Report 3.
34. In the upper right corner, click the drop down that says koha Centerville and click Set library to change your library.
35. Set your library to Fairfield.
36. Go to Saved reports as in Step 2.
37. Confirm you do not see Report 1, and only Report 2 and Report 3.
Verify Disabling LimitReportsByBranch Preference Shows All Reports
38. Disable LimitReportsByBranch preference as in steps 13 to 15.
39. Go to Saved reports as in Step 2.
40. Confirm Report 1, Report 2, Report 3 are visible.
41. Click to edit Report 1 and verify there is no Library limitation select box.
Verify Library limitations are Persisted When EnableFilteringReports is Toggled On or Off
42. Enable LimitReportsByBranch preference as in steps 13 to 15.
43. Ensure your library is set to Fairfield as in step 35.
44. Go to Saved reports as in Step 2.
45. Confirm Report 1 is not visible, and Report 2 and Report 3 are visible.
46. Click to edit Report 2 and verify the Library limitation select box is at the bottom of the page and that it has Fairfield selected.
Verify the Show all reports option shows all reports for users with the Manage Report Limits permission and superlibrarians
47. Go to Saved reports as in Step 2 and in the Show reports dropdown, select Show all reports, and click Apply filter.
48. Confirm the page reloads with all reports visible: Report 1, Report 2, and Report 3 should be visible.
49. Select Show my branch reports from the dropdown and click Apply filter.
50. Confirm Report 1 is not visible, and Report 2 and Report 3 are visible (you should only see reports for your branch, Fairfield).
Verify Report filters return the correct results
51. Go to Saved reports as in Step 2.
52. Verify you're viewing only your branch's reports in the filter.
53. Under Filter in the left sidebar, type "2" in the Keyword field and click Apply filter.
54. Confirm only Report 2 is visible.
55. Under Filter in the left sidebar, type "1" in the Keyword field and click Apply filter.
56. Confirm there are no reports visible, and the message "No saved reports match your criteria" is displayed.
Verify Library Limits column is available to superlibrarians and users with the Manage Report Limits permission
57. Go to Saved reports as in Step 2.
58. Click the Columns dropdown and ensure that the Library limits column is available to select for display. Select it.
59. Confirm that the Library limits column is now visible in the list of reports and that it shows the correct library limitations for each report (e.g., Report 1 should show 1 library limitation and Centerville (CPL) on hover, Report 2 should show 1 library limitation and Fairfield (FFL) on hover, and Report 3 should show no limitation).
Verify the Show all reports and Limit reports options are available to users that are not superlibrarian but have the Manage Report Limits permission
60. For the StaffReportLimitUser, add the permission for Manage Report Limits as described in steps 9 to 12. This will allow this user to see the Show all reports and Limit reports options.
61. Log out of the koha admin account. In the upper right corner, click the drop down that says koha Fairfield and click Log out.
62. Log in as StaffReportLimitUser, the previously created staff user with report permissions.
63. Confirm the staff user's library is Centerville in the upper right corner of the screen.
64. Go to Saved reports as in Step 2.
65. Confirm the filter option for Show reports is visible.
66. Click to edit Report 1.
67. Verify there is a Library limitation select box and that it can be interacted with.
Verify the Show all reports and Limit reports options are not available to users that are not superlibrarian and do not have the Manage Report Limits permission
68. Log out of the StaffReportLimitUser account.
69. Log in as StaffNoLimitPermission, the previously created staff user without the Manage Report Limits permission.
70. Confirm the staff user's library is Fairfield in the upper right corner of the screen.
71. Go to Saved reports as in Step 2.
72. Click the Columns dropdown and ensure that the Library limits column is not in the list of available columns to select for display.
73. Confirm the filter option for Show reports is not available.
74. Click to edit Report 2.
75. Verify there is no Library limitation select box at the bottom of the page.
[NEW] Verify library limits are auto-assigned and preserved correctly for non-permissioned users
76. Still logged in as StaffNoLimitPermission (Fairfield) from the previous section. Go to Saved reports and click New report. Enter "Limit Auto-Assign Test" as the report name and SELECT 1 as the SQL. Confirm there is no Library limitations select box visible on the form. Click Save.
77. Go back to Saved reports. Click Edit for Report 2 (limited to Fairfield). Add `-- test` as a trailing SQL comment and click Update SQL. This simulates a non-permissioned user saving an existing report that already has limits.
78. Go back to Saved reports. Click Edit for Report 3 (no library limits). Add `-- test` as a trailing SQL comment and click Update SQL. This simulates a non-permissioned user saving an existing report that has no limits.
79. Log out and log back in as StaffReportLimitUser (Centerville, has manage_report_limits).
80. Go to Saved reports. Use Show all reports if needed. Find "Limit Auto-Assign Test" and click Edit. Confirm the Library limitations select box shows Fairfield as the only selected library — the report was automatically limited to the creator's home library on create.
81. Find Report 2 and click Edit. Confirm the Library limitations select box still shows only Fairfield — the existing limit was preserved when the non-permissioned user saved the report.
82. Find Report 3 and click Edit. Confirm the Library limitations select box is empty — no limits were introduced when the non-permissioned user saved a report that had no existing limits.
Verify limited reports are not visible to users without permissions
83. While still logged in as StaffReportLimitUser from step 79, copy the URL for the edit page of Report 1.
84. Log out of the StaffReportLimitUser account.
85. Log in as StaffNoLimitPermission, the user without the Manage Report Limits permission.
86. Paste the copied URL for the edit page of Report 1 into the browser and navigate to it.
87. Confirm that you see an error message that says "This report is restricted and cannot be accessed by the current user." and that the page does not load the report details.
Run Unit Tests
88. Using Koha Testing Docker, in the Koha home directory run the command: ktd --shell to start the ktd shell.
89. Run command: reset_all to drop and recreate the koha database.
90. Run command prove t/db_dependent/Koha/Reports.t to run the test.
91. This file contains the unit tests for the reports_branches table and the new subtest `library limits enforcement on save and update`, which covers auto-assignment of branch limits on create and preservation of limits on update for non-permissioned users.
92. All tests should pass.
93. Run command prove t/Koha/Auth/Permissions.t
94. Permissions unit tests in this file were updated.
95. All tests should pass.
96. Run command prove t/db_dependent/Reports/Guided.t
97. The Guided report tests were updated to account for the new library limitations and permissions.
98. All tests should pass.
Signoff
99. Sign off if the steps above check out.
Sponsored-by: Ramapo Catskill Library System
Co-authored-by: Casey Conlin <casey.conlin@gmail.com>
Signed-off-by: Emmi Takkinen <emmi.takkinen@koha-suomi.fi>
Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
PDF::Reuse 0.43 fixes a TTFont/docProxy lifecycle bug that caused
prEnd() to crash when TrueType fonts were used.
PDF::Reuse::Barcode 0.09 is the current stable release.
Testing plan:
- Start ktd, note older perl modules versions under About Koha
- Apply patch
- ktd --shell
- sudo cpan -i PDF:Reuse; sudo cpan -i PDF:Reuse:Barcode
- Let cpan auto set itself up (not sure if sudo is needed above)
- `restart_all`
- Revisit About Koha; module versions should be 0.43 & 0.09 respectively
Signed-off-by: Eric Phetteplace <ephetteplace@cca.edu>
Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk>
Signed-off-by: Pedro Amorim <pedro.amorim@openfifth.co.uk>
Jan Kissig
Paul Derscheid
Jonathan Druart
Owen Leonard
Andrew Fuerste Henry
Phil Ringnalda
Hammat Wele
Emily Lamancusa
Baptiste Wojtkowski
Nick Clemens
Martin Renvoize
Pedro Amorim
Casey Conlin
Cameron E. Tidd
Chris Nighswonger