Commit graph

1724 commits

Author SHA1 Message Date
e8117033e4 Bug 16501: Remove some unneeded warns in Upload.t
At some point I saw some gain in warning about a missing config line.
But these warns are actually not needed here.
Removing them here.

Test plan:
Run t/db_dependent/Upload.t while having upload_path in your koha-conf and
also without it.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-16 17:46:25 +00:00
53e410b14e Bug 16412: Make sure Koha::Cache->set_in_cache will not warn anymore
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
2016-05-12 16:09:53 -06:00
5cf8bfb360 Bug 16496: Add test
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-11 10:37:12 +00:00
3d55706b3b Bug 16414: Drop aqorders.budgetgroup_id
This column has never been used and can be removed.

Test plan:
1/ update the schema
2/ prove t/db_dependent/Acquisition.t
should return green

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Test green pre & post patch
No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-05-06 03:57:36 +00:00
0e6a18bb62 Bug 16453: Make Elasticsearch tests be skipped if configuration entry missing
The current tests fail to run if the configuration entry is missing. This is
problematic on jenkins and should be fixed.

To test:
- On master, having koha-conf.xml without (or commented) an <elasticsearch> entry
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> FAIL: Tests fail due to missing configuration entry
- Apply the patch
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> SUCCESS: Tests pass, and the ES-configuration-dependent tests are skipped
- Have elasticsearch running and the koha-conf.xml entry
- Run:
  $ prove t/db_dependent/Koha_ElasticSearch_Indexer.t \
          t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t -v
=> SUCCESS: Same results as without the patch
- Sign off

Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-05-06 03:47:05 +00:00
e883c19f37 Bug 16426: Add tests for ModMember - do not update userid
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <brendan@bywatersolutions.com>
2016-05-06 03:41:36 +00:00
c4eabeda0b Bug 16447: Remove occurrence of the borrow permission which does no longer exist
Bug 7976 has removed this permission, but other patches re-added it...
Note that the occurrences in sendbasket.pl, edithelp.pl, opac/svc/login should
have been removed by bug 7976.

Test plan:
  git grep 'borrow.*=> 1'
should not return any results.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-05-05 21:28:14 +00:00
cb912decef Bug 16325: Add a test for SearchSuggestions when searching for STATUS=''
Test fails as expected without second patch and passes OK with second patch.
Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-05-05 20:46:01 +00:00
f0983f0570 Bug 16452: Remove the warnings raised by PatronLists.t
We need to define a userenv to get rid of the warnings

Test plan:
  prove t/db_dependent/PatronLists.t
should not return any warnings

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-05-05 20:44:47 +00:00
aec6235762 Bug 16411: Make Hold.t not depend on two existing branches
If you do not have two branches, this test will fail.
Can't call method "branchcode" on an undefined value.

This patch adds a borrower and two branches with TestBuilder.

Test plan:
Run the test.

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Brendan Gallagher <bredan@bywatersolutions.com>
2016-05-05 18:26:56 +00:00
6dbceac466 Bug 16390: Accounts.t does not need MPL
This test fails if you do not have a MPL branch, but a closer look shows
that we only need to replace one occurrence.

Test plan:
Run Accounts.t

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>
2016-05-04 14:25:09 +00:00
c04078abf6 Bug 16405: Fix Circulation/NoIssuesChargeGuarantees.t test 2
Resolves:
Failed test 'Patron cannot check out item due to debt for guarantee'
at t/db_dependent/Circulation/NoIssuesChargeGuarantees.t line 63.
          got: '10'
     expected: '10.00'

Test plan:
Run the test again.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 14:23:34 +00:00
9f18c16872 Bug 10171: (QA followup) Make search tests use the right form
This bug removed the search tab from the main search box, so the tests
that use /cgi-bin/koha/catalogue/search.pl for searching, should use
the advanced search box instead.

To test:
- Run
  $ prove t/db_dependent/www/search_utf8.t
=> FAIL: Several tests related to searching on the intranet interface fail.
- Apply the patch
- Run
  $ prove t/db_dependent/www/search_utf8.t
=> SUCCESS: Tests pass
- Be happy

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 14:14:50 +00:00
Julian Maurice
9d31efa2b5 Bug 13903: (QA followup) change routes to /holds
GET    /holds?borrowernumber=X (list)
POST   /holds                  (create)
PUT    /holds/{reserve_id}     (update)
DELETE /holds/{reserve_id}     (delete)

