Bug 7317

Summary: Add an Interlibrary Loan Module to Circulation and OPAC
Product: Koha Reporter: Sarah Clatterbuck <sarah>
Component: CirculationAssignee: Alex Sassmannshausen <alex>
Status: CLOSED FIXED QA Contact: Tomás Cohen Arazi <tomascohen>
Severity: new feature    
Priority: P5 - low CC: alex, benjamin.rokseth, black23, bob, bouzid.fergani, caroline.cyr-la-rose, dch, dcook, eric.begin, francois.charbonnier, frank.hansen, gmcharlt, hagud, janet.mcgowan, joe.mcglynn, jonathan.druart, jonathan.field, josef.moravec, katrin.fischer, kyle, lisettepalouse+koha, magnus, manueltamayao, martin.renvoize, melia, mirko, mtompset, niamh.walker-headon, nick, philippe.blouin, pongtawat, sally.healey, sarah, severine.queune, tomascohen, veron
Version: Main   
Hardware: All   
OS: All   
URL: http://wiki.koha-community.org/wiki/ILL_Module
See Also: http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=9852
http://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=14610
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19822
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=19605
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18590
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=18592
Change sponsored?: --- Patch complexity: Large patch
Documentation contact: Documentation submission:
Text to go in the release notes:
Adds the ability to place interlibrary loan requests in Koha. Interlibrary loan requests are common especially in academic or special libraries where the enduser or librarian may be entitled to request a book or article from an external library such as the British Library (BLDSS) using their interlibrary loan service. This patch set comes with 3 configured backends - BLDSS, Freeform and a Dummy account which is the default for testing. The BLDSS backend allows users to search stock held by the British Library using their api, and allows requests to be placed against it. The Freeform backend allows the creation of Interlibrary loan requests using a manual form. Interlibrary loan requests can be made from the OPAC or from the staff client. The enduser can query the backend database and place requests. Alternatively they can create requests using the manual form. From the OPAC interface endusers can view and comment on their requests if enabled. In the staff client librarians can manage requests placed on the OPAC, processing them against a backend (e.g. British Library). Alternatively the system allows requests to be placed with peer libraries. Peer libraries can be identified by an Organizational patron category type and requests can be sent to the peer library by email. Core request information such as links to borrowers, branch, request status, staff and customer notes, unique identifiers are stored in the database and can be displayed and reported on.
Version(s) released in:
Bug Depends on:    
Bug Blocks: 18203, 19587, 19588, 19773, 18588, 19810, 20001, 20041, 20284, 20515, 20536, 20556, 20647, 20651, 20712, 20770, 20772, 20883, 21377, 21468, 21516, 21585, 21595, 22224    
Attachments: Bug 7317: Add SQL templates & sysprefs.
DO NOT PUSH: Updated Schema files.
ILLModule
Remove BLDSS configuration from Config.pm.
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: ILLModule
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: ILLModule
Bug 7317: Bugfix: copy ill folder during installation.
Bug 7317: Allow journey to 'COMP' status.
Bug 7317: Make error messages slightly more user friendly.
Bug 7317: Allow display/hiding of arbitrary request columns
Bug 7317: Bugfix: Use correct column names in check_limits.
Bug 7317: Add missing reference for `Koha::Illrequest`.
Bug 7317: Followup: Update atomicupdate file.
Bug 7317: Add `USE Koha` to intranet-main.tt.
Bug 7317: Add branch details to generic emails.
Bug 7317: Invoke `handle_commit_maybe` on custom capabilities.
Bug 7317: Bugfix: Ensure current branch exists.
Bug 7317: Bugfix: remove `my` statements.
Bug 7317: Bugfix: remove `my` statements & dereference.
Bug 7317: Bugfix: remove `my` statements & dereference.
Bug 7317: Switch to borrowername from borrowernumber
Bug 7317: Add date requested to OPAC requests table.
Bug 7317: Add ILLModuleCopyrightClearance syspref.
Bug 7317: Add generic Copyright Clearance functionality.
Bug 7317: Implement generic optional capability system.
Bug 7317: Fix _limit_counter.
Bug 7317: Pass full ILL configuration along within config.
Bug 7317: Only allow cancellation request when status is new.
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: ILLModule
Bug 7317: Bugfix: copy ill folder during installation.
Bug 7317: Allow journey to 'COMP' status.
Bug 7317: Make error messages slightly more user friendly.
Bug 7317: Allow display/hiding of arbitrary request columns
Bug 7317: Bugfix: Use correct column names in check_limits.
Bug 7317: Add missing reference for `Koha::Illrequest`.
Bug 7317: Followup: Update atomicupdate file.
Bug 7317: Add branch details to generic emails.
Bug 7317: Invoke `handle_commit_maybe` on custom capabilities.
Bug 7317: Bugfix: Ensure current branch exists.
Bug 7317: Bugfix: remove `my` statements & dereference.
Bug 7317: Switch to borrowername from borrowernumber
Bug 7317: Add date requested to OPAC requests table.
Bug 7317: Add ILLModuleCopyrightClearance syspref.
Bug 7317: Add generic Copyright Clearance functionality.
Bug 7317: Implement generic optional capability system.
Bug 7317: Fix _limit_counter.
Bug 7317: Pass full ILL configuration along within config.
Bug 7317: Only allow cancellation request when status is new.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: ILLModule
Bug 7317: Bugfix: copy ill folder during installation.
Bug 7317: Allow journey to 'COMP' status.
Bug 7317: Make error messages slightly more user friendly.
Bug 7317: Allow display/hiding of arbitrary request columns
Bug 7317: Bugfix: Use correct column names in check_limits.
Bug 7317: Add missing reference for `Koha::Illrequest`.
Bug 7317: Followup: Update atomicupdate file.
Bug 7317: Add branch details to generic emails.
Bug 7317: Invoke `handle_commit_maybe` on custom capabilities.
Bug 7317: Bugfix: Ensure current branch exists.
Bug 7317: Bugfix: remove `my` statements & dereference.
Bug 7317: Switch to borrowername from borrowernumber
Bug 7317: Add date requested to OPAC requests table.
Bug 7317: Add ILLModuleCopyrightClearance syspref.
Bug 7317: Add generic Copyright Clearance functionality.
Bug 7317: Implement generic optional capability system.
Bug 7317: Fix _limit_counter.
Bug 7317: Pass full ILL configuration along within config.
Bug 7317: Only allow cancellation request when status is new.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: Remove obsolete files.
Bug 7317: Add unit tests & fix, add comments to Illrequest.pm.
Bug 7317: Add unit tests & fix, add comments to Illrequest.pm.
Bug 7317: Add unit tests & fix, add comments to Illrequest.pm.
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: Interlibrary loans framework for Koha.
Bug 7317: DO NOT PUSH: Update OPAC CSS
ILL overview & light documentation
Bug 7317: (QA folowup) 404 if module is disabled (intranet)
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug 7317: (QA folowup) 404 if module is disabled (intranet)
Bug 7317: Overload Koha::Illrequest::TO_JSON
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug 7317: Adjust packages koha-conf.xml
Bug 7317: Adjust packages koha-conf.xml
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: Interlibrary loans framework for Koha.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: (QA folowup) 404 if module is disabled (intranet)
Bug 7317: Overload Koha::Illrequest::TO_JSON
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug 7317: Adjust packages koha-conf.xml
Bug 7317: (followup) Fix id and class on body tag
Bug 7317: Rewrite atomicupdate file
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: Interlibrary loans framework for Koha.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: (QA folowup) 404 if module is disabled (intranet)
Bug 7317: Overload Koha::Illrequest::TO_JSON
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug 7317: Adjust packages koha-conf.xml
Bug 7317: (followup) Fix id and class on body tag
Bug 7317: Rewrite atomicupdate file
Bug 7317: Add SQL templates & sysprefs.
Bug 7317: DO NOT PUSH: Updated Schema Files.
Bug 7317: Interlibrary loans framework for Koha.
Bug 7317: DO NOT PUSH: Update OPAC CSS
Bug 7317: (QA folowup) 404 if module is disabled (intranet)
Bug 7317: Overload Koha::Illrequest::TO_JSON
Bug 7317: (followup) Migrate endpoint to OpenAPI
Bug 7317: Adjust packages koha-conf.xml
Bug 7317: (followup) Fix id and class on body tag
Bug 7317: Rewrite atomicupdate file
Bug 7317: QA followup
Bug 7317: Handle backend absense more gracefuly
Bug 7317: QA followup
Bug 7317: Handle backend absense more gracefuly
Bug 7317: Handle backend absense more gracefuly
Bug 7317: (QA followup) Get rid of warnings from the tests
Bug 7317: Handle backend absense more gracefuly
Bug 7317: (QA followup) Get rid of warnings from the tests
Bug 7317: opac.css update
Bug 7317: Handle backend absense more gracefuly
Bug 7317: (QA followup) Get rid of warnings from the tests
Bug 7317: opac.css update
Bug 7317: Handle backend absense more gracefuly
Bug 7317: (QA followup) Get rid of warnings from the tests
Bug 7317: opac.css update
Bug 7317: Add partner_code checks to abot page
Bug 7317: (followup) Make tests independent of data on the DB
Bug 7317: (QA followup) ILL sysprefs belong to circulation section
Bug 7317: Handle missing email addresses gracefuly
Bug 7317: (QA followup) Fix capitalization and terminology issues
Bug 7317: Add missing 'use Encode' statement
Bug 7317: (QA followup 2) Fix capitalization and terminology issues
Bug 7317: use preventDefault to prevent the scrollbar to move to the top
Bug 7317: Fix more translation issues
Bug 7317: (QA followup) Rename 'branch' for 'library'
Bug 7317: (followup) Remove extra columns from backends on the main list
Bug 7317: Remove traces of UnmediatedILL
Bug 7317: (QA followup) Make query parameters consistent with other endpoints
Bug 7317: (QA followup) Make query parameters consistent with other endpoints
Bug 7317: Fix infinite loop in OPAC
Bug 7317: (QA followup) Remove MySQL-ism from Koha::Illrequest::_limit_counter
Bug 7317: Help translators removing TT tags from po files
Bug 7317: Help translators removing TT tags from po files
Bug 7317: Simplify column headers
Bug 7317: Do not use GET for deletion requests
Bug 7317: Make fields mandatory when sending emails
Bug 7317: Display all the libraries, not only one
Bug 7317: Display all the libraries, not only one
Bug 7317: Fix translatable-templates.t failure

