]> git.koha-community.org Git - koha.git/log
koha.git
7 years agoBug 18275: Do not rely on CGI param userid to log a user in if auth is not required
Jonathan Druart [Wed, 15 Mar 2017 17:52:49 +0000 (14:52 -0300)]
Bug 18275: Do not rely on CGI param userid to log a user in if auth is not required

From opac/opac-memberentry.pl, authnotrequired is set.
That means a patron can access the page without being logged in. It is
used on this page for the self registration feature.

From C4::Auth::get_template_and_user, we have
  $userid = $q_userid;
$q_userid is previously set to the 'userid' CGI param.

We end up here if authonotrequired is set AND CGISESSID does not exist.

Test plan:
- Run:
  $ prove t/db_dependent/Auth.t
=> FAIL: Regression test for checkauth fails
- Apply this patch
- Run:
  $ prove t/db_dependent/Auth.t
=> SUCCESS: Tests pass.
- Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18275: Regression test
Tomas Cohen Arazi [Thu, 16 Mar 2017 12:23:24 +0000 (09:23 -0300)]
Bug 18275: Regression test

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 7143: Replace Santallan by Santellan
Marcel de Rooy [Thu, 19 Jan 2017 14:43:38 +0000 (15:43 +0100)]
Bug 7143: Replace Santallan by Santellan

Missed one typo in about.tt.
Can go along with the next patch here.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18215: Resolve warning on $tls in Database.pm
Marcel de Rooy [Mon, 6 Mar 2017 09:39:18 +0000 (10:39 +0100)]
Bug 18215: Resolve warning on $tls in Database.pm

Resolve this warning:
updatedatabase.pl: Use of uninitialized value $tls in string eq at /usr/share/koha/masterclone/Koha/Database.pm line 63.

Test plan:
[1] Check if you do not see the warning anymore.

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Warning disappears after applying the patch.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16202: GetIssuingCharges should return a collectable amount
Colin Campbell [Tue, 5 Apr 2016 11:56:24 +0000 (12:56 +0100)]
Bug 16202: GetIssuingCharges should return a collectable amount

Charges should not include elements less than a penny/cent
they are not displayed but can be saved to the database
causing "odd" behaviour down the line

Make the routine round the resultant charge to nearest cent,
so consistent values are returned.
Removed the one case where it was rounded post call.
Although the main danger is values generated by the discount
calculation apply the rounding to all returned charges in case
the item charge is defined using the 3rd or 4th decimal
places.

NOTE: prove -v t/db_dependent/Circulation.t triggers the change.
      Though, all the returned amounts are 0.00 only.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18135: Show alert if no authorities selected in batch record deletion
Aleisha Amohia [Thu, 16 Feb 2017 22:19:59 +0000 (22:19 +0000)]
Bug 18135: Show alert if no authorities selected in batch record deletion

To test:
1) Go to Tools -> Batch record deletion
2) Select Authorities as record type, put in an auth id, click Continue
3) Unselect the authorities and click Delete selected records
4) Confirm an alert shows up
5) Clear the alert and confirm deleting the authority still works as
normal

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16530 - DBRev 16.12.00.013
Kyle M Hall [Fri, 3 Mar 2017 18:36:55 +0000 (18:36 +0000)]
Bug 16530 - DBRev 16.12.00.013

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16530: Add a new method to the Branches TT Plugin to avoid c/p
Jonathan Druart [Wed, 1 Feb 2017 16:54:43 +0000 (17:54 +0100)]
Bug 16530: Add a new method to the Branches TT Plugin to avoid c/p

We should do the same for the other check (FA exists), but cannot find a
good and quick way to implement.

Doing it this way will avoid regression later, we will no have to check
if the variable is correctly passed to the template.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16530: Adding a circ sidebar navigation menu and circSidebar syspref to activate...
Aleisha [Tue, 17 May 2016 04:35:46 +0000 (04:35 +0000)]
Bug 16530: Adding a circ sidebar navigation menu and circSidebar syspref to activate/deactivate

Edit: Fast cataloging link should now show on all pages, removed offline circulation links
Edit 2: Creates the syspref to activate or deactivate the sidebar
Edit 3: Fixing merge conflicts, using Koha::BiblioFrameworks to find FA
framework and adding sidebar to on-site checkouts

This adds a sidebar to
circ/branchoverdues.tt
circ/circulation.tt (I also fixed up some of the indentation in this file to make it easier to see where the divs started and ended)
circ/renew.tt
circ/returns.tt
circ/selectbranchprinter.tt
circ/transferstoreceive.tt
circ/view_holdsqueue.tt
circ/waitingreserves.tt
circ/branchtransfers.tt
circ/on-site_checkouts.tt

To test:
1) Confirm syspref CircSidebar is activated
2) Go to all of the above pages and confirm the sidebar menu shows up
3) Confirm fast cataloguing link and transfer link are there
4) Trigger any error messages you can possibly think of (i.e. on renew.pl: barcode does not exist). Confirm that this does not mess up the layout of the page
5) Go to a user account page, Check out tab. (Since this is a circ/circulation.pl page). Ensure the circ nav sidebar doesn't show up (confirm it looks as it usually does)
6) Deactivate circSidebar
7) Confirm pages all look normal

Sponsored-by: Catalyst IT
Signed-off-by: Jan Kissig <jkissig@th-wildau.de>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 15427 : Enable TLS support for MySQL
Dimitris Kamenopoulos [Sun, 27 Dec 2015 03:01:43 +0000 (16:01 +1300)]
Bug 15427 : Enable TLS support for MySQL

In summary, changes are:
1) If you have chosen MySQL, Makefile.PL will ask you if you want TLS (default:
"no"), and then the locations for CA cert, client cert and client key
(reasonable defaults are provided). Settings <tls>, <ca>, <cert> and <key> are
added in koha-conf.xml
2) If <tls>yes</tls> in koha-conf.xml, the installer and database connection
scripts add the TLS options in both DBI connection strings and mysql command
line

To test
1/ Apply patch
2/ Check everything still works and db connections are the same as before
3/ Either run Makefile.PL and step through the options or edit your koha-conf.xml to
enable TLS
4/ Check db connections are still working

Patch provided to me by Dimitris Kamenopoulos and I reformatted it into a git patch,
any errors are probably mine

Signed-off-by: Mirko Tietgen <mirko@abunchofthings.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14764 - DBRev 16.12.00.012
Kyle M Hall [Fri, 3 Mar 2017 18:30:01 +0000 (18:30 +0000)]
Bug 14764 - DBRev 16.12.00.012

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14764 - [followup] Addressing QA concerns
Liz Rea [Thu, 2 Feb 2017 21:18:51 +0000 (10:18 +1300)]
Bug 14764 - [followup] Addressing QA concerns

* removed Policy:
* put the syspref in alpha order with the others
* added els to the if.

Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14764: OPAC news selector
Liz Rea [Wed, 2 Nov 2016 21:18:45 +0000 (10:18 +1300)]
Bug 14764: OPAC news selector

Test plan:

1) Add more than one library, at least two in total is needed. Make note
   of which is your current user's home branch.
2) Go to Tools -> News and add one global ("All") news item and one
   specific for each branch.
3) Apply the patches.
4) Perform database upgrades when you log in.
5) Go to OPAC main, it should work as before: Showing global news as
   well as the news for your home branch once logged in.
6) Go to System Preferences -> OPAC and set 'OPACNewsBranchSelect' to
   yes.
7) Go to OPAC main, you should see the global news item plus any items
   for your home branch for the current user if logged in.
8) Above the news, you will see a dropdown that lists the branches.
   Select one, and click "Change library."
9) You should now see global news and the news for the selected branch,
   regardless of whether logged in or not.

Sponsored-By: Halland County Library
Fixes QA comments:

* Keep the current selected branch selected

* Change OPACNewsBranchSelect -> OPACNewsLibrarySelect

* fix 'system wide only' view for logged in user. We still show system
  wide even when a library is selected.

* Removed the "change library" link

* Added a label to the field.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Zeno Tajoli <z.tajoli@cineca.it>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 10978: Redirect to basket list of a supplier after deleting a basket
Aleisha Amohia [Wed, 15 Feb 2017 00:30:57 +0000 (00:30 +0000)]
Bug 10978: Redirect to basket list of a supplier after deleting a basket

This patch redirects to the vendor's list of baskets after deleting a
basket, fixes breadcrumbs after deletion and also hides the toolbar
actions after deletion (seeing as you can't edit/export etc a basket
that no longer exists).

To test:
1) Go to Acquisitions -> find a vendor -> view a basket or create a new
basket
2) Delete the basket. Notice you are taken to a list of all vendors and
baskets
3) Apply patch and do step 1 again
4) Delete the basket. Notice appropriate breadcrumbs, no toolbar, and
confirm link to return to baskets for the vendor works.

Sponsored-by: Catalyst IT
Followed test plan, works as expected (links to vendor's baskets and all baskets)
Signed-off-by: Marc Véron <veron@veron.ch>
Re-tested. Wording of buttons "Show baskets for vendor..." and "Show all
active baskets" makes sense.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 7663: Fix bad merge conflict with bug 15584
Jonathan Druart [Fri, 24 Feb 2017 07:46:19 +0000 (07:46 +0000)]
Bug 7663: Fix bad merge conflict with bug 15584

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 7663 (QA Followup)
Nick Clemens [Fri, 24 Feb 2017 01:29:22 +0000 (01:29 +0000)]
Bug 7663 (QA Followup)

    Ignore empty entries

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 7663 - batch add barcodes to a list
Nick Clemens [Thu, 1 Dec 2016 11:47:31 +0000 (11:47 +0000)]
Bug 7663 - batch add barcodes to a list

This patch changes the barcode input on add list to a textarea and
process multiples barcodes to allow for adding many items to a list
at once

To test:
 1 - Create a list
 2 - Add some items
 3 - Verify items are added and success reported
 4 - Add these items again
 5 - Verify you are notified they were not added
 6 - Type invalid barcodes into the text area and submit
 7 - Verify you are notifed of failure to add
 8 - Try going to a list that doesn't exist and adding barcodes
  i.e. http://localhost:8081/cgi-bin/koha/virtualshelves/shelves.pl?op=add_biblio&shelfnumber=9999&barcodes=4
 9 - Verify correct error
10 - Try the above but with a list that exists while signed in as a patron
     without lists permission:
     i.e. http://localhost:8080/cgi-bin/koha/opac-shelves.pl?op=view&shelfnumber=3
11 - Verify correct error: "You do not have permission to view this list."

Signed-off-by: Chris Kirby <chris.kirby@ilsleypubliclibrary.org>
Works as advertised.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18169: Add test without before parameter for search_patrons_to_anonymise
Jonathan Druart [Mon, 27 Feb 2017 13:28:20 +0000 (13:28 +0000)]
Bug 18169: Add test without before parameter for search_patrons_to_anonymise

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18169: Make 'before' param non mandatory for Koha::Patrons->anonymise_issue_history
Jonathan Druart [Fri, 24 Feb 2017 12:26:29 +0000 (13:26 +0100)]
Bug 18169: Make 'before' param non mandatory for Koha::Patrons->anonymise_issue_history

From opac-privacy.pl:

    # delete all reading records for items returned
    # uses a hardcoded date ridiculously far in the future

    my $rows = eval {
        Koha::Patrons->search({ 'me.borrowernumber' => $borrowernumber })->anonymise_issue_history( { before => '2999-12-12' } );
    };

It sounds better to make this before parameter not mandatory, and remove the condition from the sql query if it is not passed.

Test plan:
1. Anonymise your reading history at the OPAC.
2. Confirm that all your reading history has been anonymised

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18122 - Audio alerts: Add hint on where to enable sounds
Marc Véron [Wed, 15 Feb 2017 09:24:56 +0000 (10:24 +0100)]
Bug 18122 - Audio alerts: Add hint on where to enable sounds

This patch adds a hint to page Home > Administration > Audio alerts
and related help page about where to enable the sound system
(syspref 'AudioAlerts').

To test:
- Apply patch
- Disable syspref 'AudioAlerts'
- Go to Home > Administration > Audio alerts
- Note hint below title 'Audio alerts'
- Enable syspref 'AudioAlerts'
- Go to  Home > Administration > Audio alerts
- Hint is no longer visible (since audios are enabled)
- Open Help page (top right)
- Verify that hint on help page makes sense.

Amended to add alert class as requested by comment #2 2017-02-21 mv

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 13685: Sorting by author in opac reading history
Aleisha Amohia [Tue, 14 Feb 2017 19:42:15 +0000 (19:42 +0000)]
Bug 13685: Sorting by author in opac reading history

opac-readingrecord.pl was actually already set up to do this, the option
to sort by author was just not put into the tt file. This patch adds
that.

To test:
1) Log into OPAC, go to your reading history
2) Confirm that 'Order by author' shows in the dropdown
3) Confirm it works as expected

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18108: Sorting by author in opac summary
Aleisha Amohia [Tue, 14 Feb 2017 01:21:01 +0000 (01:21 +0000)]
Bug 18108: Sorting by author in opac summary

To test:
1) Log into OPAC, go to your summary
2) Notice 'author' is in Title column, therefore unable to sort by
author
3) Apply patch and refresh page
4) Notice author has now been moved into Author column
5) Confirm sorting by author works as expected and sorting of all other
columns works as expected

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18070: Correct error from podchecker on line 1376
Marcel de Rooy [Tue, 7 Feb 2017 13:03:54 +0000 (14:03 +0100)]
Bug 18070: Correct error from podchecker on line 1376

Resolves:
 *** ERROR: Spurious =cut command at line 1376 in file C4/AuthoritiesMarc.pm

Test plan:
[1] Run podchecker on C4/AuthoritiesMarc.pm

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18070: [Follow-up] What should DelAuthority return?
Marcel de Rooy [Tue, 14 Feb 2017 15:41:26 +0000 (16:41 +0100)]
Bug 18070: [Follow-up] What should DelAuthority return?

