This alternate patch makes multiple markup corrections to the ILL
requests templates. It reformats the request details to use a list,
making it consistent with similar interfaces (see patron details,
baskets in acquisitions). The display of notes has been changed so that
it uses a paragraph tag instead of <pre>.
To test, apply the patch and rebuild the staff client CSS.
- Manage an ILL request which has staff and OPAC notes. Everything
should look readable.
- Running validation on the HTML should only return warnings coming from
global include files (js_includes.inc).
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This bug changes how request metadata is obtained. Previously the
appropriate backend's metadata function would return it. For at least
the FreeForm backend, the metadata property names we title cased. The
datatable JS was expecting this and breaks with the new metadata getting
which is performed in the API controller, which just returns the
property names as they appear in the DB.
This patch changes the expectation of the JS to match what's coming back
from the API
Signed-off-by: Magnus Enger <magnus@enger.priv.no>
Tested by repeatedly loading the API-call
http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,library
in Chromium and watching the reported time to load the URL. There
is a siginificant decrease in the time with the patches compared to
the time it takes without the patches. I had some concerns along the
way, but they have all been fixed.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Tested by repeatedly loading the API-call
http://intranet/api/v1/illrequests?embed=metadata,patron,capabilities,library
in Chromium and watching the reported time to load the URL. There
is a siginificant decrease in the time with the patches compared to
the time it takes without the patches. I had some concerns along the
way, but they have all been fixed.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch removes the potential use of borrower category as a ILL
request ID prefix. It makes no sense. We provide the ability for a site
to define a request prefix based on branch, there is no use case for
using the borrower category. Add to this that the borrower for every
request was being retrieved in order to get the category, it's a huge
performance hit also.
We also now require the <branch> block in the <interlibrary_loans> block
and complain if it's not present. The request prefix should be defined
in this block.
This patch also improves the performance of the API request that returns all
requests, optionally including additional data.
It also deprecates the overloaded TO_JSON method and moves the request
augmentation code into the API route's controller. It may be that we
want to shift it out of there at some point, but it is fine where it is
for now.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
To test:
- Make sure you have a patron with just a surname and no firstname
- Create an ILL request so that the name of this patron shows up in
the Patron column of the ILL requests table
- Before the patch, the name will display as "null Surname"
- Apply the patch
- Reload the ILL requests page
- The name should now display as just "Surname"
- Sign ye merrily off
Signed-off-by: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Modify the conditional display of N/A in accordance with comment #17
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
All N/A strings in ILL templates wrapped in <span> to aid translation
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
- Switch getType -> get_type
- get_type now returns a type or undef
- Ternary now performed in template
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch adds a method to the Illrequest object enabling it to return
its type, which is stored as an Illrequestattribute object
To observe broken state:
- Do not apply patch
- Ensure you have at least one Interlibrary loan created
- In OPAC view, go to "your interlibrary loan requests"
- => TEST: Observe that the "Request type" column is not populated
- Click on "View" for one of the requests
- => TEST: Observer that the "Request type" row is not populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
- => TEST: Observer that the "Request type" row is not populated
To Test:
- Apply patch
- In OPAC view, go to "your interlibrary loan requests"
- => TEST: Observe that the "Request type" column IS populated
- Click on "View" for one of the requests
- => TEST: Observer that the "Request type" row IS populated
- In Staff view, go to "ILL requests"
- Click on "Manage request" on a request
- => TEST: Observer that the "Request type" row IS populated
Signed-off-by: Niamh.Walker-Headon@it-tallaght.ie
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch makes it possible to add arbitrary comments to ILL
requests. Comments are read only and displayed in chronological
order. Comments can be added by librarians, but also added automatically
based on comments in the different protocols, so that comments from the
lending library can also be added.
To test:
- Enable the ILL module ("ILLModule" syspref + config in koha-conf.xml)
- Install the Dummy backend from here:
https://github.com/PTFS-Europe/Dummy
- Create a Dummy ILL request
- Add comments to the request, checking that the name and borrowernumber
of the person that added the comment is displayed ok
- Check that the displayed date and time the comment was submitted follows
the "dateformat" syspref
- On the "List requests" page, check that the correct number of comments
is displayed in the "Comments" column
- Check that the tests pass:
prove t/db_dependent/Illcomments.t
Signed-off-by: andrew.isherwood@ptfs-europe.com
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch has been generated with the script provided on bug 21576.
It only affects variable used in the href attribute of a link *when*
href it the first attribute of the node (grep "a href")
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Why? Because we must filter the variables when we display them.
If we escape them on assignement, they will be double escaped:
[% XXX = "<span>pouet</span>" | html %]
[% XXX | html %]
=> <span>pouet</span>
Also it will bring trouble if we are assigning a structure (see bug
21663 for instance).
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
We don't want to display the link to the biblio if the biblio_id is null
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Carry out changes requested in commenbt #27
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
We should be 'PROCESS'ing any backend JS, not 'INCLUDE'ing it
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Supplier metadata elements have a dynamically formed class name, part of
the name is formed from the metadata key, this breaks if the key has a
space, so we replace spaces
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
On the Manage Request page, borrowernumber should be read only, we do
not want users modifying the destination user once a request is placed
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch removes the erroneously added workflow that requires staff
users to agree to the copyright clearance declaration defined in the
ILLModuleCopyrightClearance preference. Only OPAC users should be
required to accept the declaration.
To test:
1) Ensure you have at least one ILL backend available:
https://wiki.koha-community.org/wiki/ILL_backends
2) Ensure you have the "ILLModule" preference enabled
3) Add some text to the "ILLModuleCopyrightClearance" preference
4) As an OPAC user make an ILL request:
a) Navigate to a search results page in the catalog
b) Click the "Make an Interlibrary Loan request" link at the bottom
c) Choose "Create a new request", then select a backend
d) Observe the text you added earlier is displayed with buttons for
agreeing or disagreeing
5) As a staff user, select the "ILL requests" button on the front page of
the intranet site
a) Choose "Create a new request", then select a backend
b) Observe that you are NOT prompted to agree to the text you added
earlier
Assigned-to: Andrew Isherwood <andrew.isherwood@ptfs-europe.com>
Signed-off-by: Barry Cannon <bc@interleaf.ie>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
The "html" filter that is being applied to these variable declarations
is inappropriate and has been removed.
I've also simplified things by removing two of the extraneous variable
declarations.
To test:
- Before applying the patch, on master, view the "Manage request" page
for an ILL request
- TEST: Observe that, apart from the "Edit request" button, the various
available action buttons do not display correctly
- Apply the patch
- TEST: Refresh the page and observe that the buttons now display
correctly
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Check the following files to see that all "biblio" or "biblio record"are changed to "bibliograhic records" and there are no typos.
modules/ill/ill-requests.tt
modules/installer/step3.tt
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Here we go, next step then.
As we did not fix the performance issue when autofiltering
the variables (see bug 20975), the only solution we have is to add the
filters explicitely.
This patch has been autogenerated (using add_html_filters.pl, see next
pathces) and add the html filter to all the variables displayed in the
template.
Exceptions are made (using the new 'raw' TT filter) to the variable we
already listed in the previous versions of this patch.
To test:
- Use t/db_dependent/Koha/Patrons.t to populate your DB with autogenerated
data which contain <script> tags
- Remove them from borrower_debarments.comments (there are allowed here)
update borrower_debarments set comment="html tags possible here";
- From the interface hit page and try to catch alert box.
If you find one it means you find a possible XSS.
To know where it comes from:
* note the exact URL where you found it
* note the alert box content
* Dump your DB and search for the string in the dump to identify its
location (for instance table.field)
Next:
* Ideally we would like to use the raw filter when it is not necessary
to HTML escape the variables (in big loop for instance)
* Provide a QA script to catch missing filters (we want html, uri, url
or raw, certainly others that I am forgetting now)
* Replace the html filters with uri when needed (!)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
This patch addresses the lack of sanitization of the "notes" field on
the OPAC "View Interlibrary loan request" page.
To test:
- Apply the patch
- As an OPAC user, create an ILL request
- Navigate to the request's "View Interlibrary loan request" page
- Add the following note:
Hello
<h1>TESTING</h1>
<script>alert('pwned');</script>
- Click "Submit modifications"
- TEST: Observe, when the page reloads, only the following is preserved in the
"Notes" textarea:
Hello
TESTING
- As a staff user, naviate to the ILL requests table
- Select "Manage request" for the request you created
- TEST: Observe that the Notes field only contains:
Hello
TESTING
- TEST: Observe that no Javascript alert is displayed
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
This patch improves the display of the extra metadata that may be
returned by an ILL supplier.
The display of the metadata has been moved from the "Toggle full
supplier metadata" link at the bottom of the page to a button on the
right of the request toolbar, labelled "Display supplier metadata".
Clicking this button opens a modal displaying the metadata.
To test:
1) Ensure ILL is enabled and you have at least one request
2) From the "View ILL requests" page, click "Manage request" on a
request
3) Click the "Display supplier metadata" button on the right of the
toolbar
4) Observe that a modal opens containing the metadata
Signed-off-by: Magnus Enger <magnus@enger.priv.no>
The modal looks good!
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Having to write [% KOHA_VERSION %] for each url is bad because:
- It's easily forgettable when adding new <script> or <link>
- It prevents grep'ing for the full filename
- It violates the DRY principle
- If at some point we want to change the "force js and css reload"
mechanism, it will be tedious
This patch:
- adds a Template::Toolkit plugin that generates <script> and
<link> tags for JS and CSS files, and inserts automatically the Koha
version in the filename
- use the new plugin to remove all occurences of [% KOHA_VERSION %]
- remove the code that was adding KOHA_VERSION as a template variable
Test plan:
1. Apply patch
2. Go to several different pages in Koha (opac and intranet) while
checking your browser's dev tools (there should be no 404 for JS and
CSS files, and the Koha version should appear in filenames) and the
server logs (there should be no "File not found")
3. `git grep KOHA_VERSION` should return nothing
4. prove t/db_dependent/Koha/Template/Plugin/Asset.t
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Allow translating "View borrower details" link title in the ILL module.
Signed-off-by: Pasi Kallinen <pasi.kallinen@joensuu.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Before this patch only the first library defined in the system where
displayed.
TODO: Make sure we want to display all the libraries and not a
"filtered" list
see `git grep PROCESS options_for_libraries libraries|grep unfiltered`
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
It will help translators (remove TT tags from po files) and simplify the
code. We do not need this translate_column that is used only once.
It also removes sorting option on the last column (actions)
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch removes the extra columns functionality, simplifying the code.
It removes redundant code (in both Perl and JS), unused vars.
It removes the use of here_link and hardcodes the script path on the template.
It also adjusts the AJAX call so it uses the 'library' param instead of 'branch'.
The library column now displays the library name instead of the ID.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch fixed terminology issues on the UI, and removes CSS-based
case forcing for column names.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
5/ This patch makes Koha::Illrequest->load_backend raise an exception
if the passed backend is invalid. This way we will catch more errors introduced.
The patch also disables the 'New Ill request' when no backends are available. Gets
rid of a related warnings.
Both OPAC and Intranet now display a warning message when no backends
are available.
Tests are added for the load_backend changes.
4/ This patch fixes the path for the checkboxes jquery plugin, and removes the include
for tablesorter, as this implementation uses Datatables. This is obviously code for older
Koha, ported to master.
TODO: There's something wrong on the styling. My idea is to get rid
of the custom column visualization tool, and have it display as regular
DataTables. We can then introduce the use of colvis on a separate bug
report.
Note: POD coverage for the exceptions file is wrongly tested. It is a false positive.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The HTML body tag looked like this:
<body id="acq_suggestion" class="acq">
Probably an overlooked copypasta. This patch changes it to:
<body id="illrequests" class="ill">
This should not have any visual side effects.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This Commit is at the heart of adding an interlibrary loans framework
for Koha. The framework does not prescribe a particular workflow.
Instead it provides a general framework that can be extended &
implemented by individual backends whose responsibility it is to
implement a specific workflow.
The module is largely self-sufficient: it adds new tables to the Koha
database and touches only a few files in the Koha source tree.
Primarily, we add our files to the Makefile and the koha-conf.xml,
define ill paths for the REST API, and introduce links from the main
intranet, opac pages & user permissions.
Outside of this we simply add new files & functionality.
Signed-off-by: Magnus Enger <magnus@libriotech.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