Unit tests in t/db_dependent/api/v1/holds.t

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:54:01 +00:00
Julian Maurice
70928807d8 Bug 13903: Add API routes to list, create, update, delete reserves
GET    /reserves?borrowernumber=X (list)
POST   /reserves                  (create)
PUT    /reserves/{reserve_id}     (update)
DELETE /reserves/{reserve_id}     (delete)

Unit tests in t/db_dependent/api/v1/reserves.t

Test plan:
1. Apply patch
2. Run unit tests
3. Play with the API with your favorite REST client, using documentation
   in the swagger.json file
4. Try to make reserves until the maximum number of reserves for a user
   is reached (you should have a 403 error)

Depends on bug 15126

Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>

Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:54:01 +00:00
a8f29dd3f2 Bug 16155: (QA followup) fix small bug in t/db_dependent/ILSDI_Services.t
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
With all patches applied all test pass,
all == git grep -l "use t::lib::TestBuilder" | grep -v -e 'pm$' -e Old | xargs prove

No errors

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:58 +00:00
9c136f3d76 Bug 16155: [QA Follow-up] Add transaction to BiblioFrameworks.t
This unit test does not have a transaction.
It does not need TestBuilder.

Test plan:
[1] Optionally remove records with mfw1, mfw2 from biblio_framework table.
    If you ran this test before and it failed, you may have them.
[2] Run the test.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:58 +00:00
2862b19918 Bug 16155: Adjust a few other tests
Accounts.t: Only added a line that ensures the MPL branch exists.
AnonymiseIssueHistory.t: Only add a branch to work with.
Barcodes.t: Replaced clear with delete_all.
CalcFine.t: Remove default issuing rule and add one instead of updating.
Holds.t: Add category S in case it would not exist.
Members.t: Replaced clear with delete_all.
MoveItemFromBiblio.t: Replace last _fk construction.

Test plan:
Run these tests. (See note).
Git grep for only_fk, {_fk} and TestBuilder::default_value.

Note: Holds.t does not pass. Tests 9 and 39 fail, but they did already.
not ok 9 - GetReservesFromItemnumber should return a valid borrowernumber
not ok 39 - Test AlterPriority(), move to bottom
So this test needs attention, but on another report please :)

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:58 +00:00
9de3872b90 Bug 16155: Adjust TestBuilder.t
The changes in TestBuilder.pm require some changes in this test.

[1] Tests have been organized under subtests. A few superfluous tests have
    been removed. (There is still some overlap between the sections
    of overduerules_transport_type and userpermission.)
[2] The results in the build all sources-test are checked one step further.
[3] Tests are added for field length, null values and delete method.
[4] The former defaults from TestBuilder are incorporated in the tests
    for userpermission.

Test plan:
Run t/db_dependent/TestBuilder.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:58 +00:00
df929ef2b4 Bug 16155: Remove a second use from Members_Attributes.t
Test plan:
Run t/db_dependent/Members_Attributes.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:57 +00:00
99672d1f00 Bug 16155: Composite keys in TestBuilder and more
The number of tests using TestBuilder is gradually growing. Once you
are familiar with its use, you will appreciate it and find yourself
using it when writing new tests. Although it works quite well, some details
still needs some polishing.

While looking at the handling of composite keys in TestBuilder, a number
of related points came up too. This patch finally ended up in setting
the following design goals:

[1] TB should not only look at the first column of a composite FK.
[2] TB should optionally create records for fixed FK values.
[3] TB should create a new record, never change an existing record.
[4] TB should respect auto_increment columns.
[5] Passing a hash for a foreign key should always imply a new record.
[6] Explicitly passing undef for a column should mean NULL.
[7] Add a delete method; it will replace the clear method.
[8] Simplify by removing $default_values, hash key _fk and param only_fk.

The comments below further clarify these points. Note that they refer to
the old behavior (before this patch) unless stated otherwise.

Comments for point 1
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => { flag => 'my flag' },
            code => 'will_be_ignored',
        },
    });
Module_bit and code here are a composite FK to permissions.
TB ignores the value for the code column here and creates a record with a
randomized code.

But if we would pass the hash in the second column of this compound key like:
            borrowernumber => $borrowerno,
            module_bit => 10,
            code => { code => 'new_code' },