Script tools/batch_delete_records.pl only checks the return value of
DelAuthority. The return value depends on DBI rows, which is not always
reliable. It may return -1 when it 'does not know'.

Testing $@ in tools/batch_delete_records.pl should actually be enough.
The return value was discarded in C4/ImportBatch.pm. Removing an unused
variable.

Test plan:
[1] Pick an authority record with a few linked biblios.
    Delete this authority record via tools/batch_delete_records.pl.
    Check if the linked biblio records are cleaned up.
[2] Bonus: Make a typo in the SQL statement of DelAuthority. Check
    if batch_delete_records shows you the error message.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18070: Extend sub merge to remove fields for deleted authorities
Marcel de Rooy [Tue, 31 Jan 2017 12:01:58 +0000 (13:01 +0100)]
Bug 18070: Extend sub merge to remove fields for deleted authorities

In order to accomplish this, we need to add some additional checks in
the merge routine. The actual change to remove the field, is quite
small.

Furthermore, we need to add a merge call in DelAuthority and adjust
the merge cron job accordingly.

The change is well supported by additional tests, including a simulation
of postponed removal via cron, if dontmerge is enabled.

Note: Deleting an authority with linked biblios is tested on the next
patch.

Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
[2] Delete an authority without linked biblios from the Authorities
    module. If the indexer is not fast enough, wait a bit and refresh to
    verify that the authority is gone on authorities-home.pl.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18070: Deleting an authority should update the linked biblio records
Marcel de Rooy [Tue, 31 Jan 2017 10:46:21 +0000 (11:46 +0100)]
Bug 18070: Deleting an authority should update the linked biblio records

Adding a test where we delete an authority and prove that the linked
biblio still contains a reference to it.

Note: Currently, you can only delete a used authority from Tools,
batch record deletion. If you do, the biblio records will still
contain references to the deleted authority.

Test plan:
[1] Run t/db_dependent/Authorities/Merge.t
    Last test should fail: not ok 1 - Field 609 should be gone too

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18037: Hold notes template cleanup (from 15545)
Marcel de Rooy [Mon, 15 Dec 2014 13:10:56 +0000 (14:10 +0100)]
Bug 18037: Hold notes template cleanup (from 15545)

From the second patch of bug 15545:
Removing some unused template code related to a former approach.
Adding some changes for future extension by bug 15545.

This patch was tested by Liz Rea when the routine IsHoldNoteRequired was
called by opac-reserve.pl. The only change here is that we do not yet
call this routine; so leaving her original signoff.

Test plan:
[1] Enable OPACHoldNotes.
[2] Place a hold on a serial record. No behavior change.

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17840: adding classes to notes in checkouts
Emma Smith [Tue, 17 Jan 2017 03:23:18 +0000 (03:23 +0000)]
Bug 17840: adding classes to notes in checkouts

Test Plan
1) Add an internal and a public note to an item.
2) Check out the item.
3) Verify that both notes are shown in between title and barcode in the
second column of the details table.
4) Verify there is an empty class="" in the HTML source code for each
5) Apply patch and refresh page
6) View page source, confirm that class has been changed

I tested this by first applying the patch for bug 17919 to catch the
check-in software error. Patch working as expected :)

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17334: Format the date according to the system preference
Jonathan Druart [Thu, 26 Jan 2017 11:43:14 +0000 (12:43 +0100)]
Bug 17334: Format the date according to the system preference

- Use KohaDates TT plugin to format the date correctly
- Do not use a temporary variable
- Replace:
    patron's info Requested: Datetime
  with
    patron's info - Requested on Datetime

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17334- Added a timestamp to the members-update.tt To show the user when the pendi...
Alex Buckley [Fri, 20 Jan 2017 03:59:24 +0000 (03:59 +0000)]
Bug 17334- Added a timestamp to the members-update.tt To show the user when the pending modifications were requested.

Test plan:
1.Log into OPAC and click on Lists->Your Lists->your personal details

2. Change/add a value in that form

3. Select the 'Submit update request'

4. Log into the Koha staff interface, then go to the url:
cgi-bin/koha/members/members-update.pl

5. You will see near the top of each box containing a pending request the following are displayed: surname, firstname,
    patron category, branch, and cardnumber but no timestamp.

6. Apply this patch

7. Repeat steps 1,2, 3, 4

8. You will now see timestamp in the format "Requested: YYYY-MM-DD
HH:MM:SS" next to each patrons details near the top of each pending
request box.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: [QA Follow-up] Check count, not size in template
Marcel de Rooy [Mon, 27 Feb 2017 12:21:18 +0000 (13:21 +0100)]
Bug 16966: [QA Follow-up] Check count, not size in template

The method size is not covered by tests at koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt line 163.
The method size is not covered by tests at koha-tmpl/intranet-tmpl/prog/en/modules/tools/cleanborrowers.tt line 183.

Script cleanborrowers passes patrons_to_delete and patrons_to_anonymize to
the template. One is an array ref where we should use 'size', the other is a
Koha::Patrons object where we should use 'count'.
If we don't, we will not anonymize a lot of records ;)

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: move parameters to hashref
Jonathan Druart [Fri, 24 Feb 2017 11:34:26 +0000 (12:34 +0100)]
Bug 16966: move parameters to hashref

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: Fix small QA issues
Jonathan Druart [Fri, 24 Feb 2017 11:25:33 +0000 (12:25 +0100)]
Bug 16966: Fix small QA issues

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: Teach K::S::R::OldIssue its Koha objects class
Jonathan Druart [Fri, 17 Feb 2017 09:39:09 +0000 (10:39 +0100)]
Bug 16966: Teach K::S::R::OldIssue its Koha objects class

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: The method should return a Koha::Patrons object
Jonathan Druart [Tue, 20 Dec 2016 11:15:10 +0000 (12:15 +0100)]
Bug 16966: The method should return a Koha::Patrons object

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: Make the tests pass again
Jonathan Druart [Tue, 20 Dec 2016 10:08:35 +0000 (10:08 +0000)]
Bug 16966: Make the tests pass again

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patr...
Jonathan Druart [Fri, 22 Jul 2016 16:25:24 +0000 (17:25 +0100)]
Bug 16966: Koha::Patrons - Move GetBorrowersWithIssuesHistoryOlderThan to search_patrons_to_anonymise

The C4::Members::GetBorrowersWithIssuesHistoryOlderThan subroutine is supposed
to return the patrons with an issue history older than a given date.

It would make more sense to return a list of Koha::Patrons.

On the way, the code from AnonymiseIssueHistory will be moved as well to
anonymise_issue_history.

Note that these 2 subroutines are strongly linked: one is used to know the
number of patrons we will anonymise the history, the other one is used to
anonymise the issues history. The problem is that the first one is not used to
do the action, but only for displayed purpose.