Description Sarah Clatterbuck 2011-12-04 18:58:14 UTC
A library I consult for is interested in Koha as an ILS solution. There are two blockers currently for the adoption of Koha. The first is an Interlibrary Loan module. The library currently uses Innovative Millennium software, so that functionality will serve as the base requirements for this feature.

This bug will serve as the enhancement request for Interlibrary Loan.

I plan to contribute this feature over the coming months.

There will be a Circulation piece and an OPAC piece for this module.
Comment 1 Katrin Fischer 2011-12-04 19:26:24 UTC
Hi Sarah, 

thank you for opening a bug about this :)

There is already a page on the wiki where we have started to collect information about ILL protocols and core functionality:
http://wiki.koha-community.org/wiki/Inter_Library_Loan

I suggest putting your ideas and plans in an RFC on the wiki too. This way other libraries interested in that feature can see what's being planned. It can also help to get ideas and create discussion about how how this can fit into Koha in a good and flexible way. 

There is also another open source project related to ILL that might be interesting: http://www.fulfillment-ill.org/about.php
Comment 2 Owen Leonard 2011-12-05 16:17:37 UTC
I second the recommendation to look at the FulfILLment project. I've just volunteered to help test the project's Koha connector. I haven't gotten as far as seeing anything in action yet but Equinox has a great reputation so I'm looking forward to it.

At the very least getting to know that project better might give you some guidance on what problems it *doesn't* solve that you would have yours do or vice versa.
Comment 3 Sarah Clatterbuck 2011-12-05 19:54:35 UTC
Katrin, 

I will be documenting the current functionality and developing and RFC wiki over the next week or so. 

Owen, 

My understanding is that the FulfILLment project will not be ready for some time yet. I'm thinking about creating an iterative feature where we support ILL "base", which would allow the receiving library to track a request and manage the asset and then in the future add the enhanced functionality that would be a holistic system for both sides of the transaction (likely via FulfiLLment). Thoughts? 