TB would now crash when passing the hash for code thru to DBIx.

Comments for point 2
====================
Look at:
    $builder->build({
        source => 'UserPermission',
        value => {
            borrowernumber => $borrowerno,
            module_bit => 99,
            code => 'new_super_tool',
        },
    });
TB detects a fixed value for the module_bit, continues and will crash on
DBIx if the foreign keys are not found. In this case it would be friendly
to create a missing linked record.

Comments for point 3
====================
Look at:
    $builder->build({
        source => 'Branch',
        value  => { branchcode => 'CPL' },
    });
If this branch already exists, this call would modify the branch record and
overwrite all columns with randomized values (expected or not). In any case,
it would be safer here to return undef than modifying an existing record.

Comments for point 4
====================
Look at:
    $builder->build({
        source => 'Borrower',
        value  => { borrowernumber => '123456789' },
    });
If this number would exist, we would update (as earlier). But if this
number does not exist, we would create the record. Although that is
technically possible, I would prefer to have TB respect the auto
increment property of this column.

Comments for point 5
====================
Look at:
    $builder->build({
        source => 'Item',
        value  => { homebranch => { branchcode => 'MPL' } },
    });
As discussed under point 3, we should actually not pass primary key values
if we expect to build new records. The same also holds for the deeper
(recursive) calls to build when using hashes like here for homebranch.
In this case again an existing record might be overwritten.

Comments for point 6
====================
Look at:
    $builder->build({
        source => 'Reserve',
        value => { itemnumber => undef },
    });
As you know, a reserve without an itemnumber is a biblio level hold.
Unfortunately, TB did not care about passing undefs until now. So you would
get an item level hold.
In the new situation TB will respect these explicit undefs, as long as the
corresponding foreign key column is nullable of course.

Comments for point 7
====================
This patch will allow you to delete records created by TB:
    my $patron = $builder->build({ source => 'Borrower' });
    $builder->delete({ source => 'Borrower', records => $patron });
Or:
    $builder->delete({ source => 'Borrower', records => [ $patron, ... ] });
For safety, delete requires you to provide all primary key values in the
passed hashref(s).
Deleting all records in a table via clear is no longer supported and can
still be arranged in one statement.

Comments for point 8
====================
Current use of TestBuilder reveals that $default_values and only_fk
are not really needed. The current $default_values should imo not be in the
module anyway; if you want to use it, you could still pass it to TB:
    $builder->build({ ..., value => { %defa, %your_values } });

Only_fk stops at the very last step of saving the top level record while
storing all linked records at the lower levels. Practical use is not
very obvious; it can be easily simulated by one delete statement.

The hash key _fk is now used to store all linked records one or more levels
down. It is used in a few tests to retrieve a value one level down.
Why not retrieve that one value via the database and get rid of the
whole structure?

Implementation
==============
This highlights the main changes:

The $default_value hash (with some hardcoded values for UserPermission)
is removed from the module. It was used by a test in TestBuilder.t and has
been relocated.
The value of $gen_type is returned now by sub _gen_type. (See new.)

The main change in the build method is moving the foreign keys logic to a
new subroutine _create_links. This routine now looks at all columns of a
composite FK. It checks if a linked record exists for passed values, and
it looks at NULL values.

Routine _buildColumnValues is slightly adjusted to allow for passed undef
values. The theoretically endless loop is replaced by three tries. For
composite unique constraints we only check complete sets of values.

Routine _getForeignKeys contains a check to not return the same relation
twice in case of doubled belongs_to relations in the DBIx scheme.

The eval in _storeColumnValues is removed. The autoincrement check in sub
_buildColumnValue got more priority; the handling of foreign keys has been
adjusted and a check for not-nullable columns has been added.

TEST PLAN
=========
This patch only deals with the TestBuilder module itself. In the follow-up
patches TestBuilder.t and a few other unit tests are adjusted.

[1] Do not yet apply this patch. But apply the 'OldBehavior' patch.
    Verify that all tests pass. (They cover the first six design goals.)
[2] Apply this patch. Does TestBuilder still compile (perl -c)?
[3] Run the OldBehavior test again. Do all tests fail now? This means
    that we got rid of all unwanted side-effects in the list of goals.