In some cases, these 2 values can differ, which could be confusing.
Case 1:
The logged in librarian is not superlibrarian and IndependentBranches is set:
if 2+ patrons from different libraries match the date parameter, the interface
will display "Checkout history for 2 patrons will be anonymized", when actually
only 1 will be.
Case 2:
If 2+ patrons match the date parameter but one of them has his privacy set to
forever (privacy=0), the same issue will appear.

This patch moves the code from C4::Members::GetBorrowersWithIssuesHistoryOlderThan
to Koha::Patrons->search_patrons_to_anonymise and from
C4::Circulation::AnonymiseIssueHistory to
Koha::Patrons->anonymise_issue_history

Test plan:
1/ Confirm the 2 issues and make sure they are fixed using the Batch
patron anonymization tool (tools/cleanborrowers.pl)
2/ At the OPAC, use the 'Immediate deletion' button to delete all your
reading history (regardless the setting of the privacy rule)
3/ Use the cronjob script (misc/cronjobs/batch_anonymise.pl) to
anonymise patrons.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests
Jonathan Druart [Fri, 22 Jul 2016 15:43:56 +0000 (16:43 +0100)]
Bug 16966: Koha::Patrons - Move AnonymiseIssueHistory tests

The next patch will move C4::Circulation::AnonymiseIssueHistory and
C4::Members::GetBorrowersWithIssuesHistoryOlderThan to Koha::Patrons
This patch move the history anonymisation code to the Patrons.t test
file and the entire subtest related to StoreLastBorrower to
StoreLastBorrower.t

It just moves and add some minor adjustements.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue...
Jonathan Druart [Fri, 22 Jul 2016 14:57:26 +0000 (15:57 +0100)]
Bug 16966: GetBorrowersWithIssuesHistoryOlderThan - Highlight the existing issue in tests

If IndependentBranches is set, the code is buggy. This patch only
highlight the bug by providing a test.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 13968: Follow up - Clean HTML tags for hints
Mark Tompsett [Tue, 21 Feb 2017 18:13:02 +0000 (13:13 -0500)]
Bug 13968: Follow up - Clean HTML tags for hints

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 13968 - Branch email hints are misleading
Marc Véron [Sat, 18 Feb 2017 16:15:06 +0000 (17:15 +0100)]
Bug 13968 - Branch email hints are misleading

This patch replaces missleading notes with Reply-To and Return-Path fields
to reflect indicate falback values if they are empty (ReplyToDefault and
ReturnpathDefault system prefs).

To test:
- Apply patch
- In Home > Administration > Libraties and Groups, edit a library
- Verify that hints display for fields Reply-To and Return-Path, indicating
  their default values.

Amended to remove extra </li> - 2017-02-21 mv

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18087: Handle invalid filetypes
Jonathan Druart [Tue, 14 Feb 2017 09:52:14 +0000 (10:52 +0100)]
Bug 18087: Handle invalid filetypes

If an invalid file is used as biblionumber list, we should display a
message.

Test plan:
1/ Use csv, plain text files
=> Should work
2/ Use invalid files (binary files like pdf, doc*, xsl*, etc.)
=> Should not work and see a warning message.

Amended patch after signoff: Remove one warn debug line

Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18087 - Clarification on File type when using file of biblionumbers to export...
Joy Nelson [Thu, 9 Feb 2017 19:42:57 +0000 (11:42 -0800)]
Bug 18087 - Clarification on File type when using file of biblionumbers to export data

Added a line to the screen  detailing the types of files that can be used to upload a list of biblionumbers.

Test plan:
1. Go to tools->export data
2. under File option,  (File types accepted: .csv and .txt)  should appear
3. Check both Bibliographic and Authority Export screens for this new string.

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Joy Nelson <joy@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17992: REST api: Remove the use of ->unblessed from Cities controller
Tomas Cohen Arazi [Wed, 25 Jan 2017 12:35:42 +0000 (09:35 -0300)]
Bug 17992: REST api: Remove the use of ->unblessed from Cities controller

As bug 17932 adds TO_JSON to Koha::Object(s), there's no need for using
it. Also, as this is a reference implementation for developers to copy
and paste it is sensible to make this change even when this is not
causing any bug.

To test:
- Apply the patch
- Run:
  $ sudo koha-shell kohadev
 k$ cd kohaclone
 k$ prove t/db_dependent/api/v1/cities.t
=> SUCCESS: Tests still pass!
- Sign off :-D

Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18160 [QA Followup] - Correct whitespace
Kyle M Hall [Fri, 3 Mar 2017 16:54:15 +0000 (16:54 +0000)]
Bug 18160 [QA Followup] - Correct whitespace

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18160: Follow up tweak if condition
Mark Tompsett [Thu, 23 Feb 2017 16:47:00 +0000 (11:47 -0500)]
Bug 18160: Follow up tweak if condition

Based on comment #9, moved if condition.
Based on comment #8, tweaked condition to grab preference.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18160 - Error when OverDriveCirculation not enabled
Nick Clemens [Thu, 23 Feb 2017 00:35:13 +0000 (00:35 +0000)]
Bug 18160 - Error when OverDriveCirculation not enabled

This patch only executes OverDrive circulation JS if
OverDriveCirculation pref is enabled

To test:
1 - Disable OverDriveCirculation
2 - Log in to opac and view summary tab
3 - Note 'Internal server error'
4 - Apply patch
5 - Repeat and note error is not present

Signed-off-by: Srdjan <srdjan@catalyst.net.nz>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17941 add tests to cover the optimization of the nested loop
Francesco Rivetti [Mon, 20 Feb 2017 10:02:40 +0000 (11:02 +0100)]
Bug 17941 add tests to cover the optimization of the nested loop

the new code skip testing for more borrowers if the item can't be renewed
this require more tests for the case where 2+ items are reserved.

I tried to add 1 more reserve to the main test suit, but too many other tests
rely on specific holds, and I couldn't get around it.

Instead, I added a subtest that consider the specific simple case and leave the
other test cases exactly has they were designed.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Amended-patch: Just fix 3 indendations

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17941 don't compute priority more than needed
Francesco Rivetti [Tue, 14 Feb 2017 10:16:01 +0000 (11:16 +0100)]
Bug 17941 don't compute priority more than needed

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17941 avoid scanning the full cartesian product
Francesco Rivetti [Tue, 14 Feb 2017 10:13:51 +0000 (11:13 +0100)]
Bug 17941 avoid scanning the full cartesian product

when a item match a borrower, there is no point in checking the
other borrowers

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17260 - DBRev 16.12.00.011
Kyle M Hall [Fri, 3 Mar 2017 16:45:22 +0000 (16:45 +0000)]
Bug 17260 - DBRev 16.12.00.011

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17260: Update existing installs
Jonathan Druart [Fri, 24 Feb 2017 08:08:30 +0000 (08:08 +0000)]
Bug 17260: Update existing installs

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns
Blou [Fri, 27 Jan 2017 16:51:26 +0000 (16:51 +0000)]
Bug 17260: updatedatabase.pl fails on invalid entries in ENUM and BOOLEAN columns

The loading of file admin/searchengine/elasticsearch/mappings.yaml
specifies 'type' as empty, which fails with Mysql 5.7+ which is more
stringent.