-Sarah
Comment 4 Owen Leonard 2011-12-06 16:31:05 UTC
(In reply to comment #3)
> My understanding is that the FulfILLment project will not be ready for some
> time yet.

I don't know. I certainly wouldn't undertake a big development project without knowing for sure. On the other hand, a solution that didn't require a separate system (like FulfILLment) might be preferable to some libraries.

> Thoughts? 

I don't have any smart things to say about how an ILL addon should or shouldn't work, I just wanted to make sure you were taking  FilfILLment into account.
Comment 5 Mirko Tietgen 2015-02-28 08:42:24 UTC Comment hidden (obsolete)
Comment 6 Alex Sassmannshausen 2015-03-15 16:18:57 UTC Comment hidden (obsolete)
Comment 7 Bob Birchall 2017-01-23 01:05:45 UTC Comment hidden (obsolete)
Comment 8 Alex Sassmannshausen 2017-01-26 09:19:12 UTC Comment hidden (obsolete)
Comment 9 Andrew Isherwood 2017-02-03 17:35:53 UTC Comment hidden (obsolete)
Comment 10 Andrew Isherwood 2017-02-03 17:36:01 UTC Comment hidden (obsolete)
Comment 11 Andrew Isherwood 2017-02-03 17:36:07 UTC Comment hidden (obsolete)
Comment 12 Andrew Isherwood 2017-02-03 17:37:31 UTC Comment hidden (obsolete)
Comment 13 Magnus Enger 2017-02-06 08:04:18 UTC Comment hidden (obsolete)
Comment 14 Andrew Isherwood 2017-02-06 08:34:11 UTC Comment hidden (obsolete)
Comment 15 Andrew Isherwood 2017-02-06 09:04:41 UTC Comment hidden (obsolete)
Comment 16 Andrew Isherwood 2017-02-21 09:01:30 UTC Comment hidden (obsolete)
Comment 17 Andrew Isherwood 2017-02-21 09:01:58 UTC Comment hidden (obsolete)
Comment 18 Andrew Isherwood 2017-02-21 09:02:19 UTC Comment hidden (obsolete)
Comment 19 Andrew Isherwood 2017-02-21 09:02:48 UTC Comment hidden (obsolete)
Comment 20 Alex Sassmannshausen 2017-02-21 09:07:14 UTC
Table of Contents
─────────────────

1 Change Log (since last set of patches pushed to bugzilla)
.. 1.1 Core module
.. 1.2 Backends
..... 1.2.1 New backend: FreeForm
..... 1.2.2 New backend: BLDSS (British Library)
..... 1.2.3 Dummy
2 Module Features
.. 2.1 What is handled by the module?
.. 2.2 What is not yet handled
3 Installation
4 Optional
5 Missing before inclusion in community
6 Test plan
.. 6.1 Core Module (+ Dummy backend)
..... 6.1.1 Creating requests (Staff)
..... 6.1.2 Creating requests (Patrons)
..... 6.1.3 Editing requests (Staff)
..... 6.1.4 Editing requests (Patrons)
..... 6.1.5 Requesting Cancellation (Patrons)
..... 6.1.6 Confirming requests (Staff)
..... 6.1.7 Reverting requests (Staff)
..... 6.1.8 Deleting requests (Staff)
..... 6.1.9 Place request with partners (Staff)
.. 6.2 BLDSS backend
.. 6.3 FreeForm backend


1 Change Log (since last set of patches pushed to bugzilla)
═══════════════════════════════════════════════════════════

1.1 Core module
───────────────

  • Many improvements to general documentation in UI and configuration.
  • Many improvements to the user interface.
  • OPAC interface to viewing and commenting on their requests.
  • OPAC interface for creating new requests with backends of choice.
  • Add 'metadata' abstraction allowing backends to translate raw data
    from their source to user friendly content.
  • Add 'custom_capability's, means by which backends can extend the
    workflow of the ILL module in a seamless manner.
  • Add support for custom JS blocks in ILL backend template includes.
  • Add generic 'backend request -> email to partners' functionality.
  • Remove loads of legacy code.


1.2 Backends
────────────

1.2.1 New backend: FreeForm
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • Allows creation of ILL requests using a manual form, which can be
    extended at creation time.


1.2.2 New backend: BLDSS (British Library)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • Allows searching stock held by the british library, and placing
    requests against it.
  • By default uses test account, which means it is useful for learning.


1.2.3 Dummy
╌╌╌╌╌╌╌╌╌╌╌

  • Upgrade to support latest features.


2 Module Features
═════════════════

  • Extensible core for handling interlibrary loan requests in Koha
  • As many backends can be added as desirable
  • OPAC request form as well as Intranet request form
  • Intranet request management
  • Ability to create items against a backend (e.g. British Library),
    using its advanced search features, and to then request the item
    from peer libraries.


2.1 What is handled by the module?
──────────────────────────────────

  • Create new requests
  • Allow staff and customer notes
  • Process those requests using custom workflows for backends
  • Retain associations between requests and biblioitems that may be
    created locally
  • Peer lending libraries contact management (using Koha's
    Organizational borrower types)
  • Storage of core request information (borrower links, branch links,
    request status, staff and customer notes, unique identifiers)
  • Storage of arbitrary data returned by backend, and display of that
    data.


2.2 What is not yet handled
───────────────────────────

  • Automatic processing of received ILLs (biblio + biblioitem creation)
  • Integration into specialized ILL loan & reminder policies


3 Installation
══════════════

  • Apply patches (they should apply cleanly and without trouble on
    master)
  • Run databaseupdate to generate the ILL tables.
  • Copy the 'interlibrary_loans' element from
    '$kohaclone_root/etc/koha-conf.xml' to your active koha_conf.xml
  • Update the path in 'interlibrary_loans > backend_dir' in
    koha_conf.xml to point to '$kohaclone_root/Koha/Illbackends'
  • git clone backends into the backend directory:
    ┌────
    │ cd $kohaclone_root/Koha/ && mkdir Illbackends && cd Illbackends
    │ git clone https://github.com/a-sassmannshausen/{Dummy,FreeForm,BLDSS}
    └────
  • Enable ILLModule in syspreferences


4 Optional
══════════

  • Ensure latest Koha-Conf configuration values for interlibrary_loans
    are set (see $kohaclone_root/etc/koha-conf.xml)
  • Create peer lending libraries (by default they are expected to be
    part of the 'ILLLIBS' borrower category, which also needs to be
    created).


5 Missing before inclusion in community
═══════════════════════════════════════

  • Unit tests
  • Documentation (code + manual)
  • Unmediated ILL Request placement workflow
  • Advanced configuration
  • Complete Generic Requests workflow (allow setting requests to
    completed in this way, allow requests to resume backend specific
    workflows after generic email is sent)
  • Friendlier error handling.


6 Test plan
═══════════

6.1 Core Module (+ Dummy backend)
─────────────────────────────────

  • Ensure the Dummy backend is installed (see [Installation])
  • Enable the ILLModule (located under Administration sysprefs)
  • Create a test patron, grant them at least ILL and usual staff
    permissions.
  • Browse to 'Home > ILL requests' (button should be visible)


[Installation] See section 3

6.1.1 Creating requests (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • Click on the 'New ILL request' button.
    ⁃ If you have multiple backends installed, select 'Dummy'
  • Complete the form
    ⁃ Borrowernumber must be a borrowernumber, not a cardnumber.
    ⁃ You will always be shown the same 2 results
  • Request one of the items
  • You have now 'created' the request


6.1.2 Creating requests (Patrons)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • Log in to the OPAC
  • Click on 'Your interlibrary loan requests'
  • Click on 'Create a new request'
    ⁃ If you have multiple backends installed, select 'Dummy'
  • Complete the form
    ⁃ As above, you will always be shown the same 2 results
  • Request one of the items
  • You have now 'created' the request.


6.1.3 Editing requests (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • From the ill-requests list, click on 'Manage request'
  • Click on 'Edit request'
  • Add some staff notes (or change other values)
  • Click 'Submit'.


6.1.4 Editing requests (Patrons)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • From the patron screen, click on 'Your interlibrary loan requests'
  • click on 'View'
  • Enter some notes.
  • Click on 'Submit modifications'


6.1.5 Requesting Cancellation (Patrons)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • From the patron screen, click on 'Your interlibrary loan requests'
  • Click on 'View'
  • Click on 'Request Cancellation'
  • The request status should have changed
  • The request can now be cancelled by staff if they consider the
    cancellation is still possible


6.1.6 Confirming requests (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  Once a request is created, it exists in the ILLModule.  But the
  request has not yet been ordered from the source.  To do the latter, a
  request must be confirmed by staff:
  • From the ill-requests list, click on 'Manage request'
  • Click on 'Confirm request'.
    ⁃ The following process will be different for each backend.
    ⁃ In the case of Dummy, a simple click on the button is sufficient.
    ⁃ Other backends may take you through several screens.


6.1.7 Reverting requests (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  Once a request is confirmed it may still be possible to undo this
  operation.  This depends on the backend used, and the policies at the
  source.
  • From the ill-requests list, click on 'Manage request'
  • Click on 'Revert request'.
    ⁃ The following process will be different for each backend
    ⁃ In the case of Dummy, a simple click on the button is sufficient.


6.1.8 Deleting requests (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  • From the ill-requests list, click on 'Manage request'
  • Click on 'Delete request'
    ⁃ This button will only be visible when the request has not yet been
      confirmed.


6.1.9 Place request with partners (Staff)
╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌

  Rather than use the sources workflows to source a title for a specific
  interlibrary loan, it may be desirable to ask a friendly library
  whether they are able to satisfy the request.

  The ILLModule supports this through "Generic requests".  To use this
  you will need to create 'Organizational patrons' to represent the
  partner libraries.  They must be of the 'ILLLIBS' patron category
  (this can be changed by editing the koha-conf.xml file), which must
  first be created.

  Ensure that these patrons have email addresses!

  Once a few patrons have been set up:
  • From the ill-requests list, click on 'Manage request'
  • Click on 'Place request with partners'
    ⁃ This button will only be visible if the request has not yet been
      confirmed.
  • Complete the form
    ⁃ You may select multiple partners in the select list
    ⁃ You may edit the email as desired.
  • Click on Send email
  • If your server is correctly configured to send emails, all should
    have worked fine and an email should have been sent.


6.2 BLDSS backend
─────────────────

  A backend to place and manage requests with the British library.

  • Repeat tests as above.
  • The workflows will be different for:
    ⁃ 'create'
    ⁃ 'confirm'
  • There is an additional workflow:
    ⁃ 'status': retrieve the current status of the request at the
      British library.


6.3 FreeForm backend
────────────────────

  A backend to allow arbitrary manual creation of requests using an
  unrestricted form of fields & values.

  This backend has no 'source'.  As such, requests that are created need
  to then be 'confirmed' outside of ILLModule (or using the generic
  requests functionality).

  • Repeat tests as above.
  • The workflows will be different for:
    ⁃ 'create'
    ⁃ 'confirm'
    ⁃ 'revert request'
Comment 21 Alex Sassmannshausen 2017-02-22 08:50:46 UTC Comment hidden (obsolete)
Comment 22 Daphne Hoolahan 2017-04-12 10:15:07 UTC
It all went pretty good and worked fine.

We just found a couple of things that are worth pointing out:

6.1.1 – Why is the biblio number empty on this screen? Actually one of them has the value zero now that we went through the whole process.

6.1.2 – we noticed that we could request the same title more than once. Maybe not an issue but worth mentioning.

6.1.9 – we received the email fine but it does not say who it is from, ther than it has the from email address. Should it have the from organisation above the status or is the staff member expected to customise the email text to indicate their organisation?

Dear Sir/Madam,

    We would like to request an interlibrary loan for a title matching the following description:

  - Status: New
  - Author: A.N. Other
  - ID: 5678
  - Title: Interlibrary loans in Koha

Please let us know if you are able to supply this to us.

Kind Regards

6.2 – We found it difficult to find any matching titles. What is keywords for? (especially as title has a separate entry field)

image attached

Also we got an error when we tried this search – didn’t seem to like the book filter:
Error performing operation
image attached

When trying to confirm a request from the BL

Software error:

The API responded with an error: 500 Internal Server Error
Detail: <?xml version="1.0" encoding="UTF-8"?><apiResponse><timestamp>2017-03-28 13:15:56.941 BST</timestamp><status>8</status><message>Authentication failed: Exception occurred processing authentication, please contact BL Customer Services</message><result /></apiResponse> at /usr/share/koha-interleaf/Koha/Illbackends/BLDSS/Base.pm line 481.
For help, please send mail to the webmaster (alex.sassmannshausen@ptfs-europe.com), giving this error message and the time and date of the error.
Comment 23 Alex Sassmannshausen 2017-04-24 13:25:06 UTC
Hi Daphne,

Thank you very much for taking the time to test!

I've added some comments in line.

(In reply to Daphne Hoolahan from comment #22)
> It all went pretty good and worked fine.
> 
> We just found a couple of things that are worth pointing out:
> 
> 6.1.1 – Why is the biblio number empty on this screen? Actually one of them
> has the value zero now that we went through the whole process.

The biblio number would be the number that a library assigns to an ILL when they receive it, before they lend it out to a borrower.  As a result, the ILLmodule will just allow it to be empty and never assigns a value to it.  It is the librarian that manually assigns a number to it.

I need to investigate why the biblio is set to 0 in more detail, but I believe this is because a blank field is converted to a 0 at some point in conversion from form input to koha object.

> 6.1.2 – we noticed that we could request the same title more than once.
> Maybe not an issue but worth mentioning.

Indeed — the module currently carries out no validation.  This might be an interesting enhancement at a later stage!

> 6.1.9 – we received the email fine but it does not say who it is from, ther
> than it has the from email address. Should it have the from organisation
> above the status or is the staff member expected to customise the email text
> to indicate their organisation?
> 
> Dear Sir/Madam,
> 
>     We would like to request an interlibrary loan for a title matching the
> following description:
> 
>   - Status: New
>   - Author: A.N. Other
>   - ID: 5678
>   - Title: Interlibrary loans in Koha
> 
> Please let us know if you are able to supply this to us.
> 
> Kind Regards

The email headers (from address) should be set at the server level.  I.e. changing the server configuration should fix that issue.  Having said that, it may well be a good idea to add a hard-coded block with the libraries delivery address to this template.

A future patch should use notice templates to allow customization of these emails.

> 6.2 – We found it difficult to find any matching titles. What is keywords
> for? (especially as title has a separate entry field)

The keywords field should just search "anywhere".  Adding more keywords should significantly reduce the number of hits.

The searches against the test system will be searching against the British Library test API — which I'm very confident has an extremely limited result set.

I believe that explains the difficulty you had in finding results?

> image attached
> 
> Also we got an error when we tried this search – didn’t seem to like the
> book filter:
> Error performing operation
> image attached
> 
> When trying to confirm a request from the BL
> 
> Software error:
> 
> The API responded with an error: 500 Internal Server Error
> Detail: <?xml version="1.0"
> encoding="UTF-8"?><apiResponse><timestamp>2017-03-28 13:15:56.941
> BST</timestamp><status>8</status><message>Authentication failed: Exception
> occurred processing authentication, please contact BL Customer
> Services</message><result /></apiResponse> at
> /usr/share/koha-interleaf/Koha/Illbackends/BLDSS/Base.pm line 481.
> For help, please send mail to the webmaster
> (alex.sassmannshausen@ptfs-europe.com), giving this error message and the
> time and date of the error.

This is a peculiar error message, as it's to do with authentication.  I expect a great number of request confirmations against the BL test system to fail, but normally with different error messages.

I will investigate this issue in more detail.

I will also add a block with library details to the generic email template.  Probably just after the "Kind regards".  What do you think?

Best wishes,

Alex
Comment 24 Magnus Enger 2017-04-28 12:55:53 UTC Comment hidden (obsolete)
Comment 25 Daphne Hoolahan 2017-04-28 13:20:09 UTC
I will also add a block with library details to the generic email template.  Probably just after the "Kind regards".  What do you think?

That's an excellent idea.
Comment 26 Daphne Hoolahan 2017-04-28 13:22:08 UTC
The keywords field should just search "anywhere".  Adding more keywords should significantly reduce the number of hits.

The searches against the test system will be searching against the British Library test API — which I'm very confident has an extremely limited result set.

I believe that explains the difficulty you had in finding results?

Daphne:Agreed
Comment 27 Alex Sassmannshausen 2017-05-02 08:21:49 UTC Comment hidden (obsolete)
Comment 28 Magnus Enger 2017-05-03 09:35:20 UTC Comment hidden (obsolete)
Comment 29 Alex Sassmannshausen 2017-05-04 10:32:37 UTC Comment hidden (obsolete)
Comment 30 Alex Sassmannshausen 2017-05-04 10:32:48 UTC Comment hidden (obsolete)
Comment 31 Alex Sassmannshausen 2017-05-04 10:32:56 UTC Comment hidden (obsolete)
Comment 32 Alex Sassmannshausen 2017-05-04 10:33:06 UTC Comment hidden (obsolete)
Comment 33 Alex Sassmannshausen 2017-05-04 10:33:14 UTC Comment hidden (obsolete)
Comment 34 Alex Sassmannshausen 2017-05-04 10:33:20 UTC Comment hidden (obsolete)
Comment 35 Alex Sassmannshausen 2017-05-04 10:33:28 UTC Comment hidden (obsolete)
Comment 36 Alex Sassmannshausen 2017-05-04 10:33:35 UTC Comment hidden (obsolete)
Comment 37 Alex Sassmannshausen 2017-05-04 10:33:41 UTC Comment hidden (obsolete)
Comment 38 Alex Sassmannshausen 2017-05-04 10:33:49 UTC Comment hidden (obsolete)
Comment 39 Alex Sassmannshausen 2017-05-04 10:35:09 UTC Comment hidden (obsolete)
Comment 40 Magnus Enger 2017-05-04 13:19:05 UTC Comment hidden (obsolete)
Comment 41 Alex Sassmannshausen 2017-05-04 14:27:43 UTC Comment hidden (obsolete)
Comment 42 Alex Sassmannshausen 2017-05-04 14:28:31 UTC Comment hidden (obsolete)
Comment 43 Alex Sassmannshausen 2017-05-05 15:06:56 UTC Comment hidden (obsolete)
Comment 44 Alex Sassmannshausen 2017-05-05 15:25:57 UTC Comment hidden (obsolete)
Comment 45 Alex Sassmannshausen 2017-05-05 15:27:41 UTC
Hi Daphne

(In reply to Daphne Hoolahan from comment #25)
> I will also add a block with library details to the generic email template. 
> Probably just after the "Kind regards".  What do you think?
> 
> That's an excellent idea.

I've now added logic for adding this kind of branch details block.

Please feel free to test and let me know what you think!

Alex
Comment 46 Alex Sassmannshausen 2017-05-10 15:41:56 UTC Comment hidden (obsolete)
Comment 47 Magnus Enger 2017-05-10 17:42:30 UTC Comment hidden (obsolete)
Comment 48 Alex Sassmannshausen 2017-05-12 10:23:31 UTC Comment hidden (obsolete)
Comment 49 Magnus Enger 2017-05-15 13:07:34 UTC Comment hidden (obsolete)
Comment 50 Alex Sassmannshausen 2017-05-15 13:32:28 UTC Comment hidden (obsolete)
Comment 51 Alex Sassmannshausen 2017-05-15 13:33:44 UTC Comment hidden (obsolete)
Comment 52 Alex Sassmannshausen 2017-05-15 14:13:51 UTC Comment hidden (obsolete)
Comment 53 Magnus Enger 2017-06-07 11:00:50 UTC
Comment on attachment 63084 [details] [review]
Bug 7317: ILLModule

Review of attachment 63084 [details] [review]:
-----------------------------------------------------------------

::: koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
@@ +156,5 @@
> +    //]]>
> +</script>
> +</head>
> +
> +<body id="acq_suggestion" class="acq">

The id on the body tag should probably not be acq_suggestion?
Comment 54 Hugo Agud 2017-06-15 09:20:04 UTC Comment hidden (obsolete)
Comment 55 Magnus Enger 2017-06-15 11:08:08 UTC Comment hidden (obsolete)
Comment 56 Alex Sassmannshausen 2017-08-03 15:36:59 UTC Comment hidden (obsolete)
Comment 57 Alex Sassmannshausen 2017-08-03 15:38:53 UTC Comment hidden (obsolete)
Comment 58 Alex Sassmannshausen 2017-08-03 15:39:02 UTC Comment hidden (obsolete)
Comment 59 Alex Sassmannshausen 2017-08-03 15:39:08 UTC Comment hidden (obsolete)
Comment 60 Alex Sassmannshausen 2017-08-03 15:39:15 UTC Comment hidden (obsolete)
Comment 61 Alex Sassmannshausen 2017-08-03 15:39:21 UTC Comment hidden (obsolete)
Comment 62 Alex Sassmannshausen 2017-08-03 15:39:28 UTC Comment hidden (obsolete)
Comment 63 Alex Sassmannshausen 2017-08-03 15:39:35 UTC Comment hidden (obsolete)
Comment 64 Alex Sassmannshausen 2017-08-21 10:10:08 UTC Comment hidden (obsolete)
Comment 65 Jonathan Druart 2017-09-20 15:53:19 UTC Comment hidden (obsolete)
Comment 66 Jonathan Druart 2017-09-20 15:53:30 UTC Comment hidden (obsolete)
Comment 67 Jonathan Druart 2017-09-20 15:53:40 UTC Comment hidden (obsolete)
Comment 68 Jonathan Druart 2017-09-20 15:53:46 UTC Comment hidden (obsolete)
Comment 69 Jonathan Druart 2017-09-20 15:53:53 UTC Comment hidden (obsolete)
Comment 70 Jonathan Druart 2017-09-20 15:53:59 UTC Comment hidden (obsolete)
Comment 71 Jonathan Druart 2017-09-20 15:54:07 UTC Comment hidden (obsolete)
Comment 72 Jonathan Druart 2017-09-20 15:54:13 UTC Comment hidden (obsolete)
Comment 73 Jonathan Druart 2017-09-20 15:54:20 UTC Comment hidden (obsolete)
Comment 74 Jonathan Druart 2017-09-20 15:54:27 UTC Comment hidden (obsolete)
Comment 75 Jonathan Druart 2017-09-20 15:54:34 UTC Comment hidden (obsolete)
Comment 76 Jonathan Druart 2017-09-20 15:54:40 UTC Comment hidden (obsolete)
Comment 77 Jonathan Druart 2017-09-20 15:54:47 UTC Comment hidden (obsolete)
Comment 78 Jonathan Druart 2017-09-20 15:54:53 UTC Comment hidden (obsolete)
Comment 79 Jonathan Druart 2017-09-20 15:54:59 UTC Comment hidden (obsolete)
Comment 80 Jonathan Druart 2017-09-20 15:55:06 UTC Comment hidden (obsolete)
Comment 81 Jonathan Druart 2017-09-20 15:55:12 UTC Comment hidden (obsolete)
Comment 82 Jonathan Druart 2017-09-20 15:55:19 UTC Comment hidden (obsolete)
Comment 83 Jonathan Druart 2017-09-20 15:55:25 UTC Comment hidden (obsolete)
Comment 84 Jonathan Druart 2017-09-20 15:55:32 UTC Comment hidden (obsolete)
Comment 85 Jonathan Druart 2017-09-20 15:55:38 UTC Comment hidden (obsolete)
Comment 86 Jonathan Druart 2017-09-20 15:55:44 UTC Comment hidden (obsolete)
Comment 87 Jonathan Druart 2017-09-20 15:55:52 UTC Comment hidden (obsolete)
Comment 88 Jonathan Druart 2017-09-20 15:57:35 UTC Comment hidden (obsolete)
Comment 89 Magnus Enger 2017-09-21 19:27:21 UTC Comment hidden (obsolete)
Comment 90 Magnus Enger 2017-09-21 19:27:37 UTC Comment hidden (obsolete)
Comment 91 Magnus Enger 2017-09-21 19:27:51 UTC Comment hidden (obsolete)
Comment 92 Magnus Enger 2017-09-21 19:28:04 UTC Comment hidden (obsolete)
Comment 93 Magnus Enger 2017-09-21 19:28:15 UTC Comment hidden (obsolete)
Comment 94 Magnus Enger 2017-09-21 19:28:25 UTC Comment hidden (obsolete)
Comment 95 Magnus Enger 2017-09-21 19:28:35 UTC Comment hidden (obsolete)
Comment 96 Magnus Enger 2017-09-21 19:28:46 UTC Comment hidden (obsolete)
Comment 97 Magnus Enger 2017-09-21 19:28:56 UTC Comment hidden (obsolete)
Comment 98 Magnus Enger 2017-09-21 19:29:07 UTC Comment hidden (obsolete)
Comment 99 Magnus Enger 2017-09-21 19:29:17 UTC Comment hidden (obsolete)
Comment 100 Magnus Enger 2017-09-21 19:29:27 UTC Comment hidden (obsolete)
Comment 101 Magnus Enger 2017-09-21 19:29:36 UTC Comment hidden (obsolete)
Comment 102 Magnus Enger 2017-09-21 19:29:46 UTC Comment hidden (obsolete)
Comment 103 Magnus Enger 2017-09-21 19:29:56 UTC Comment hidden (obsolete)
Comment 104 Magnus Enger 2017-09-21 19:30:09 UTC Comment hidden (obsolete)
Comment 105 Magnus Enger 2017-09-21 19:30:21 UTC Comment hidden (obsolete)
Comment 106 Magnus Enger 2017-09-21 19:30:30 UTC Comment hidden (obsolete)
Comment 107 Magnus Enger 2017-09-21 19:30:40 UTC Comment hidden (obsolete)
Comment 108 Magnus Enger 2017-09-21 19:30:50 UTC Comment hidden (obsolete)
Comment 109 Magnus Enger 2017-09-21 19:30:59 UTC Comment hidden (obsolete)
Comment 110 Magnus Enger 2017-09-21 19:31:10 UTC Comment hidden (obsolete)
Comment 111 Magnus Enger 2017-09-21 19:31:22 UTC Comment hidden (obsolete)
Comment 112 Benjamin Rokseth 2017-09-28 13:48:04 UTC
Have tested on a live system now, with the NNCIPP backend module from Magnus, and after some fixes we can confirm the entire processof ordering from a remote library, receiving, lending out and returning the item has been successful.

Have yet to test the opposite direction, but will have a second look on this module patch and hopefully sign off when we're done testing
Comment 113 Kyle M Hall 2017-09-29 18:01:20 UTC Comment hidden (obsolete)
Comment 114 Magnus Enger 2017-09-29 18:53:40 UTC Comment hidden (obsolete)
Comment 115 Alex Sassmannshausen 2017-10-16 16:28:21 UTC Comment hidden (obsolete)
Comment 116 Alex Sassmannshausen 2017-10-16 16:28:45 UTC Comment hidden (obsolete)
Comment 117 Magnus Enger 2017-10-20 08:38:27 UTC
I added a page to the wiki where we can document backends that have been/are being implemented:
https://wiki.koha-community.org/wiki/ILL_backends
Comment 118 Alex Sassmannshausen 2017-10-20 17:09:43 UTC Comment hidden (obsolete)
Comment 119 Alex Sassmannshausen 2017-10-23 13:40:22 UTC Comment hidden (obsolete)
Comment 120 Alex Sassmannshausen 2017-10-23 14:17:00 UTC Comment hidden (obsolete)
Comment 121 Alex Sassmannshausen 2017-10-23 14:17:15 UTC Comment hidden (obsolete)
Comment 122 Alex Sassmannshausen 2017-10-23 14:17:26 UTC Comment hidden (obsolete)
Comment 123 Alex Sassmannshausen 2017-10-23 14:17:37 UTC Comment hidden (obsolete)
Comment 124 Alex Sassmannshausen 2017-10-23 15:20:35 UTC
Created attachment 68418 [details]
ILL overview & light documentation

Please see attached text file for an initial attempt at user-friendly documentation of the module.

It should give an overview of this module's impact on Koha itself, of the architecture of the module, and of the notion of 'backends'.

Cheers,

Alex
Comment 125 Magnus Enger 2017-10-23 17:41:48 UTC Comment hidden (obsolete)
Comment 126 Tomás Cohen Arazi 2017-10-23 19:07:54 UTC Comment hidden (obsolete)
Comment 127 Tomás Cohen Arazi 2017-10-23 19:08:03 UTC Comment hidden (obsolete)
Comment 128 Tomás Cohen Arazi 2017-10-24 16:54:09 UTC Comment hidden (obsolete)
Comment 129 Tomás Cohen Arazi 2017-10-24 16:54:21 UTC Comment hidden (obsolete)
Comment 130 Tomás Cohen Arazi 2017-10-24 16:54:30 UTC Comment hidden (obsolete)
Comment 131 Tomás Cohen Arazi 2017-10-24 17:16:54 UTC Comment hidden (obsolete)
Comment 132 Magnus Enger 2017-10-25 09:28:24 UTC Comment hidden (obsolete)
Comment 133 Magnus Enger 2017-10-25 09:29:42 UTC Comment hidden (obsolete)
Comment 134 Magnus Enger 2017-10-25 09:29:56 UTC Comment hidden (obsolete)
Comment 135 Magnus Enger 2017-10-25 09:30:08 UTC Comment hidden (obsolete)
Comment 136 Magnus Enger 2017-10-25 09:30:19 UTC Comment hidden (obsolete)
Comment 137 Magnus Enger 2017-10-25 09:30:31 UTC Comment hidden (obsolete)
Comment 138 Magnus Enger 2017-10-25 09:30:39 UTC Comment hidden (obsolete)
Comment 139 Magnus Enger 2017-10-25 09:30:51 UTC Comment hidden (obsolete)
Comment 140 Magnus Enger 2017-10-25 09:31:03 UTC Comment hidden (obsolete)
Comment 141 Magnus Enger 2017-10-25 09:56:03 UTC Comment hidden (obsolete)
Comment 142 Tomás Cohen Arazi 2017-10-25 18:35:24 UTC Comment hidden (obsolete)
Comment 143 Tomás Cohen Arazi 2017-10-25 18:42:31 UTC Comment hidden (obsolete)
Comment 144 Tomás Cohen Arazi 2017-10-25 18:42:50 UTC Comment hidden (obsolete)
Comment 145 Tomás Cohen Arazi 2017-10-25 18:43:01 UTC Comment hidden (obsolete)
Comment 146 Tomás Cohen Arazi 2017-10-25 18:43:10 UTC Comment hidden (obsolete)
Comment 147 Tomás Cohen Arazi 2017-10-25 18:43:17 UTC Comment hidden (obsolete)
Comment 148 Tomás Cohen Arazi 2017-10-25 18:43:28 UTC Comment hidden (obsolete)
Comment 149 Tomás Cohen Arazi 2017-10-25 18:43:41 UTC Comment hidden (obsolete)
Comment 150 Tomás Cohen Arazi 2017-10-25 18:43:56 UTC Comment hidden (obsolete)
Comment 151 Tomás Cohen Arazi 2017-10-25 18:44:08 UTC Comment hidden (obsolete)
Comment 152 Tomás Cohen Arazi 2017-10-25 18:44:18 UTC Comment hidden (obsolete)
Comment 153 Benjamin Rokseth 2017-10-25 20:34:48 UTC
Created attachment 68558 [details] [review]
Bug 7317: Add SQL templates & sysprefs.

* installer/data/mysql/atomicupdate/ill_tables.sql: New file.
* installer/data/mysql/kohastructure.sql: Add tables.
* installer/data/mysql/sysprefs.sql: Add sysprefs.
* installer/data/mysql/userflags.sql: Add userflags.
* koha-tmpl/intranet-tmpl/prog/en/modules/admin/preferences/admin.pref:
  Add sysprefs to UI.

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>
Comment 154 Benjamin Rokseth 2017-10-25 20:34:58 UTC
Created attachment 68559 [details] [review]
Bug 7317: DO NOT PUSH: Updated Schema Files.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 155 Benjamin Rokseth 2017-10-25 20:35:10 UTC
Created attachment 68560 [details] [review]
Bug 7317: Interlibrary loans framework for Koha.

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>
Comment 156 Benjamin Rokseth 2017-10-25 20:35:22 UTC
Created attachment 68561 [details] [review]
Bug 7317: DO NOT PUSH: Update OPAC CSS

Signed-off-by: Magnus Enger <magnus@libriotech.no>
It's impossible to give a test plan for this...
I have implemented a "plugin" for Norwegian ILL (based on NCIP):
https://github.com/Libriotech/koha-illbackend-nncipp
Several issues have been discovered while writing the plugin, and
Alex has fixed them along the way. Currently I do not know about
any funcional issues, and I am happy to sign off. I think this
is one of those things we need to get into Koha so we can start
using it and find any remaining bugs.

One non-functional thing that needs to be looked at are the tests.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 157 Benjamin Rokseth 2017-10-25 20:35:34 UTC
Created attachment 68562 [details] [review]
Bug 7317: (QA folowup) 404 if module is disabled (intranet)

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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>
Comment 158 Benjamin Rokseth 2017-10-25 20:35:43 UTC
Created attachment 68563 [details] [review]
Bug 7317: Overload Koha::Illrequest::TO_JSON

This patch implements an overloaded TO_JSON method, that introduces
the option to (selectively) embed information required on the REST
api.

Tests are included for the new method.

To test:
- Apply this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/Illrequests.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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>
Comment 159 Benjamin Rokseth 2017-10-25 20:35:54 UTC
Created attachment 68564 [details] [review]
Bug 7317: (followup) Migrate endpoint to OpenAPI

This patch moves the current endpoint implementation from Swagger2 to
the OpenAPI plugin.

It also takes advantage of the overloaded Koha::Illrequest::TO_JSON method
which has now the option to embed what's needed for the REST api.

The path spec is adjusted to fit OpenAPI, and some minor fixes are
applied:
- Missing 'metadata' query param
- 'ill' permissions should be required instead of 'borrowers'
- Full test coverage

To test:
- Apply  this patch
- Run:
  $ kshell
 k$ prove t/db_dependent/api/v1/illrequests.t
=> SUCCESS: Tests pass!
- Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

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>
Comment 160 Benjamin Rokseth 2017-10-25 20:36:03 UTC
Created attachment 68565 [details] [review]
Bug 7317: Adjust packages koha-conf.xml

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Original sign off message:
It's impossible to give a test plan for this...
I have implemented a "plugin" for Norwegian ILL (based on NCIP):
https://github.com/Libriotech/koha-illbackend-nncipp
Several issues have been discovered while writing the plugin, and
Alex has fixed them along the way. Currently I do not know about
any funcional issues, and I am happy to sign off.

Update:
I have applied the new patchset and done some exploratory testing,
using the Dummy backend. Everything seems to be fine after the
patches were re-arranged and with the followups from Tomas.
All tests pass.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 161 Benjamin Rokseth 2017-10-25 20:36:13 UTC
Created attachment 68566 [details] [review]
Bug 7317: (followup) Fix id and class on body tag

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>
Comment 162 Benjamin Rokseth 2017-10-25 20:36:23 UTC
Created attachment 68567 [details] [review]
Bug 7317: Rewrite atomicupdate file

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Comment 163 Jonathan Druart 2017-10-30 14:59:10 UTC
Please fix QA script failures.
Comment 164 Jonathan Druart 2017-10-30 15:15:28 UTC
1/ 
+TODO:
+
+- Anything invoking the ->status method; annotated with:
+  + # Old use of ->status !

What do it mean?

2/ I will not make you rewrite everything, but it would have been better to stay consistent and not mix snake_case and camelCase for method names.

First steps with the GUI:
3/ This is what I see when I enable the module:
https://screenshots.firefox.com/z9dNoRL91NQ5F3cD/pro.kohadev.org
Looks wrong.

4/ Hit /cgi-bin/koha/ill/ill-requests.pl and got:
Loading failed for the <script> with source “http://pro.kohadev.org/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.tablesorter.min.js”.  ill-requests.pl:151
Loading failed for the <script> with source “http://pro.kohadev.org/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.checkboxes.min.js”.  ill-requests.pl:152

5/ Clicked "New ILL request" button and got:
Can't locate Koha/Illbackends//Base.pm
Comment 165 Magnus Enger 2017-10-31 14:14:38 UTC
Created attachment 68858 [details] [review]
Bug 7317: QA followup

This fixes some of the issues reported by the QA script, but not all.
Comment 166 Tomás Cohen Arazi 2017-10-31 19:43:36 UTC
Created attachment 68874 [details] [review]
Bug 7317: Handle backend absense more gracefuly

5/ This patch is WIP, it 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 warning.

TODO: Should add a noticeable warning on the UI, or a tooltip on the button.
TODO: add a couple tests for the exception

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.

3/ I know it doesn't look right, but am not sure how to display. This is the usual empty
datatable display we have everywhere in Koha.

2/ We could fix it with a proper IDE, I use Komodo and refactoring code is pretty straigh-forward.
Worth a separate bug report.

1/ That TODO is misleading. It refers to their own migration from a different design. I checked this
code and cannot find occurences of ->status with that annotation. I think this relates to backends code.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 167 Magnus Enger 2017-10-31 20:04:26 UTC Comment hidden (obsolete)
Comment 168 Magnus Enger 2017-11-01 10:53:14 UTC
Created attachment 68877 [details] [review]
Bug 7317: QA followup

This fixes some of the issues reported by the QA script, but not all.
Comment 169 Magnus Enger 2017-11-01 10:53:21 UTC
Created attachment 68878 [details] [review]
Bug 7317: Handle backend absense more gracefuly

5/ This patch is WIP, it 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 warning.

TODO: Should add a noticeable warning on the UI, or a tooltip on the button.
TODO: add a couple tests for the exception

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.

3/ I know it doesn't look right, but am not sure how to display. This is the usual empty
datatable display we have everywhere in Koha.

2/ We could fix it with a proper IDE, I use Komodo and refactoring code is pretty straigh-forward.
Worth a separate bug report.

1/ That TODO is misleading. It refers to their own migration from a different design. I checked this
code and cannot find occurences of ->status with that annotation. I think this relates to backends code.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 170 Magnus Enger 2017-11-01 11:03:32 UTC
Trying to summarize the status of this bug now. 

*** QA script

** POD coverage

Some FAILS are "POD coverage was greater before, try perl -MPod::Coverage=PackageName -e666". It sounds like these can be ignored. 

** Atomicupdate

 FAIL	installer/data/mysql/atomicupdate/ill_tables.sql
   FAIL	  git manipulation
		The file has been added and deleted in the same patchset

Alex provided ill_tables.sql. Tomas rewrote it as bug_7317_ill.perl. Not sure there is anything to fix here? 

** tt_valid

 FAIL	koha-tmpl/intranet-tmpl/prog/en/modules/ill/ill-requests.tt
   OK	  forbidden patterns
   OK	  git manipulation
   OK	  spelling
   FAIL	  tt_valid
		lines 665
   FAIL	  valid_template
		: not found

TT is used to set some HTML attributes. Not sure if this can be fixed. Tomas? 

*** (In reply to Jonathan Druart from comment #164)

> 1/ 
> +TODO:
> +
> +- Anything invoking the ->status method; annotated with:
> +  + # Old use of ->status !
> 
> What do it mean?

OK, according to Tomas.
 
> 2/ I will not make you rewrite everything, but it would have been better to
> stay consistent and not mix snake_case and camelCase for method names.

OK

> First steps with the GUI:
> 3/ This is what I see when I enable the module:
> https://screenshots.firefox.com/z9dNoRL91NQ5F3cD/pro.kohadev.org
> Looks wrong.

Not sure if this can be fixed?

> 4/ Hit /cgi-bin/koha/ill/ill-requests.pl and got:
> Loading failed for the <script> with source
> “http://pro.kohadev.org/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.
> tablesorter.min.js”.  ill-requests.pl:151
> Loading failed for the <script> with source
> “http://pro.kohadev.org/intranet-tmpl/prog/en/lib/jquery/plugins/jquery.
> checkboxes.min.js”.  ill-requests.pl:152

OK. Fixed by Tomas

> 5/ Clicked "New ILL request" button and got:
> Can't locate Koha/Illbackends//Base.pm

OK. Fixed by Tomas. But it sounds like Tomas will be working a bit more on related things.
Comment 171 Tomás Cohen Arazi 2017-11-01 19:31:41 UTC Comment hidden (obsolete)
Comment 172 Tomás Cohen Arazi 2017-11-01 19:32:00 UTC Comment hidden (obsolete)
Comment 173 Tomás Cohen Arazi 2017-11-03 16:50:09 UTC Comment hidden (obsolete)
Comment 174 Tomás Cohen Arazi 2017-11-03 16:50:22 UTC Comment hidden (obsolete)
Comment 175 Tomás Cohen Arazi 2017-11-03 18:29:22 UTC Comment hidden (obsolete)
Comment 176 Tomás Cohen Arazi 2017-11-03 19:06:49 UTC Comment hidden (obsolete)
Comment 177 Tomás Cohen Arazi 2017-11-03 19:07:01 UTC Comment hidden (obsolete)
Comment 178 Tomás Cohen Arazi 2017-11-03 19:07:12 UTC Comment hidden (obsolete)
Comment 179 Tomás Cohen Arazi 2017-11-03 19:12:10 UTC
Created attachment 68931 [details] [review]
Bug 7317: Handle backend absense more gracefuly

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>
Comment 180 Tomás Cohen Arazi 2017-11-03 19:12:23 UTC
Created attachment 68932 [details] [review]
Bug 7317: (QA followup) Get rid of warnings from the tests

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 181 Tomás Cohen Arazi 2017-11-03 19:12:35 UTC
Created attachment 68933 [details] [review]
Bug 7317: opac.css update

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 182 Jonathan Druart 2017-11-03 19:55:12 UTC
1/ Error handling on borrowernumber when creating or updating a request (backend issue?)
2/ Capitalization problems, deprecated words (Branch, borrowers), etc
3/ Use dateformat to display dates
4/ ill/ill-requests.pl?method=generic_confirm&illrequest_id=3
click "send email"
Software error to handle correctly:
No target email addresses found. Either select at least one partner or check your ILL partner library records. at /home/vagrant/kohaclone/Koha/Illrequest.pm line 896.

5/ Same for "Your branch has no email address. Please set it. at /home/vagrant/kohaclone/Koha/Illrequest.pm line 901."

6/ Should not we use message_queue instead of sendmail directly?

7/ Maybe we should add a warning to the about page if the partner_code defined in the config does not match a category_code (?)

(not finished)
Comment 183 Tomás Cohen Arazi 2017-11-06 17:34:27 UTC
Created attachment 68988 [details] [review]
Bug 7317: Add partner_code checks to abot page

This patch adds checks to the configured (or fallback) partner_code.
It produces two different warnings:

- Configured partner_code doesn't match an existing patron category
code.
- partner_code is not defined, a fallback to ILLLIBS is done

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 184 Tomás Cohen Arazi 2017-11-06 17:34:45 UTC
Created attachment 68989 [details] [review]
Bug 7317: (followup) Make tests independent of data on the DB

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 185 Tomás Cohen Arazi 2017-11-06 17:37:46 UTC
(In reply to Jonathan Druart from comment #182)
> 1/ Error handling on borrowernumber when creating or updating a request
> (backend issue?)

This is definitely a backend issue. The Dummy backend does no check, it just passes the borrowernumber from the UI to the Koha::Illrequest->store method with no error catching. The FreeForm plugin does some checks before the ->store, though. Out of the scope of the bug anyway.

> 6/ Should not we use message_queue instead of sendmail directly?

There's a bug filed for this: bug 18588.

> 7/ Maybe we should add a warning to the about page if the partner_code
> defined in the config does not match a category_code (?)

Done in one of the recent followups.
Comment 186 Tomás Cohen Arazi 2017-11-07 15:06:08 UTC
Created attachment 68999 [details] [review]
Bug 7317: (QA followup) ILL sysprefs belong to circulation section

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 187 Tomás Cohen Arazi 2017-11-07 15:06:15 UTC
Created attachment 69000 [details] [review]
Bug 7317: Handle missing email addresses gracefuly

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 188 Tomás Cohen Arazi 2017-11-07 19:13:12 UTC
Created attachment 69006 [details] [review]
Bug 7317: (QA followup) Fix capitalization and terminology issues

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>
Comment 189 Jonathan Druart 2017-11-07 20:00:50 UTC
Created attachment 69008 [details] [review]
Bug 7317: Add missing 'use Encode' statement
Comment 190 Jonathan Druart 2017-11-07 20:00:58 UTC
Created attachment 69009 [details] [review]
Bug 7317: (QA followup 2) Fix capitalization and terminology issues
Comment 191 Jonathan Druart 2017-11-07 20:01:05 UTC
Created attachment 69012 [details] [review]
Bug 7317: use preventDefault to prevent the scrollbar to move to the top
Comment 192 Jonathan Druart 2017-11-07 20:44:19 UTC
Created attachment 69014 [details] [review]
Bug 7317: Fix more translation issues
Comment 193 Jonathan Druart 2017-11-07 20:45:28 UTC
Some more:

1. This looks wrong
260     here_link  => "/cgi-bin/koha/ill/ill-requests.pl"

url should be in template I'd say

2. This is not used, you are using an ajax call
219     } elsif ( $op eq 'illlist') {
220         # Display all current ILLs                                                                   
221         my $requests = $illRequests->search();
222     
223         $template->param(                                                                            
224             requests => $requests                                                                    
225         );

3. 257     media      => [ "Book", "Article", "Journal" ],
Is this used?

4.
Config.pm
312     die "No DEFAULT_FORMATS has been defined in koha-conf.xml, but UNMEDIATEDILL is active."
313         if ( $unmediated && !$configuration->{default_formats}->{default} );

Not used yet, please remove

5. 794                 \"YEAR(placed) = YEAR(NOW())"
This looks like a mysqlism

6. The branchcode is displayed instead of the branchname in the list of ILL requests. Sounds easy to fix as we embed "branch" (must be library?)

7. api/v1/swagger/paths/*.json
  "name": "branchcode"
vs
  "name": "branch_id"
same with
  borrowernumber vs borrower_id
and
  biblionumber vs biblio_id

(to be continued...)
Comment 194 Tomás Cohen Arazi 2017-11-08 17:13:12 UTC
Comment on attachment 69009 [details] [review]
Bug 7317: (QA followup 2) Fix capitalization and terminology issues

This patch doesn't apply because the attached patches already fix this. Not sure how it happened.
Comment 195 Tomás Cohen Arazi 2017-11-08 19:24:56 UTC
(In reply to Jonathan Druart from comment #193)
> Some more:
> 
> 1. This looks wrong
> 260     here_link  => "/cgi-bin/koha/ill/ill-requests.pl"
> 
> url should be in template I'd say

The path shouldn't need to be added manually, I'd say any option is wrong, fixed anyway in an upcoming patch.

> 2. This is not used, you are using an ajax call
> 219     } elsif ( $op eq 'illlist') {
> 220         # Display all current ILLs                                      
> 
> 221         my $requests = $illRequests->search();
> 222     
> 223         $template->param(                                               
> 
> 224             requests => $requests                                       
> 
> 225         );

This is not used and is removed on an upcoming patch.

> 3. 257     media      => [ "Book", "Article", "Journal" ],
> Is this used?

This is used as a fallback in case the backends don't implement their own list. Needs to be kept (e.g. for the Dummy backend to work).

> 4.
> Config.pm
> 312     die "No DEFAULT_FORMATS has been defined in koha-conf.xml, but
> UNMEDIATEDILL is active."
> 313         if ( $unmediated &&
> !$configuration->{default_formats}->{default} );
> 
> Not used yet, please remove

Yup, removed in an upcoming patch.

> 5. 794                 \"YEAR(placed) = YEAR(NOW())"
> This looks like a mysqlism

True. But I cannot find any option to specify the same behaviour in SQL::Abstract or the DBIC docs. The very same line of code can be found on the DBIC docs as an example.

> 6. The branchcode is displayed instead of the branchname in the list of ILL
> requests. Sounds easy to fix as we embed "branch" (must be library?)

Fixed in an upcoming patch.

> 7. api/v1/swagger/paths/*.json
>   "name": "branchcode"
> vs
>   "name": "branch_id"
> same with
>   borrowernumber vs borrower_id
> and
>   biblionumber vs biblio_id

I can work on this tomorrow. Today's been enough!
Comment 196 Tomás Cohen Arazi 2017-11-08 19:33:40 UTC
Created attachment 69044 [details] [review]
Bug 7317: (QA followup) Rename 'branch' for 'library'

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 197 Tomás Cohen Arazi 2017-11-08 19:33:52 UTC
Created attachment 69045 [details] [review]
Bug 7317: (followup) Remove extra columns from backends on the main list

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>
Comment 198 Tomás Cohen Arazi 2017-11-08 19:39:16 UTC
Created attachment 69046 [details] [review]
Bug 7317: Remove traces of UnmediatedILL

This patch removes references to UnmediatedILL. This feature is introduced on
a separate bug.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 199 Tomás Cohen Arazi 2017-11-09 14:03:49 UTC
Created attachment 69056 [details] [review]
Bug 7317: (QA followup) Make query parameters consistent with other endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 200 Tomás Cohen Arazi 2017-11-09 14:50:10 UTC
Created attachment 69059 [details] [review]
Bug 7317: (QA followup) Make query parameters consistent with other endpoints

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 201 Tomás Cohen Arazi 2017-11-09 14:50:25 UTC
Created attachment 69060 [details] [review]
Bug 7317: Fix infinite loop in OPAC

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 202 Tomás Cohen Arazi 2017-11-09 14:55:25 UTC
Created attachment 69061 [details] [review]
Bug 7317: (QA followup) Remove MySQL-ism from Koha::Illrequest::_limit_counter

This trivial fix changes the query syntax to avoid MySQL-isms.

To test:
- Run:
  $ kshell
 k$ prove t/db_dependent/Illrequests.t
=> SUCCESS: Tests pass!
- Apply this patch
- Run:
 k$ prove t/db_dependent/Illrequests.t
=> SUCCESS: Tests still pass!

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 203 Jonathan Druart 2017-11-09 15:22:32 UTC
Created attachment 69063 [details] [review]
Bug 7317: Help translators removing TT tags from po files
Comment 204 Jonathan Druart 2017-11-09 15:25:56 UTC
Created attachment 69064 [details] [review]
Bug 7317: Help translators removing TT tags from po files
Comment 205 Jonathan Druart 2017-11-09 15:36:45 UTC
Created attachment 69065 [details] [review]
Bug 7317: Simplify column headers

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)
Comment 206 Jonathan Druart 2017-11-09 15:51:41 UTC
Created attachment 69066 [details] [review]
Bug 7317: Do not use GET for deletion requests
Comment 207 Jonathan Druart 2017-11-09 16:27:33 UTC
Created attachment 69069 [details] [review]
Bug 7317: Make fields mandatory when sending emails
Comment 208 Jonathan Druart 2017-11-09 16:27:41 UTC
Created attachment 69070 [details] [review]
Bug 7317: Display all the libraries, not only one

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`
Comment 209 Jonathan Druart 2017-11-09 16:29:16 UTC
Created attachment 69071 [details] [review]
Bug 7317: Display all the libraries, not only one

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`
Comment 210 Jonathan Druart 2017-11-09 16:33:32 UTC
Pushed to master for 17.11, thanks to everybody involved!
Comment 211 Jonathan Druart 2017-11-09 17:23:39 UTC
Created attachment 69074 [details] [review]
Bug 7317: Fix translatable-templates.t failure

Failed with
 /tmp/Lqv0BH2xC9:7979:13: invalid multibyte sequence
 msgmerge: found 1 fatal error
Comment 212 Caroline Cyr La Rose 2017-12-08 19:14:20 UTC
Is there any way to make the configuration of the ILL module more user friendly? I wish I didn't have to go in the conf file and on the server to configure it.

Here are some suggestions

- Tab in system preferences for all ILL configurations
- Make StaffRequestComments (hide/show) a sys pref
- Make ReplyDate (hide/show) a sys pref
- Make DigitalRecipient (borrower/branch) a sys pref
- Make PartnerCode (text entry, with by default ILLLIBS entered) a sys pref
- In Tools or Administration, add a page with a form to add backends with our own providers

So far, I've only been able to enable the ILLModule sys pref and create the database tables, so I don't really have any suggestions for the other parts... I will add them once I figure out the rest.
Comment 213 Katrin Fischer 2017-12-10 21:37:39 UTC
(In reply to Caroline Cyr La Rose from comment #212)
> Is there any way to make the configuration of the ILL module more user
> friendly? I wish I didn't have to go in the conf file and on the server to
> configure it.
> 
> Here are some suggestions
> 
> - Tab in system preferences for all ILL configurations
> - Make StaffRequestComments (hide/show) a sys pref
> - Make ReplyDate (hide/show) a sys pref
> - Make DigitalRecipient (borrower/branch) a sys pref
> - Make PartnerCode (text entry, with by default ILLLIBS entered) a sys pref
> - In Tools or Administration, add a page with a form to add backends with
> our own providers
> 
> So far, I've only been able to enable the ILLModule sys pref and create the
> database tables, so I don't really have any suggestions for the other
> parts... I will add them once I figure out the rest.

Hi Caroline, please don't hesitate to file your enhancement ideas and suggestions as separate bugs and link them depending on this one.
Comment 214 Magnus Enger 2017-12-11 07:56:17 UTC
(In reply to Caroline Cyr La Rose from comment #212)
> - In Tools or Administration, add a page with a form to add backends with
> our own providers

There is bug 19605, which suggests making the ILL backends into regular "KPZ" plugins. I think Tomas will be working on that.
Comment 215 Caroline Cyr La Rose 2017-12-15 14:51:52 UTC
(In reply to Katrin Fischer from comment #213)
> (In reply to Caroline Cyr La Rose from comment #212)
> > Is there any way to make the configuration of the ILL module more user
> > friendly? I wish I didn't have to go in the conf file and on the server to
> > configure it.
> > 
> > Here are some suggestions
> > 
> > - Tab in system preferences for all ILL configurations
> > - Make StaffRequestComments (hide/show) a sys pref
> > - Make ReplyDate (hide/show) a sys pref
> > - Make DigitalRecipient (borrower/branch) a sys pref
> > - Make PartnerCode (text entry, with by default ILLLIBS entered) a sys pref
> > - In Tools or Administration, add a page with a form to add backends with
> > our own providers
> > 
> > So far, I've only been able to enable the ILLModule sys pref and create the
> > database tables, so I don't really have any suggestions for the other
> > parts... I will add them once I figure out the rest.
> 
> Hi Caroline, please don't hesitate to file your enhancement ideas and
> suggestions as separate bugs and link them depending on this one.

Done! Bug 19822
Comment 216 Tomás Cohen Arazi 2017-12-15 15:13:35 UTC
(In reply to Magnus Enger from comment #214)
> (In reply to Caroline Cyr La Rose from comment #212)
> > - In Tools or Administration, add a page with a form to add backends with
> > our own providers
> 
> There is bug 19605, which suggests making the ILL backends into regular
> "KPZ" plugins. I think Tomas will be working on that.

I'm not working on this in a short term, so anyone willing to pick that please do.

Having ILLbackends be plugins will help removing the configuration from koha-conf.xml too.
Comment 217 Katrin Fischer 2019-04-28 10:44:46 UTC
*** Bug 9852 has been marked as a duplicate of this bug. ***
Comment 218 David Cook 2020-05-05 06:53:41 UTC
Comment on attachment 68560 [details] [review]
Bug 7317: Interlibrary loans framework for Koha.

Review of attachment 68560 [details] [review]:
-----------------------------------------------------------------

::: Koha/Illrequest.pm
@@ +838,5 @@
> +    } elsif ( 'draft' eq $params->{stage} ) {
> +        # Create the to header
> +        my $to = $params->{partners};
> +        $to =~ s/^\x00//;       # Strip leading NULLs
> +        $to =~ s/\x00/; /;      # Replace others with '; '

Why would you need to replace a null byte with a semicolon (;)? 

It seems to me that email addresses should be separated by a comma?
Comment 219 David Cook 2020-05-05 06:58:48 UTC
(In reply to David Cook from comment #218)
> Why would you need to replace a null byte with a semicolon (;)? 
> 
> It seems to me that email addresses should be separated by a comma?

Actually, looking at https://metacpan.org/release/Mail-Sendmail/source/lib/Mail/Sendmail.pm#L334, it seems that Mail::Sendmail doesn't care about separators. It just iterates through a string looking for valid email addresses... yikes...