[4] Run some other tests that use TestBuilder. (See below.)
    Skip the following tests; a follow-up patch deals with them.
    t/db_dependent/Accounts.t
    t/db_dependent/Barcodes.t
    t/db_dependent/Circulation/AnonymiseIssueHistory.t
    t/db_dependent/Circulation/CalcFine.t
    t/db_dependent/Holds.t
    t/db_dependent/Items/MoveItemFromBiblio.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Members.t
    t/db_dependent/TestBuilder.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
The following tests pass:
    t/db_dependent/Acquisition/OrderUsers.t
    t/db_dependent/Auth/haspermission.t
    t/db_dependent/Barcodes_ValueBuilder.t
    t/db_dependent/Budgets.t
    t/db_dependent/Category.t
    t/db_dependent/Circulation.t
    t/db_dependent/Circulation/GetTopIssues.t
    t/db_dependent/Circulation/IsItemIssued.t
    t/db_dependent/Circulation/IssuingRules/maxsuspensiondays.t
    t/db_dependent/Circulation/Returns.t
    t/db_dependent/Circulation/TooMany.t
    t/db_dependent/Circulation_dateexpiry.t
    t/db_dependent/Circulation_transfers.t
    t/db_dependent/CourseReserves.t
    t/db_dependent/Creators/Lib.t
    t/db_dependent/DecreaseLoanHighHolds.t
    t/db_dependent/Exporter/Record.t
    t/db_dependent/Holds/LocalHoldsPriority.t
    t/db_dependent/Holds/RevertWaitingStatus.t:
    t/db_dependent/HoldsQueue.t
    t/db_dependent/Holidays.t
    t/db_dependent/Items.t
    t/db_dependent/Items_DelItem.t
    t/db_dependent/Koha/Acquisition/Currencies.t
    t/db_dependent/Koha/Authorities.t
    t/db_dependent/Koha/BiblioFrameworks.t
    t/db_dependent/Koha/Cities.t
    t/db_dependent/Koha/Libraries.t
    t/db_dependent/Koha/Objects.t
    t/db_dependent/Koha/Patron/Categories.t
    t/db_dependent/Koha/Patron/Images.t
    t/db_dependent/Koha/Patron/Messages.t
    t/db_dependent/Koha/Patrons.t
    t/db_dependent/Koha/SMS_Providers.t
    t/db_dependent/Koha_template_plugin_Branches.t
    t/db_dependent/Letters.t
    t/db_dependent/Members/AddEnrolmentFeeIfNeeded.t
    t/db_dependent/Members/GetUpcomingMembershipExpires.t
    t/db_dependent/Members_Attributes.t
    t/db_dependent/Patron/Borrower_Debarments.t
    t/db_dependent/Patron/Borrower_Discharge.t
    t/db_dependent/Patron/Borrower_Files.t
    t/db_dependent/Ratings.t
    t/db_dependent/Reports_Guided.t
    t/db_dependent/Reserves/GetReserveFee.t
    t/db_dependent/Review.t
    t/db_dependent/Serials_2.t
    t/db_dependent/ShelfBrowser.t
    t/db_dependent/Virtualshelves.t
    t/db_dependent/api/v1/patrons.t

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-04 13:47:57 +00:00
5657e262b7 Bug 16408: Fix UsageStats.t
Bug 16167 removed some prefs. This impacts the number of tests.

Test plan:
Run the test.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:47:02 +00:00
3c67f1d9a5 Bug 16423: [QA Follow-up] Remove $opac
It is not used. And even may raise an uninitialized warnings.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Test still passes :)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:33:00 +00:00
05bab15841 Bug 16423: (followup) Add more tests
On fixing this tests, we retrieved the JSON data for the datatable used on
the 'Manage staged records' page. It would be cool to check the data it
carries makes sense.

To test:
- Run t/db_dependent/www/batch.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

All test pass
prove t/db_dependent/www/batch.t
t/db_dependent/www/batch.t .. ok
All tests successful.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:33:00 +00:00
635961dcdb Bug 16423: Fix t/db_dependent/www/batch.t so it matches new layout
This patch adjusts batch.t so it matches the use of datatables on the
'Manage staged records' page, and small layout changes already fixed
on search_utf8.t.

The tests are slightly modified so they actually test interesting stuff.
Some were passing only because an undefined value was passed.

To test:
- On master, run
  $ prove t/db_dependent/www/batch.t
