This patch separates Swagger-specifications and the minifySwagger.pl from other
api-files by moving specifications & minifier into api/v1/swagger.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Since we have defined some basic x-primitives in x-primitives.json, we can now
start to reuse them in our currently defined objects.
To test:
1. Apply patch
2. Run minifySwagger.pl
3. Validate your Swagger specifications
4. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Currently it is not possible to define multiple types for primitive definitions
in /definitions/*. If you try to use the following
"firstname": {
"type": ["string", "null"],
"description": "patron's first name"
}
in definitions.json, online.swagger.io validator will not validate it:
{"messages":["attribute definitions.firstname.type is not of type `string`"]}
One way to get around this issue is to extend definitions with custom
"x-primitives" object, where we will define all reusable primitive definitions.
To test:
1. Add the "firstname" example above to definitions.json
2. Run minifySwagger.pl
3. Validate your specification
4. Observe that error with description mentioned above is given
5. Apply patch
6. Repeat step 2 and 3
7. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
These definitions had indentation of 4 spaces, while rest of the specification
uses 2 spaces. This patch simply maintains the consistency in indentations and
provides no other modifications to code.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The borrowernumber as a query parameter should be defined in parameters.json
in order to allow its reusability.
To test:
1. Apply patch
2. Run minifySwagger.pl
3. Validate swagger.min.json in online.swagger.io/validator/debug?url=url_to+
_your_swagger_min_json or your local swagger-api/validator-badge validator
4. Observe that validation passes
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Parameters and paths should be split in our Swagger specification, because
otherwise swagger.json would become messy with all the paths and their
further specification in the same file. Also parameters should be split
for the same reason.
Instead of using index.json for definitions, parameters and paths, we define
new files "definitions.json", "parameters.json" and "paths.json" in order to
simplify the references. If we kept using index.json and try to reference
"/definitions/error.json" from "/paths/holds.json", reference would be
"../definitions/index.json#/error" instead of now simplified version,
"../definitions.json#/error".
Here is the proposed structure:
.
├── swagger.json
├── definitions.json
├── paths.json
├── parameters.json
├── definitions
│ └── error.json
│ └── patron.json
├── parameters
│ └── patron.json
├── paths
│ └── patrons.json
├── minifySwagger.pl
└── swagger.min.js
The swagger.json paths, definitions and parameters will look as follows:
...
"paths": {
"$ref": "paths.json"
},
"definitions": {
"$ref": "definitions.json"
},
"parameters": {
"$ref": "parameters.json"
}
...
A problem with splitting specification into multiple files directly from
swagger.json (e.g. "paths": { "$ref": "paths.json" }) is that it is not
following the Swagger specification and an error will be thrown by the
Swagger-UI default validator (online.swagger.io/validator).
To overcome this problem, we use the minifySwagger.pl script from Buug 16212.
This allows the developers to work with the structure introduced in this patch
thus allowing developers to split the specification nicely, and still have a
valid Swagger specification in the minified swagger.min.json.
To test:
-2: Apply the minifier-patch in Buug 16212.
-1: Make sure you can validate your specification with Swagger2 validator at
online.swagger.io/validator/debug?url=url_to_swaggerjson, or install it
locally from https://github.com/swagger-api/validator-badge.
1. Don't apply this patch yet, but first validate swagger.json
with swagger.io-validator (or your local version, if you installed it)
2. Observe that validation errors are given
3. Run minifySwagger.pl
4. Validate swagger.min.json with the validator you used in step 1
5. Observe that validation passes and we overcame the invalid specification
problem in swagger.min.json
6. Apply this patch
7. Run minifySwagger.pl
8. Repeat step 4
9. Observe that validation passes with new structure
10. Run REST tests at t/db_dependents/api/v1
(11. Study the new structure of our Swagger specifications :))
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
We participated in the development of the Mojolicious::Plugin::Swagger and know
it well. We have made an extension to the plugin to provide full CORS support
and have been building all our in-house features on the new REST API.
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch re-introduces the transaction into t/db_dependent/Items_DelItemCheck.t
and does some cleaning on the touched tests so they raise less warnings.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Remove $dbh as argument to C4::Items::DelItemCheck
and C4::Items::ItemSafeToDelete, also change all
calls to these functions throughout the codebase.
Also remove remaining reference to 'do_not_commit' in
t/db_dependent/Items_DelItemCheck.t
Fixed doubled "$$" in C4/ImportBatch.pm
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Use t::lib::TestBuilder in t/db_dependent/Items_DelItemCheck.t
Remove the option 'do_not_commit' from C4::Items::DelItemCheck.
Whitespace cleanup.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
* Fix POD warning.
* Remove redundant 'use stric' and 'use warnings'
* Remove $VERSION and --version option.
* Remove --dry-run option
* Split test for --help and check for @criteria into two separate pod2usage calls,
enabling -msg on the latter.
* Fix 'target_tiems' typo.
* Test for holds on items to be deleted.
* Fix whitespace
* Fix test for holds.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
http://bugs.koha-community.org/show_bug.cgi?id=14504
Signed-off-by: Heather Braum <hbraum@nekls.org>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Our current approach with splitting the Swagger specification brings problems.
For example, in swagger.json, if we split the whole "definitions" object with
$ref, validations will fail with online.swagger.io validator.
(See http://online.swagger.io/validator/debug?url=url_to_your_swagger_json)
The problem also occurs with "paths" (Paths Object), because simply $ref to all
paths for example in /paths/index.json does not meet the Swagger2 specification.
The problem is solved by using the minification script and the minified version
of swagger.json after which the swagger.min.json is valid Swagger2 specification,
because the minifier resolves the problematic $refs of swagger.json file in the
minified version.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch introduces the misc/devel/minifySwagger.pl script that
loads the Swagger files, follows references and produces a compact
("minified") version of the swagger file which is suitable for
distribution.
The wiki page should be updated with instructions on how to regenerate
it so the Release Manager does it on each spec upgrade.
Signed-off-by: Olli-Antti Kivilahti <olli-antti.kivilahti@jns.fi>
My name is Olli-Antti Kivilahti and I approve this commit.
We have been using the Swagger2.0-driven REST API on Mojolicious for 1 year now
in production and I am certain we have a pretty good idea on how to work with
the limitations of Swagger2.0
Signed-off-by: Johanna Raisa <johanna.raisa@gmail.com>
My name is Johanna Räisä and I approve this commit.
We have been using Swagger2.0-driven REST API in production successfully.
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Exclude O, F and M when outstanding == 0.
Check if the issue_id points to a FU record.
Note: We only warn now when we see a second FU record with this issue id.
That should be a rare exception. As before, we are just counting it in
our total. Added a FIXME.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Tested fine on overdue. Renewed and backdated for a second fine. The F
and FU can be seen on the Fines tab and are totaled on Check out.
Signed-off-by: Jacek Ablewicz <abl@biblos.pk.edu.pl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Renewing an overdue would not work.
Log shows:
renew: Can't use string ("2144746608") as a HASH ref while "strict refs" in use at C4/Overdues.pm line 508., referer: /cgi-bin/koha/circ/circulation.pl?borrowernumber=1
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test Plan:
1) Find an overdue checkout with a fine
2) Renew item, note fine is not closed out (Account type F)
3) Apply this patch
4) Find another overdue checkout with a fine
5) Renew item, note fine is now correctly closed out
6) Backdate a checkout to be already overdue ( but not have a fine since
fines.pl hasn't run yet )
7) Renew item, note a closed out fine is created
Signed-off-by: Sean Minkel <sminkel@rcplib.org>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
By setting 'use Modern::Perl' some previously hidden warnings arised.
This patch removes them by testing the variable $balance for undef
before using it in a comparisson.
Sponsored-by: VOKAL
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes the bulk patron delete/anonymize functionality be limited
by branch. It does so by adding a branch selection dropdown and using the
already defined APIs for filtering by branch.
It makes use of C4::Branches::onlymine for the IndependentBranches use case
and it adds a way to call it from the Branches template plugin.
To test:
- Apply the patch
1) Have a superlibrarian user
- Go to Tools > Batch patron deletion/anonymization
=> SUCCESS: Verify you can pick a branch (or all of them)
- Try doing some operations
=> SUCCESS: Verify the selection is respected, and carried around all steps
2) Have a user with tools/delete_anonymize_patrons permissions
- Set IndependentBranches on
- Go to Tools > Batch patron deletion/anonymization
=> SUCCESS: It picks the librarian's branch and doesn't let us choose another one
- Try doing some operations
=> SUCCESS: Verify the user's branch is respected, and carried around all steps
- Sign off :-D
Sponsored-by: VOKAL
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
While the original patch fixes the issue, reading at Mojolicious source code, revealed
that Mojo::Transaction already has proper processing for detecting the remote address:
sub remote_address {
my $self = shift;
return $self->original_remote_address(@_) if @_;
return $self->original_remote_address unless $self->req->reverse_proxy;
# Reverse proxy
return ($self->req->headers->header('X-Forwarded-For') // '') =~ /([^,\s]+)$/
? $1
: $self->original_remote_address;
}
Without this patch, if there's a request without HTTP_X_FORWARDED_FOR, then the remote
address would be empty. Such would be the case of a dev/standard setup without Plack.
This patch makes Koha::REST::V1::startup use tx->remote_address instead of dealing with the
headers on its own.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Mojolicious does not set $ENV{REMOTE_ADDR} (neither $ENV{HTTP_*}) as
it may share ENV between different requests.
Fortunately for us, Plack does not!
This is a dirty patch to fix this issue but it seems that there is not
lot of solutions. It adds a remote_addr parameter to
C4::Auth::check_cookie_authin order to send it from
Koha::Rest::V1::startup reading the headers sent by Mojolicious.
Test plan:
Hit /cgi-bin/koha/mainpage.pl
Hit /api/v1/patrons/42
Hit /cgi-bin/koha/mainpage.pl
With this patch applied, everything will be fine and you won't be
logged out.
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Benjamin Rokseth <benjamin.rokseth@kul.oslo.kommune.no>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If a patron edit his/her details a second time, we need to delete the
first ones to avoid a "duplicate entry for key PRIMARY" error.
Test plan:
Log in at the OPAC
Edit your details
Edit them again
=> Without this patch, Koha will crash
=> With the patch applied, everything should work as expected
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Followed steps in comment #14, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Bob Birchall <bob@calyx.net.au>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The changes from opac-memberentry do not reach the table, since the
Patron::Modifications object does not work well.
Test Plan:
1) Apply this patch
2) Create some patron modification requests
3) Ensure you can approve and deny modifications
4) Ensure patron self registration works
Signed-off-by: Bob Birchall <bob@calyx.net.au>
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 15930 modified default value for DataTables patron search.
The doc text should also be modified :
"Can be 'contain' or 'start_with' (default value). Used for the
searchmember parameter."
Test plan :
- install Koha with patch
- look at man page man/man3/C4::Utils::DataTables::Members.3pm
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>
To verify:
- git grep itemissues
Result: Appears in C4/Circulation.pm only
To test:
- apply patch
- git grep itemissues
Expected result: No occurences
- prove t/db_dependent/Circulation.t
Expected result: Pass OK
Signed-off-by: Claire Gravely <c.gravely@arts.ac.uk>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
There is no need to change the previous logic here, so let's restore it.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This alternative patch moves logic and formatting to the template file.
To test:
* without patch
1/ find a patron with no lines in accountlines table : print summary shows no "account fines and payments" => OK
2/ find a patron with some lines in accountlines table and the total amount > 0 : print summary shows a table "account fines and payments" with fines to recover => OK
3/ find a patron with some lines in accountlines table but the total amount = 0 : print summary shows a table "account fines and payments" with nothing in it => NOK
* with the patch, same cases as before :
1/ same as without patch
2/ same as without patch
3/ print summary does not show "account fines and payments"
- Additionally, verify that formatting follows syspref 'CurrencyFormat'
- Verify that amount column is right-aligned
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>
When the TimeFormat system preference is set to "12 hour",
setting a filter on the due date can result in:
- no overdue loans being reported, even if there are some
that meet the criteria
OR
- overdue loans being omitted from the report if they
are due on the "until" date in the filter
This patch fixes this by replacing output_pref() with
DateTime::Format::MySQL to format the date filter values
to pass to the SQL query.
To test
-------
[1] Run the overdue report (circ/overdue.pl) and set a filter
on due date, using values that should bring up one or
more overdue loans.
[2] Note that zero overdue loans are returned (if using MySQL
5.5, 5.6, or 5.7 or MariaDB 5) or that loans due on the
"until" date are omitted (if using MarioDB 10).
[3] Apply the patch and repeat step 1. This time, the correct
set of overdue loans should be reported.
Signed-off-by: Galen Charlton <gmcharlt@gmail.com>
Signed-off-by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Add ident and Identifier-standard to known indexes in C4::Search::getIndexes().
Those indexes can be very useful, for example for IdRef feature.
Test plan :
- Make sure some records have a field indexed with Identifier-standard, ISBN=1234 for example
- Perform a search /cgi-bin/koha/opac-search.pl?idx=ident,phr&q=1234
=> you find the record
- Perform a search /cgi-bin/koha/opac-search.pl?q=ident:1234
=> Without patch : you get no results
=> With patch : you find the record
Idem for 'Identifier-standard'
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 16082, 'Display a friendly "patron does not exist" message if that's the
case - circulation.pl' changed the behavior of the checkout page so that if
you hit that page with a borrowernumber which doesn't exist a nice message will
tell you so.
Now when you do that you get an error message:
Can't call method "is_debarred" on an undefined value at /circ/circulation.pl line 300.
Test plan:
confirm that the friendly message is back :)
Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To duplicate frameworks, the code retrieve all the subfields, then
execute 1 insert per subfield.
It's unnecessary slow, we can use the DBMS to do it.
Test plan:
Create a new framework and duplicate the structure of another framework.
Signed-off-by: Marc <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
In patron acount ("your personal details" tab, which serves as a form
for submitting update requests as well) it's not possible to display
some account details like expiration date etc., without enabling
the field for branch / library update requests too.
To reproduce:
1) set OPACPatronDetails to "Allow"
2) clear PatronSelfModificationBorrowerUnwantedField preference
- all possible fields in the update requests form are visible
in patron account, including some extra details (card number,
expiration date, category) in the "Library" section on top
3) put 'branchcode' in PatronSelfModificationBorrowerUnwantedField preference
4) the whole 'Library' section disappears
To test:
1) apply patch
2) put 'branchcode' in PatronSelfModificationBorrowerUnwantedField,
'Library' section should remain visible (sans branch selection option)
3) put 'branchcode|cardnumber|datexpiry|categorycode' in the same
preference, 'Library' section should now disappear
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
One test of DecreaseLoanHighHolds.t does not pass,
A hashref representing a patron has a 'borrower' key defined, instead of
'borrowernumber'.
Test plan:
prove t/db_dependent/DecreaseLoanHighHolds.t
must return green
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Adds one exit statement, and some whitespace.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Verified deleting a patron again.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
If an attacker can get an authenticated Koha user to visit their page
with the url below, they can delete patrons details.
/members/deletemem.pl?member=42
Test plan:
0/ Do not apply any patches
1/ Adapt and hit the url above
=> The patron will be deleted without confirmation
2/ Apply first patch
3/ Hit the url
=> you will get a confirmation page
4/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> The patron will be deleted without confirmation
5/ Apply the second patch (this one)
6/ Hit /members/deletemem.pl?member=42&delete_confirmed=1
=> you will get a crash "Wrong CSRF token" (no need to stylish)
7/ Delete a patron from the detail page and confirm the deletion
=> you will be redirected to the patron module home page and the patron
has been deleted
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
It won't hurt to have a confirmation page when deleting a patron.
Moreover it's the more easy way to protect against CSRF attacks :)
Test plan:
Make sure you get a confirmation page when deleting a patron
Confirm that approving or denying the confirmation work as expected
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