Also, forcing an empty value into a boolean also fails.

Both issues arise when updatedatabase.pl hit 3.23.00.050.

NOTE: both issues could also be resolved by actually setting values in
      the load file. This doesn't make this solution incorrect, though.

To Test/reproduce:
-1) Happens with Mysql 5.7.4.  Maybe earlier, but certainly at
    that point. Use a Xenial kohadevbox to more easily test.
0) Find a database on 3.22 or earlier, save it.
    a) place
           sql_mode=IGNORE_SPACE,NO_ZERO_IN_DATE,NO_ZERO_DATE,ERROR_FOR_DIVISION_BY_ZERO,NO_AUTO_CREATE_USER,NO_ENGINE_SUBSTITUTION
       into the [mysqld] section of /etc/mysql/mysql.conf.d/mysqld.cnf
    b) restart you mysql server
    c) drop the db and recreate it
    d) checkout the 3.22.x branch
    e) do a web install
    f) remove the added sql_mode
    g) restart the mysql server
1) Set your code base to master
2) run updatedatabase.pl
3) See the errors on 3.23.00.050
4) Apply the patch
5) Reload the 3.22 db.
   a) repeat steps 0(a)-0(g)
   b) don't forget the caching issues
6) succeed with updatedatabase.pl
7) drop the db and recreate it
8) run the web installer
9) notice no issues either.
10) run koha qa test tools

NOTE: This bug only solved the upgrade portion.
      I added the kohastructure.sql change as well.
      I confirmed that all the code changes were
      triggered with this test plan.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Mehdi Hamidi <mehdi.hamidi@inlibro.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17216: Fix SQL statement to setting marc_subfield_structure.authorised_value...
Jonathan Druart [Wed, 23 Nov 2016 14:47:10 +0000 (14:47 +0000)]
Bug 17216: Fix SQL statement to setting marc_subfield_structure.authorised_value to NULL

We want to update it if it's empty, not equal to ';'
nopaste vim mode is to blame

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoRevert "Bug 12461 - Add patron clubs feature"
Kyle M Hall [Mon, 27 Feb 2017 01:41:27 +0000 (20:41 -0500)]
Revert "Bug 12461 - Add patron clubs feature"

This reverts commit 4f1eefdbb8bf4df07b03fc451c1c18ebe989c8b7.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16034: (followup) Fix DB update and schema files
Tomas Cohen Arazi [Fri, 24 Feb 2017 12:22:26 +0000 (09:22 -0300)]
Bug 16034: (followup) Fix DB update and schema files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 12461 - Add patron clubs feature
Kyle M Hall [Fri, 23 May 2014 15:54:26 +0000 (11:54 -0400)]
Bug 12461 - Add patron clubs feature

This features would add the ability to create clubs which patrons may be
enrolled in. It would be particularly useful for tracking summer reading
programs, book clubs and other such clubs.

Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Ensure your staff user has the new 'Patron clubs' permissions
4) Under the tools menu, click the "Patron clubs" link
5) Create a new club template
   * Here you can add fields that can be filled out at the time
     a new club is created based on the template, or a new enrollment
     is created for a given club based on the template.
6) Create a new club based on that template
7) Attempt to enroll a patron in that club
8) Create a club with email required set
9) Attempt to enroll a patron without an email address in that club
10) Create a club that is enrollable from the OPAC
11) Attempt to enroll a patron in that club
12) Attempt to cancel a club enrollment from the OPAC
13) Attempt to cancel a club enrollment from the staff interface

Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
7 years agoBug 16034 - DBRev 16.12.00.010
Kyle M Hall [Tue, 21 Feb 2017 20:01:17 +0000 (20:01 +0000)]
Bug 16034 - DBRev 16.12.00.010

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 16034 follow-up: added WebService::ILS to PerlDependencies
Srdjan [Fri, 29 Jul 2016 00:42:23 +0000 (12:42 +1200)]
Bug 16034 follow-up: added WebService::ILS to PerlDependencies

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agobug_16034 Add overdrive info to the users page in the public interface
Srdjan [Tue, 8 Dec 2015 06:07:41 +0000 (19:07 +1300)]
bug_16034 Add overdrive info to the users page in the public interface

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agobug_16034 Adding a new syspref - OverDriveCirculation
Chris Cormack [Tue, 18 Aug 2015 03:55:51 +0000 (15:55 +1200)]
bug_16034 Adding a new syspref - OverDriveCirculation

If set to true, Opac will integrate OverDrive patron services

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agobug_16034 Koha::ExternalContent::OverDrive - a wrapper around WebService::ILS::Overdr...
Srdjan [Tue, 8 Dec 2015 06:06:27 +0000 (19:06 +1300)]
bug_16034 Koha::ExternalContent::OverDrive - a wrapper around WebService::ILS::Overdrive::Patron

* Using the upstream module for all the heavy lifting
* opac/external/overdrive/auth.pl - 3-legged authentication handler

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18077: Batch item modification link is bold when batch item deletion is active...
Aleisha Amohia [Wed, 8 Feb 2017 02:07:29 +0000 (02:07 +0000)]
Bug 18077: Batch item modification link is bold when batch item deletion is active in tools menu

To test:
1) Go to Tools -> Batch item deletion
2) Notice the link for Batch item MODIFICATION is bold, not the deletion
link
3) Apply patch and refresh page
4) Correct link should now be bold.
5) Go to batch item modification and confirm the correct link is bold

Sponsored-by: Catalyst IT
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Re-tested, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18109: Adding font awesome icon to uncertain prices button
Aleisha Amohia [Tue, 14 Feb 2017 02:13:33 +0000 (02:13 +0000)]
Bug 18109: Adding font awesome icon to uncertain prices button

To test:
1) Go to Acquisitions -> search for a vendor
2) View a basket with uncertain prices or add an order with uncertain
prices to a basket
3) Confirm that font awesome icon is appropriate and button still works
as expected

Sponsored-by: Catalyst IT
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17852 Follow up.Multiple URLs (856) in list email are broken
Caitlin Goodger [Thu, 19 Jan 2017 00:54:24 +0000 (00:54 +0000)]
Bug 17852 Follow up.Multiple URLs (856) in list email are broken

This is a follow-up to my previous patch. This fixes it on the OPAC and
the previous one was from the intranet
Test Plan
1: Add multiple urls to a biblographic record
2: Add the biblographic record to a list and then email it from the OPAC
3: See that the links are broken and have been merged into one.
4: Apply patch
5: Send the list again and see that the links work

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17852; Multiple URLs (856) in list email are broken.
Caitlin Goodger [Tue, 17 Jan 2017 21:37:49 +0000 (21:37 +0000)]
Bug 17852; Multiple URLs (856) in list email are broken.

Test Plan
1: Add multiple urls to a biblographic record
2: Add the biblographic record to a list and then email it.
3: See that the links are broken and have been merged into one.
4: Apply patch
5: Send the list again and see that the links work

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17865 'If a subscription has no history end date, it shows as expired today in...
Luke Honiss [Tue, 17 Jan 2017 02:27:55 +0000 (02:27 +0000)]
Bug 17865 'If a subscription has no history end date, it shows as expired today in OPAC'