=> FAIL: Tests fail notably
- Apply the patch
- Run:
  $ prove t/db_dependent/www/batch.t
=> SUCCESS: Notice tests now pass.
- Sign off :-D

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:33:00 +00:00
93ef7e1c13 Bug 16419: follow-up of bug 11371 - Fix t/db_dependent/Acquisition.t
The tests added by bug 11371 have been put after the rollback statement.

Test plan:
  prove t/db_dependent/Acquisition.t
should return green

Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Tests fail before the patch, and pass with it. QA scripts like the patch too.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:30:42 +00:00
671f81e877 Bug 16407: Simplify comments
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:28:23 +00:00
d8d4277471 Bug 16407: Fix Koha_borrower_modifications.t
This test was using hardcoded borrower number, assuming they should be
present. Now we use TestBuilder.

Test plan:
Run the test.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Test pass before and after patch.
No errors

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:28:22 +00:00
27b2f04ad9 Bug 16403: Remove unnecessary undef param
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:25:13 +00:00
073a6bda11 Bug 16403: Fix holds.t (tests 9 and 39)
Resolves this:

Failed test 'GetReservesFromItemnumber should return a valid borrowernumber'
at t/db_dependent/Holds.t line 97.
          got: '2000001890'
     expected: '2000001889'
Failed test 'Test AlterPriority(), move to bottom'
at t/db_dependent/Holds.t line 220.
          got: '4'
     expected: '5'

Test plan:
Run the test before and after applying this patch.

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>
2016-05-03 15:25:13 +00:00
cf7052a378 Bug 16404: Fix Circulation/Branch.t (tests 8-11)
Resolves:
Failed test 'GetBranchitem returns holdallowed and return branch'
at t/db_dependent/Circulation/Branch.t line 226.
     Structures begin differing at:
          $got->{hold_fulfillment_policy} = 'any'
     $expected->{hold_fulfillment_policy} = Does not exist
Failed test 'Without parameters GetBranchItemRule returns the values in default_circ_rules'
at t/db_dependent/Circulation/Branch.t line 234.
     Structures begin differing at:
          $got->{hold_fulfillment_policy} = 'any'
     $expected->{hold_fulfillment_policy} = Does not exist
Failed test 'With only a branchcode GetBranchItemRule returns values in default_branch_circ_rules'
at t/db_dependent/Circulation/Branch.t line 239.
     Structures begin differing at:
          $got->{hold_fulfillment_policy} = 'any'
     $expected->{hold_fulfillment_policy} = Does not exist
Failed test 'With only one parametern GetBranchItemRule returns default values'
at t/db_dependent/Circulation/Branch.t line 244.
     Structures begin differing at:
          $got->{hold_fulfillment_policy} = 'any'
     $expected->{hold_fulfillment_policy} = Does not exist

Test plan:
Run test before and after applying this patch.

Signed-off-by: Marc Véron <veron@veron.ch>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Note: I do not see the usefulness of the lazy_any variable, I don't
think it make things easier to read.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:21:31 +00:00
6e0b6ecf13 Bug 16170: [QA Follow-up] Fix embeded typos
Just noticed this typo three times here.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:01:13 +00:00
1608b89b7f Bug 16170: [QA Follow-up] Adjust Items.t
Items.t inserts an item via biblioitem not correctly linked to biblio.
The new foreign key constraint does not allow that.
Actually, we should be choosing to either remove biblioitems from Koha or
remove biblionumber from items.

Note: This seems to be the only case where an item is added this way.

Test plan:
Run Items.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-05-03 15:01:13 +00:00
68dbe8415f Bug 14632: Add Copyright for the Koha Dev Team
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

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

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 18:06:40 +00:00
Mark Tompsett
a2aba3c86f Bug 14362: Regression tests
This should trigger the error. Attempts to shift system time
zones did not make sense as to the number of failures.

Added Time::Fake dependency, if it isn't installed these extra
tests don't run. There is a nice skip message about it.

Added License text.

