The check methods were positioned under the 'Internal methods' section
of the meodule but are used externally.
It also felt strange to have a noop or die method. Instead, I propose
renaming them to `repeatable_ok` and `unique_ok` and returning a
boolean denoting their state.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The current implementation of store+check_repeatable+check_unique_id
notably retrieves the related Koha::Patron::Attribute::Type object three
times.
This can be easily solved by retrieving it once and reusing. This patch
does that.
This changes the signature for the helper methods.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron/Attribute.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests pass!
4. Verify the old _check_repeatable method is not used anywhere
$ git grep _check_repeatable
=> SUCCESS: It is not!
5. Verify check_unique_id is not used anywhere, so no risk changing the
signature
$ git grep check_unique_id
=> SUCCESS: It is safe to update it!
6. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In the case a non-repeatable attribute for a patron is being updated
(its value) the routines that check the type is repeatable should
exclude its own id from the search for things to work.
This patch solves that.
To test:
1. Apply the regression tests patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron/Attribute.t
=> FAIL: Tests explode! An unexpected exception is thrown!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
On using this dev for the API, I found that the original FKConstraint
exception was too generic, and could be confused with those risen by
Koha::Patron->store. So this patch changes the exception that is thrown
in Koha::Patron->extended_attributes
The change is reflected on the test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass, and they include the new exception
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the search for mandatory attribute types actually use
'code' instead of 'class' for comparisson.
Tests are added to cover this behavior.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://bugs.koha-community.org/show_bug.cgi?id=27587
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds handling for globally mandatory extended attributes and
the corresponding unit tests.
To test:
1. Apply the patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the extended_attributes method use
Koha::Patron->add_extended_attribute inside the loop, instead of direct
DB access. This way, it takes advantage of the validation that is done
there.
All happens inside a transaction, so if 'add_extended_attribute' throws
an exception, everything is rolled back.
To test:
1. Apply the previous patches
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t
=> FAIL: The behavior is not the correct one, tests fail
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds regression tests for the changed behavior.
Koha::Patron->extended_attributes will now throw exceptions and rollback
on invalid attributes.
The existing tests are amended so they don't try to deal with skipping
bad attributes.
Specific subtests are added for error situations.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t
=> FAIL: Tests fail because the new behavior is not implemented.
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
They really belong here. both Patron.t and Patrons.t got lengthy and it
is not intuitive to look for tests for a Koha::Patron method in the
Patrons.t file.
To test:
1. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron.t \
t/db_dependent/Koha/Patrons.t
=> SUCCESS: Tests pass!
2. Apply this patch
3. Repeat 1
=> SUCCESS: Tests still pass, no new stuff
4. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This trivial patch adds a new exception for more fine-grained control on
the errors when dealing with patrons and their extended attributes.
To test:
1. Apply this patches
2. Run:
$ kshell
k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds type validation to the ->store method. And exception
should be thrown if the type is invalid. Things currently explode in
such situations, so this doesn't change the behavior (just adding more
informative failure information).
To test:
1. Apply the previous patches
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron/Attribute.t
=> FAIL: No exception is thrown on the bad case, weird error
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass! Exception thrown!
5. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch introduces unit tests for the new exception to be thrown.
It generates a new extended attribute type, keeps the randomly generated
'code' and deletes the object. This way we can be sure the code doesn't
exist on the DB. It then tries to generate an attribute of that type, to
force the exception to be thrown.
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch renames the tests file, and also groups subtests by method.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/db_dependent/Koha/Patron/Attribute.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Working on bug 27857 showed we needed one more exception, for when the
passed attribute type is invalid.
This patch adds the Koha::Exceptions::Patron::Attribute::InvalidType
exception. It adds a format for stringifying it, and tests are added.
The previous exceptions tests lacked one case, so I add them on this
patch as well.
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove tt/Koha/Exceptions.t
=> SUCCESS: Tests pass!
3. Sign off :-D
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the 'check_unique_id' and '_check_repeatable' methods
pass the 'attribute' parameter when throwing an exception. This way we
can provid emore meaningfull error messages.
To test:
1. Apply the previous patches
2. Run
$ kshell
k$ prove t/db_dependent/Koha/Patron/Attributes.t
=> FAIL: The exceptions are not passed the attribute!
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds a 'field' to this exceptions. They now have an
'attribute' parameter referencing the object itself,
To test:
1. Apply this patch
2. Run:
$ kshell
k$ prove t/Koha/Exceptions.t
=> SUCCESS: Tests pass!
3. Sign off!
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Fixes the checkbox label by adding an id, so that when you click on
"Show only subscriptons" the checkbox will be checked.
Adds standard classes to the existing "Merge selected invoices" button
to make things more consistent. Also added the fa-compress icon that
we use in cataloguing on the merge records button.
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Rebase add comma
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Bug 22773: The deprecated plugin is removed
Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Christopher Kellermeyer <ckellermeyer@altadenalibrary.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Remove asset for removed js
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Since bug 8338 the recalcuated zero amount fine is subsequently removed
in _FixOverduesOnReturn which causes this test to fail if finesMode is
'production' and 'CalculateFinesOnReturn' is enabled.
Perhaps we should be 'VOID'ing fines rather than completely deleting
them, but that's a question for another day.
Test plan
1/ Enable finesMode in the staff client
2/ Run the test and watch it fail
3/ Apply the patch
4/ Run the test and watch it pass
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Apply all other patches and dependencies
2 - prove -v t/db_dependent/Circulation.t
3 - It fails
4 - Apply this patch
5 - It passes
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Cover the case of multiple patrons and multiple items that can fill them
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
The current call checks for truth against a returned hash, we need
to check the 'status' value
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In the case of 'AllowRenewalIfOtherItemsAvailable' we check all existing reserves against
all existing items. This patchset reduces the number of DB/subroutine calls
To test:
1 - Apply patch
2 - prove -v t/db_dependent/Circulation.t
Signed-off-by: Amit Gupta <amitddng135@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds an optional param 'ignore_hold_counts' to the routine, while still forbidding holds when 0 are allowed
To test:
1 - prove -v t/db_dependent/Holds.t
Signed-off-by: David Nind <david@davidnind.com>
JK: Commit message amended: Fixed title formatting
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds an atomic update file that takes care of translating
existing rules. It constrains the rules to translate to those currently
expected by Koha and leaves any other value out.
To test:
1. Have some manually added rules:
$ koha-mysql kohadev
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', -1);
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', 1);
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', 2);
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', 3);
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', 0);
> INSERT INTO circulation_rules (rule_name,rule_value) VALUES
('holdallowed', 4);
2. Apply this patch
3. Run:
$ updatedatabase
=> SUCCESS: It doesn't explode
4. Verify the created rules were updated correctly:
> SELECT * FROM circulation_rules WHERE rule_name='holdallowed';
5: Verify all the tests that dealt with this rule still pass!
$ kshell
k$ git diff origin/master --name-only | grep -e '\.t$' | \
xargs prove
=> SUCCESS: Tests pass!
6. Verify the UI handles setting things correctly
7. Sign off :-D
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
-PLace a hold and add a reserve note to that hold
-Go to Circulation > Holds to pull
-Notice there is no reserve notes column
-Apply patch and restart all the things
-Reload the Holds to pull report and you should see a reserve notes column
-Go to table settings (/cgi-bin/koha/admin/columns_settings.pl) and make sure you can properly hide the column.
-Test other columns in the table and make sure you can properly hide them
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Petro Vashchuk <stalkernoid@gmail.com>
Signed-off-by: Joonas Kylmälä <joonas.kylmala@helsinki.fi>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch takes advantage of the previous changes, and makes the
select2 dropdown display a warning sign (with a tooltip) on pickup
locations that, if chosen, it would mean a circ rule would be
overridden.
To test:
1. Have some pickup locations that are not valid for existing holds and
AllowHoldPolicyOverride set to "Don't allow"
2. Visit a biblio with some holds
3. Choose a hold that cannot use some pickup location and use the
dropdown
=> SUCCESS: Only valid pickup locations are retrieved
4. Set Allow HoldPolicyOverride to "Allow"
5. Refresh the page (I usually click on the Holds tab)
6 .Repeat 3
=> SUCCESS: All pickup locations in the system are displayed
=> FAIL: No visual feedback on overridden pickup locations :-(
7. Apply this patch and refresh the page
8. Repeat 3
=> SUCCESS: All pickup locations in the systema are displayed
=> SUCCESS: Visual feedback on overridden pickup locations is shown!
9. Set AllowHoldPolicyOverride to "Don't allow"
10. Repeat 3
=> SUCCESS: Only valid pickup locations are retrieved
=> SUCCESS: No overrridden warn/symbol, as expected
11. Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch makes the controller for the route, return all valid pickup
locations (i.e. pickup_location => 1) when AllowHoldPolicyOverride is
set to 'Allow', but also adds a calculated attribute: 'needs_override'
so the consumer knows the specific pickup location needs an override
(and thus be able to provide visual feedback on a single run).
To test:
1. Apply the regression tests
2. Run:
$ kshell
k$ prove t/db_dependent/api/v1/holds.t
=> FAILURE: Tests fail, the change is not implemented
3. Apply this patch
4. Repeat 2
=> SUCCESS: Tests pass!
5. Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
This patch adds tests for the changed behavior.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
In Elasticsearch mapping tables :
In order to better see which row is currently in drag-an-drop,
add an highlight color like in circulation and fine rules when editing a row.
Test plan :
1) Go to Administration > Search engine configuration (Elasticsearch)
2) In each table : biblio, authorities, facets
3) Clik on a row to start a drag => See row turning orange
4) Release to drop => See row turning back to original color
https://bugs.koha-community.org/show_bug.cgi?id=26970
Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
To test:
1 - Apply patch
2 - Restart all
3 - Place an item level hold for a patron via the staff client
4 - Cancel the hold using the sip_cli_emulator with hold-mode: -
perl misc/sip_cli_emulator.pl -a localhost -p 6001 -su term1 -sp term1 --patron {cardnumber} --item {BARCODE} -m hold -l CPL --hold-mode -
5 - Run the sip_cli_emulator with no params
6 - Verify help text is understandable
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