--TEST PLAN--
1) View a subscription with no history end date
2) Search for an item with a subcription in OPAC
3) Under subscription tab click more details
4) The end date will be the current date
5) Apply patch and refresh
6) The end date will not be shown

Signed-off-by: Baptiste Wojtkowski <baptiste.wojtkowski@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18026: Correct URL to database columns from .pref files
Jonathan Druart [Mon, 20 Feb 2017 09:49:11 +0000 (10:49 +0100)]
Bug 18026: Correct URL to database columns from .pref files

Recently the DB schema website has been updated.
The link to the columns' tables is now
  http://schema.koha-community.org/master/tables/table.html
  http://schema.koha-community.org/16_11/tables/table.html
  http://schema.koha-community.org/16_05/tables/table.html
instead of
  http://schema.koha-community.org/tables/table.html

To have a specific schema per Koha version.

To avoid fixing this link every release this patch adds a __VERSION__
marker to replace with the $version if stable ($version % 2) or 'master'

Test plan:
On master, 16.11.x and 16.05.x
Search for PatronSelfRegistrationBorrowerUnwantedField,
BorrowerMandatoryField and UniqueItemFields
Click on the link
You should get the correct page

This patch WILL NOT WORK on 3.22.x

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18111: Swap framework_name and frameworkcode
Mark Tompsett [Sun, 19 Feb 2017 04:06:16 +0000 (23:06 -0500)]
Bug 18111: Swap framework_name and frameworkcode

With the frameworkcode changed to an empty string, which really
is closer to the physical reality, some javascript somewhere
breaks. This means the spinner does not appear when you confirm
that you wish to import it.

TEST PLAN
----------
1) apply first patch
2) export default framework, doesn't matter format
3) import default framework using that file.
4) answer yes or whatever it is.
   -- spinner does not appear.
5) apply this patch
6) import default framework use the same file.
7) answer yes or whatever it is.
   -- spinner does appear.
8) decide if you wish to find and fix the javascript conditions.
   if yes -- do so, and then obsolete this once you have.
   if not -- sign this off after running koha qa test tools

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18111: Fix import of default framework
Jonathan Druart [Tue, 14 Feb 2017 16:21:45 +0000 (16:21 +0000)]
Bug 18111: Fix import of default framework

Caused by bug 16035.

Recreate the issue:
Go to Home › Administration › MARC frameworks
For the default framework: Actions > import
It will not work

Test plan:
Confirm test plans from bug 16035 and bug 17389 still pass
Confirm that the import for the default framework now works

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17461: Warn about plugins that can't be loaded
Magnus Enger [Tue, 18 Oct 2016 09:47:19 +0000 (09:47 +0000)]
Bug 17461: Warn about plugins that can't be loaded

The logs should have some info if a module has been installed that can
not be loaded.

To test:
- Make sure you have an up to date dev install
- Install v1.0 of the KitchenSink plugin from
  https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases
- Make sure the plugin does not show up in the list of installed
  plugins, and that the intranet-error.log has no info about it
- Apply the patch
- Reload plugins-home.pl
- Verify that there is now some interesting info in intranet-error.log

Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Works as advertised, generates useful diagnostic information in the form of a log entry to either intranet-error or plack-error.log. Relief, thanks Magnus!
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453 - DBRev 16.12.00.009
Kyle M Hall [Fri, 17 Feb 2017 17:17:12 +0000 (17:17 +0000)]
Bug 17453 - DBRev 16.12.00.009

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453 (QA Followup)
Nick Clemens [Wed, 15 Feb 2017 20:13:14 +0000 (20:13 +0000)]
Bug 17453 (QA Followup)

    Add line break before note message

https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Take into account items that are lost or damaged
Jonathan Druart [Fri, 10 Feb 2017 07:54:39 +0000 (08:54 +0100)]
Bug 17453: Take into account items that are lost or damaged

If all the items are either lost, damaged or checked out, then pickup
should be allowed.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Add a visible notification to patron to explain why the hold locations...
Jonathan Druart [Thu, 9 Feb 2017 17:46:38 +0000 (18:46 +0100)]
Bug 17453: Add a visible notification to patron to explain why the hold locations are disabled

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Allow pickup at a library where all items are checked out
Jonathan Druart [Mon, 31 Oct 2016 11:16:05 +0000 (11:16 +0000)]
Bug 17453: Allow pickup at a library where all items are checked out

If all items are checked out then it should be possible to select the pickup
library for that record.

Signed-off-by: Janet McGowan <janet.mcgowan@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Add exceptions
Jonathan Druart [Mon, 24 Oct 2016 09:02:19 +0000 (09:02 +0000)]
Bug 17453: Add exceptions

This patch adds the ability to define patron categories not affected by
the behavior of OPACHoldsIfAvailableAtPickup.
The new pref OPACHoldsIfAvailableAtPickupExceptions get a list of patron
categories (separated by pipes |).

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Inter-site holds improvement
Jonathan Druart [Tue, 18 Oct 2016 13:02:58 +0000 (14:02 +0100)]
Bug 17453: Inter-site holds improvement

At the moment users can reserve items and choose any library as a pick up
location, but there is no mechanism to prevent users from reserving items that
are available on the shelf at any given location from reserving the item at the
same location, essentially creating a Fetch and Collect scenario.
This has an impact on staff workloads as they are having to process reservations
and check shelves for items that students can already come and collect from the
open library shelves.
The aim of this enhancement is to decrease the impact on staff workload there
should be a restriction in place that prevents users from requesting items for
collection at a library where the item is currently available.

Implementation:
We first tried to add a new circulation rule adding a 4th
“NotIfAvailableAtPickupLibrary” option to "On shelf holds allowed".
That would make the development more flexible.
But in that case we quickly faced non-trivial problematics:
Let's say you have 3 items I1, I2 and I3. The first one has onshelfholds
set to Yes and 2 others has it set to “NotIfAvailableAtPickupLibrary”.
What would be the expected behavior if a hold is placed at biblio level?
And if a hold is placed at item level for I1?
This second point could be answered by reworking the interface to move
the libraries dropdown list elsewhere (1 list per item) or by adding a
lot of JS code to handle the different situation. But it would be
much more complicated to implement.
So finally I moved back to the simple approach and added a new pref to
handle the behavior globally.

Test plan:
0/ Switch off OPACHoldsIfAvailableAtPickup
1/ Let's say you have 3 libraries L1, L2, L3, create 2 items owned by L1
and L2
2/ Place a biblio level hold. You should only be able to pick it up at
L3
2/ Place a item level hold. You should only be able to pick it up at
L3
3/ Create a third items owned by L3
4/ Now you should not be able to place a hold on this record anymore