TEST PLAN
---------
 1) apply test patch
 2) sudo dpkg-reconfigure tzdata
    -- set your system time to GMT (Africa/Abidjan)
 3) prove t/Circulation/AgeRestrictionMarkers.t
    -- should not fail, even if you change system
       time to any time.
 4) sudo dpkg-reconfigure tzdata
    -- set your timezone to Eastern
 5) sudo date -s"2015-06-18 21:15:00"
 6) date
    -- should be past 9pm Eastern timezone
 7) prove t/Circulation/AgeRestrictionMarkers.t
    -- kaboom!
 8) sudo date -s"2015-06-18 12:00:00"
 9) date
    -- should be noon Eastern timezone
10) prove t/Circulation/AgeRestrictionMarkers.t
    -- success?! Time sensitive tests are bad tests.
11) sudo apt-get install libtime-fake-perl
12) prove t/Circulation/AgeRestrictionMarkers.t
    -- kaboom!
    -- changing timezone to anything other than GMT
       should trigger a kaboom.
13) apply fix patch
14) prove t/Circulation/AgeRestrictionMarkers.t
    -- should work all the time.
15) less t/Circulation/AgeRestrictionMarkers.t
    -- the license text should be similar to
       http://wiki.koha-community.org/wiki/Coding_Guidelines#Licence
16) koha qa test tools.

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 18:06:40 +00:00
51be8ecd9d Bug 14834: Make membership_expiry cronjob more flexible
This patch adds three parameters to the cron job: -before and -after, and
-branch.

You can run the cronjob now in an adjusted frequency: say once a week with
before 6 or after 6 (not both together). If your pref is set to 14, running
before=6 will include expiries from 8 days to 14 days ahead. When you
use after=6, you would include 14 days to 20 days ahead, etc.

You could also rerun the job of yesterday by setting before=1 and after=-1;
this could help in case of problem recovery.

Obviously, the branch parameter can be used as a filter.

NOTE: Why are these parameters passed only via the command line?
Well, obviously the branch parameter is not suitable for a pref.
The before/after parameter allows you to handle expiry mails different from
the normal scheme or could be used in some sort of recovery. In those cases
it will be more practical to use a command line parameter than editing a
pref.

NOTE: The unit test has been adjusted for the above reasons, but I also
added some lines to let existing expires not interfere with the added
borrowers by an additional count and using the branchcode parameter.

Test plan:
[1] Run the adjusted unit test GetUpcomingMembershipExpires.t
[2] Set the expiry date for patron A to now+16 (with pref 14).
    Set the expiry date for patron B to now+11.
[3] Run the cronjob without range. You should not see A and B.
[4] Run the cronjob with before 3. You should see patron B.
[5] Run the cronjob with before 3 and after 2. You should see A and B.
[6] Repeat step 5 with a branchcode that does not exist. No patrons.

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Work as described following test plan.
Test pass
No errors

New parameters work with one (-) or two(--) dashes, no problem
with that but convention suggest that 'long' options use two-dashes.

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 16:57:51 +00:00
6b28a47e8c Bug 16320: [QA Follow-up] Fix borrower attribute test in ILSDI_Services
Since bug 12267 removed the password option, this test should be
adjusted.

Test plan:
Run the test.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 16:02:53 +00:00
4a6fe346f0 Bug 16320: Refactor ILSDI_Services.t so it uses TestBuilder
This patch refactors the tests so they are corerctly built using TestBuilder
and wrapper inside a DB transaction in a less ambiguous way.

To test:
- Verify that the tests pass with the patch:
- Run:
  $ prove t/db_dependent/ILSDI_Services.t
=> SUCCESS: Tests pass
- Sign off :-D

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 16:02:53 +00:00
Magnus Enger
c474b9fd30 Bug 16316 - QA FOLLOWUP - Add tests
Add 6 tests for searching by surname.

To test, run: prove t/db_dependent/Utils/Datatables_Members.t

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 15:53:25 +00:00
85e7d186ec Bug 16167: Remove Authorised value images prefs
There are 2 prefs to drive this feature: StaffAuthorisedValueImages and
AuthorisedValueImages. AuthorisedValueImages is not added by
sysprefs.sql and does not appear in updatedatabase.pl, we could easily
imagine that nobody uses it.

With XSLT enabled, the feature is only visible on a record detail page
at the OPAC, if AuthorisedValueImages is set. Otherwise you need to turn
the XSLT off. In this case you will see the images on the result list
(OPAC+Staff interfaces) and OPAC detail page, but not the Staff detail
page.

This patch suggests to remove completely this feature as it does not
work correctly.