Sponsored-by: University of the Arts London
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17453: Add pref OPACHoldsIfAvailableAtPickup
Jonathan Druart [Tue, 18 Oct 2016 12:06:55 +0000 (13:06 +0100)]
Bug 17453: Add pref OPACHoldsIfAvailableAtPickup

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
https://bugs.koha-community.org/show_bug.cgi?id=14753

7 years agoBug 14405: Remove pagination type, there is no pagination at the OPAC
Jonathan Druart [Tue, 14 Feb 2017 12:42:41 +0000 (13:42 +0100)]
Bug 14405: Remove pagination type, there is no pagination at the OPAC

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 14405: Add datatables to fines table in OPAC
Aleisha Amohia [Thu, 2 Feb 2017 04:16:53 +0000 (04:16 +0000)]
Bug 14405: Add datatables to fines table in OPAC

To test:
1) Apply patch
2) Go to OPAC -> Your account
3) Confirm you can now sort on all columns

Sponsored-by: Catalyst IT
NOTE: NICE!

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 11932: Move delete checkbox on patron modification to right
Aleisha Amohia [Thu, 2 Feb 2017 23:30:43 +0000 (23:30 +0000)]
Bug 11932: Move delete checkbox on patron modification to right

To test:
1) Go to Tools -> Batch patron modification
2) Put in a user cardnumber and click Continue
3) Notice checkboxes on the left of the input fields
4) Apply patch and refresh page (resend information if prompted)
5) Notice checkboxes on the right of the input fields
6) Confirm they still work as expected

Sponsored-by: Catalyst IT
NOTE: Why not left before the label? And while functional,
      is there a pretty align it nicely with all the other
      checkboxes CSS styling somehow?

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 7626: Delete multiple tags in OPAC at once
Aleisha Amohia [Wed, 15 Feb 2017 00:03:28 +0000 (00:03 +0000)]
Bug 7626: Delete multiple tags in OPAC at once

This patch removes the individual 'Delete' buttons for every tag as
suggested in Comment 0. It adds a checkbox to every tag, with a 'Delete
selected tags' button at the bottom of the table. This patch also adds a
function to check if any checkboxes have been checked before submitting
the form (only works with JS enabled).

To test:
1) Find a record in the OPAC and add some tags to it
2) Go to your tags and notice the checkboxes, no more delete buttons for
each tag
3) Click 'Delete selected tags' button without selecting any tags
4) Confirm alert shows up. Click OK
5) Select one or more tags and click 'Delete selected tags' button
6) Confirm delete is successful and correct number of deleted tags shows
up

Sponsored-by: Catalyst IT
Followed test plan, works as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18099 - Put call number in its own column on inventory screen
David Cook [Mon, 13 Feb 2017 05:39:41 +0000 (16:39 +1100)]
Bug 18099 - Put call number in its own column on inventory screen

TEST PLAN

0) Apply patch

1) Create an item with a call number in 952$o (e.g. ABC 123)
2) Using the inventory tool, make sure you "Select items you want to check"
which include that item (e.g. Item callnumber between: A and C)
3) Click "Submit"
4) The call number appears in the column marked "Call number"
instead of appearing as [ABC 123] under the library in the
"Library" column

Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18063: Remove dead code from tools/manage-marc-import.tt
Julian Maurice [Mon, 6 Feb 2017 13:58:06 +0000 (14:58 +0100)]
Bug 18063: Remove dead code from tools/manage-marc-import.tt

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18040: Updating buttons in Tools -> Upload local cover image
Aleisha Amohia [Thu, 2 Feb 2017 22:58:34 +0000 (22:58 +0000)]
Bug 18040: Updating buttons in Tools -> Upload local cover image

To test:
1) Go to Tools -> Upload local cover image
2) Select an image and click Upload file
3) Notice Process images button looks different to Upload file button
4) Apply patch and refresh page
5) Follow steps 1-3 again and notice the changed styling of the buttons

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18036: [QA Follow-up] Descriptions and mock_preference
Marcel de Rooy [Fri, 3 Feb 2017 07:08:07 +0000 (08:08 +0100)]
Bug 18036: [QA Follow-up] Descriptions and mock_preference

Adjusted a few test descriptions.
The test depends on template being set to prog. Made that explicit by
adding a mock_preference. If you change the mock to bootstrap, this
test will fail.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18036: All 4 cases tested
Mark Tompsett [Thu, 2 Feb 2017 17:04:37 +0000 (12:04 -0500)]
Bug 18036: All 4 cases tested

Added theme, lang, and availablethemes tests for all 4 cases.
Previously just availablethemes was tested.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The warns from Mark's additional patch showed all four cases.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 18036: Perlcritic improve t/db_dependent/Templates.t
Mark Tompsett [Thu, 2 Feb 2017 17:16:01 +0000 (12:16 -0500)]
Bug 18036: Perlcritic improve t/db_dependent/Templates.t

TEST PLAN
---------
1) apply 17982
2) perlcritic -3 t/db_dependent/Templates.t
   -- messages
3) apply this patch
4) perlcritic -3 t/db_dependent/Templates.t
   -- OK
5) run koha qa test tools

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17968: Remove useless variable $item_format in C4::Overdues::parse_overdues_letter
Jonathan Druart [Thu, 19 Jan 2017 17:03:55 +0000 (18:03 +0100)]
Bug 17968: Remove useless variable $item_format in C4::Overdues::parse_overdues_letter

The variable $item_format is not used and should be removed from this
subroutine.

Moreover it the letter parameter, but it is never sent to this
subroutine. letter_code is expected instead.

Test plan:
No test plan, just read the code and `git grep `

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17947: Searching my library first shows the branchcode by the search bar rather...
Aleisha Amohia [Mon, 23 Jan 2017 22:32:39 +0000 (22:32 +0000)]
Bug 17947: Searching my library first shows the branchcode by the search bar rather than branchname

Test Plan
1: Turn on SearchMyLibraryFirst and go into OPAC
2: It should say Search (in <Branch Code> Only)
3: Apply patch and refresh page. It should now say Search (in <Library Name> only)

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17895: 'Re-set' typo fix
Aleisha Amohia [Fri, 13 Jan 2017 00:19:48 +0000 (00:19 +0000)]
Bug 17895: 'Re-set' typo fix

To test:
1) Log into OPAC and go to change your password
2) Enter current password incorrectly and click Submit changes
3) Observe error message and typo 're-set'
4) Apply patch and refresh page
5) May be asked to resend data - if so, click Resend, if not, repeat
step 2
6) Observe error message and corrected spelling of 'reset'

Sponsored-by: Catalyst IT
Signed-off-by: Caitlin Goodger <caitlingoodger.student@wegc.school.nz>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17846: Remove get_infos_of declaration
Mark Tompsett [Wed, 4 Jan 2017 16:11:03 +0000 (16:11 +0000)]
Bug 17846: Remove get_infos_of declaration

The function which was removed was still being exported.
This removes it completely.

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17846: Remove C4::Koha::get_infos_of
Jonathan Druart [Wed, 4 Jan 2017 11:39:18 +0000 (12:39 +0100)]
Bug 17846: Remove C4::Koha::get_infos_of

This subroutine does not longer make any senses. The call to
get_infos_of can be replaced with $dbh->selectall_hashref.
The third argument of this subroutine was never used.

Test plan (for developer only):
Compare the 2 codes and confirm that they are equivalent

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17793: Follow up for identified missing changes
Mark Tompsett [Mon, 9 Jan 2017 14:53:11 +0000 (14:53 +0000)]
Bug 17793: Follow up for identified missing changes

I ran:
$ git grep -l cat-search.inc | grep admin

This means I believe the outstanding ones are
(koha-tmpl/intranet-tmpl/prog/en/modules/):
- admin/auth_subfields_structure.tt
- admin/clone-rules.tt
- admin/marc_subfields_structure.tt
- admin/searchengine/elasticsearch/mappings.tt

One other was recommended by Katrin in comment #9:
- plugins/plugins-home.tt

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17793: Make sysprefs search show on all Admin pages
Aleisha Amohia [Mon, 19 Dec 2016 22:56:12 +0000 (22:56 +0000)]
Bug 17793: Make sysprefs search show on all Admin pages

This patch is amended to use the sysprefs search on all Administration
pages that do not have their own custom search.

To test:
1) Go to Administration
2) Notice Catalogue search at the top - seems out of place.
3) Apply patch and refresh page.
4) Notice admin / sysprefs search now shows and is more appropriate.
5) Confirm searching for sysprefs still works

This patch affects the following pages:
- admin-home.pl
- audio_alerts.pl
- authtypes.pl
- auth_tag_structure.pl
- authorised_values.pl
- biblio_framework.pl
- marctagstructure.pl
- branch_transfer_limits.pl
- branches.pl
- checkmarc.pl
- classsources.pl
- columns_settings.pl
- didyoumean.pl
- edi_accounts.pl
- edi_ean_accounts.pl
- fieldmapping.pl
- item_circulation_alerts.pl
- items_search_fields.pl
- items_search_field.pl
- item_types.pl
- koha2marklinks.pl
- matching-rules.pl
- oai_sets.pl
- oai_set_mappings.pl
- patron-attr-types.pl
- smart-rules.pl
- transport-cost-matrix.pl
- sms_providers.pl

Sponsored-by: Catalyst IT
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17932: (followup) Tidy tests
Tomas Cohen Arazi [Tue, 24 Jan 2017 13:48:31 +0000 (10:48 -0300)]
Bug 17932: (followup) Tidy tests

This patch makes the tests create its own data instead of searching the
DB for a branchcode and a categorycode. It does so on each subtest,
because there shouldn't be side effects between subtests.

I also wrapped each subtest inside a transaction, for the same reason.

To test:
- Apply this patch
- Run:
  $ prove t/db_dependent/Koha/Object.t
=> SUCCESS: Tests return green with this patch
- Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17932: Unit tests
Tomas Cohen Arazi [Tue, 24 Jan 2017 13:41:00 +0000 (10:41 -0300)]
Bug 17932: Unit tests

This patch adds unit tests for the Koha::Object::TO_JSON function.
It tests on top of Koha::Patron as Koha::Object needs to be
instantiated.

To test:
- Apply the patch
- Run:
  $ prove -v t/db_dependent/Koha/Object.t
=> SUCCESS: New tests for TO_JSON are run and return green.
- Sign off :-D

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17932: (followup) Fix /patrons endpoint
Tomas Cohen Arazi [Mon, 23 Jan 2017 12:47:54 +0000 (09:47 -0300)]
Bug 17932: (followup) Fix /patrons endpoint

Bug 17927 introduced data type fixes on the /patrons endpoint (integer
and boolean types got fixed). This led to the /patrons endpoint not to
work because the underlying code didn't provide the right data.

With the introduction of TO_JSON on Koha::Object(s) we now have a way to
output the proper data types.

This patch does so by:
- Adding is_boolean => 1 to the relevant columns on the Borrower.pm
  schema file.
- Tweaking the controller class for the /patrons endpoint so it doesn't
  use the $object(s)->unblessed call but just let the Mojo::JSON library
  pick out TO_JSON implementation instead on rendering the output.
- It adds a new test for booleans.

To test:
- Have 17927 applied
- Run:
  $ prove t/db_dependent/api/v1/patrons.t
=> FAIL: Tests fail [1]
- Apply this patches
- Run:
  $ prove t/db_dependent/api/v1/patrons.t
=> SUCCESS: Tests pass!
- Sign off! :-D

[1] It is self explanatory to just try the API using any of the
available tools (I use HttpRequester on Firefox)

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
7 years agoBug 17932: Add a TO_JSON method to Koha::Object(s)
Tomas Cohen Arazi [Wed, 18 Jan 2017 18:37:42 +0000 (15:37 -0300)]
Bug 17932: Add a TO_JSON method to Koha::Object(s)

Bug 17927 fixes data types on the current REST endpoints. If you
test those endpoints, you will notice trying to access them (for listing
or retrieving single objects yields a data type error. Notably on
booleans but also on integers.

Integers fail due to https://rt.cpan.org/Ticket/Display.html?id=119904
but it needs some global solution until there's a DBD::mysql release
backported to the supported distros. There's the option to use
http://search.cpan.org/~frew/DBIx-Class-Helpers-2.033002/lib/DBIx/Class/Helper/Row/NumifyGet.pm
to get the integer columns fixed as a workaround:

 __PACKAGE__->add_columns(
    borrowernumber => {
        data_type         => 'integer',
        is_nullable       => 0,
        is_numeric        => 1,
    }
);

I didn't find bug reports related to this (maybe because we don't use
warnings everywhere) But I don't think is worth going such a heavy
overhead.

A similar situation takes place for Boolean values. They need to be
prepared for JSON output. This could have been done using DBIx filters
as pointed out by Martin:

__PACKAGE__->filter_column(
    lost => {
       filter_to_storage => sub { $_[1] ? 1 : 0 },
       filter_from_storage =>
               sub { $_[1] ? Mojo::JSON->true :
                             Mojo::JSON->false }
       }
);

but this could have other consequences that are worth exploring on
another bug (i.e. it would mean we need to take care of every place
where this boolean data is used/set needs to handle this data types
nicely. Such would be the case if we were a Mojo-only app, but we
aren't. We use Koha::Obect(s) in the whole app. Period.

This patch adds the need to specify on the schema files, columns that
are actually boolean, because we have no way to detect them for now
(i.e. they are all tinyint, but we use tinyint for non-boolean stuff
 too).
So if this patch is accepted, we would need to specify boolean columns
like this:

__PACKAGE__->add_columns(
    '+lost' => {
        is_boolean => 1
    }
);

This patch adds a TO_JSON method for Koha::Object(s) to be used for
serializing Koha::Object-derived objects into JSON strings.

To test it (as Koha::Object(s) need to be instantiated) I provide tests
on top of the Koha::Patron(s) classes in the followup patches.

[1] Yes, we use TINYINT(1) for booleans, but from DBIC's perspective
there's no way to read the (1) in runtime.

Sponsored-by: ByWater Solutions
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>