The ability to assign an image to an authorised value is now always
displayed, but the image will only be displayed on the advanced search
if defined.

Test plan:
Confirm that the authorised value images are no longer visible at the
opac and the staff interfaces.
The prefs should have been removed too.

Signed-off-by: Owen Leonard <oleonard@myacpl.org>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:59:58 +00:00
792017e548 Bug 15956: New unit test SIP/Message.t
This patch adds a unit test for the handlers of Patron Status
and Patron Info requests. It can be extended with test for the
other handlers later (hopefully).

Note: The tests have been written for SIP protocol version 2.

Test plan:
Run t/db_dependent/SIP/Message.t.

Note: If bug 13871 has not yet been pushed, test 12 (subtest 1) should fail
with something like 'Code AF not found'. Same for test 15 of subtest 2.
That is no problem; after that push, all tests should pass.

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

All tests pass
t/db_dependent/SIP/ILS.t .......... ok
t/db_dependent/SIP/Message.t ...... ok
t/db_dependent/SIP/Patron.t ....... ok
t/db_dependent/SIP/Transaction.t .. ok
All tests successful.

No errors

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:38:33 +00:00
f79ef42b58 Bug 15956: Move SIPILS.t to SIP/ILS.t
Move the file. Some trivial (cosmetic) edits: whitespace, comments.

Test plan:
Run SIP/ILS.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:38:33 +00:00
7ce9008a00 Bug 15956: Rearranging some SIP unit tests
The original SIP_ILS.t is split up into SIP/Patron.t and
SIP/Transaction.t.
The assumption on a hardcoded existing and non-existing card number is
replaced by adding and deleting a card number (:patron) with TestBuilder.

Test plan:
Run SIP/Patron.t
Run SIP/Transaction.t

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:38:33 +00:00
Jesse Weaver
97d988b54f Bug 15531: (followup) Use a quantity of 1, not null, for standing orders
This seems to cause fewer problems with the existing acquisitions code.

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:07:18 +00:00
Jesse Weaver
9501ac2fef Bug 15531: Add support for standing orders
This allows creation of special baskets that include standing orders.
These orders do not have a known quantity (and may not have a known
price in advance). Upon receipt, the received items are split into a new
completed order.

Test plan:
  1) Run updatedatabase.pl.
  2) Run prove t/db_dependent/Acquisition/StandingOrders.t . (and the
     other Acquisition tests).
  3) Create a new basket, mark it as a standing order basket.
  4) Add an order to this basket, and notice that the quantity field is
     missing (and thus not required).
  5) Receive items for this order, and notice that the original order is
     unchanged. The new child order line should have the correct price
     and quantity information.

(Note: the QA tools output what seems to be a spurious spelling error
for Test::More's "isnt" in StandingOrders.t.)

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:07:17 +00:00
dbba8cfeba Bug 15009 [QA Followup] - Update number of tests run
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:01:07 +00:00
9c5b819100 Bug 15009: QA follow-up
This patch adds a test to check the unicity of auth cats, simplify
the GetBudgetAuthCats subroutine and make it return an arrayref of scalar
instead of an arrayref of hashref with only 1 key.

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 13:00:21 +00:00
charles
1a9ca84293 Bug 15009 - Adds unit tests for C4::Budget::GetBudgetAuthCats
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>
2016-04-29 12:59:40 +00:00
Mark Tompsett
fdda5d7d7c Bug 14144: Clean ups and refactors
perltidy
old new calls to new call format
attrType -> attr_type (perlcritic friendlier)
double quotes to single quotes
'' combinations to q{} (perlcritic friendlier)
refactored parameters to mock_net_ldap_search into a HASH

This piece is not necessary, but I think it is nicer.

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as expected. No koha-qa erros

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 12:40:16 +00:00
Mark Tompsett
b140c4b2fd Bug 14144: Silence warnings t/db_dependent/Auth_with_ldap.t
Revisiting this bug, I noticed that C4::Context->config() was mocked
poorly. I expanded the mock function a little and have quieted the
test which seemed to be working in a broken manner before.

TEST PLAN
---------
1) prove t/db_dependend/Auth_with_ldap.t
   -- shibboleth noise
2) apply this patch
3) prove t/db_dependend/Auth_with_ldap.t
   -- no noise
4) run koha qa test tools

Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
2016-04-29 12:40:16 +00:00