DBIx::Class does not provide such method, but it can be handy in some
cases.
Test plan:
prove t/db_dependent/Koha/Objects.t
should return green
Test returned green.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
For instance an issue is not fetch from its fk but using the fk
itemnumber.
We need to support them.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
On of the awesome things we will be able to do with the TT syntax is the support of plurals.
For instance we will be able to send a list of items, checkouts, etc. to the notice template.
That way we will get rid of our custom syntax like <<items.content>> or <item></item> for instance.
The existing code already has the playground for that but it is not used.
Basically the idea is to add a "loops" key which can contain a list of
object to retrieve from the DB and send to the template.
For instance:
loops => { overdues => [ $itemnumber_1, .., $itemnumber_N ] }
will send a variable "overdues" to the template. It will contain the
Koha::Checkout objects relative to the id passed.
There is one quite big inconvenient to this approach so far: since we
are still supporting the historical syntax, the objects can be fetch by
a script, then the script will send the id to GetPreparedLetter which
will refetch them.
This must be improved, but I suggest to do that later.
Test plan:
prove t/db_dependent/Letters/TemplateToolkit.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
Nothing new here since bug 17962, the AR_* notice messages are quite
simple. They send the article_request, patron, biblio, biblioitem, item and
library linked to the article request.
All the fields from these 6 tables should still be accessible using the
TT syntax.
Test plan:
Define TT notice templates for AR_PENDING, AR_PROCESSING, AR_COMPLETED
or AR_CANCELED.
You should manage to create a template to generate the same result as
the historical syntax.
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>
The 3 subroutines GetFieldMapping, SetFieldMapping and
DeleteFieldMapping from the C4::Biblio module were only used from the
field mappings admin page.
They can easily replaced with new packages Koha::FieldMappings based on
Koha::Object[s]
Test plan:
Add and delete field mappings (admin/fieldmapping.pl, Home ›
Administration › Keyword to MARC mapping).
Add an existing mapping > Nothing should be added
Note that this page has not been rewritten and you will not get any
feedbacks, but it's not the goal of this page to improve it.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Biblios.t
should return green
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Adds a internal routine to handle default values for the parameters
id and secret.
Also adds a parameter session_id for generate_csrf and check_csrf. This
session parameter is combined with the id parameter when generating or
checking a token.
Test plan:
Run t/Token.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Currently the CSRF token generated is based on the borrowernumber, and
is valid across user's session.
We need to restrict the CSRF token to the current session.
With this patch the CSRF token is generated concatenating the id
(borrowernumber) and the CGISESSID cookie.
Test plan:
Run t/Token.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
This patch makes Koha::Patron::Modifications->pending return
Koha::Patron:Attribute objects. They are not stored on the DB but only
live in memory on runtime.
members-update.pl is the only place this is used, and this way we have
all we need for better rendering the UI.
Tests are added for the changed API.
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>
This patch introduces tests for the required functionality.
To test:
- Run:
$ prove t/db_dependent/Koha/Patron/Modifications.t
=> FAIL: The current code doesn't work like that
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
https://bugs.koha-community.org/show_bug.cgi?id=13737
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
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>
This patch adds two methods to the Koha::Patron::Attribute:
- opac_display
- opac_editable
Both method just check the corresponding Koha::Patron::Attribute::Type
and return the values for those attributes. This is useful to avoid
checking that manually on the controller scripts.
To test:
- Run:
$ prove t/db_dependent/Koha/Patron/Attributes.t
=> SUCCESS: Tests pass!
- Sign off :-D
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>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
As failure situations raise exceptions that should be handled outside
the object class, methods should return $self so successive calls can be
chained nicely.
This patch makes methods return $self and adjusts the tests to reflect
this change.
Make sure tests pass:
- Run:
$ prove t/db_dependent/Koha/Patron/Attribute/Types.t
=> SUCCESS: Tests return green
- Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch introduces unit tests for Koha::Object::Library::Limit. It is
done this way because it needs to be instantiated to be usable.
To test:
- Run:
$ prove t/db_dependent/Koha/Patron/Attribute/Types.t
=> SUCCESS: Tests pass
- Sign off :-D
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
A trivial test, similar to the ones in Auth.t.
Without the check in gettemplate (added in the second patch), the passwd
file will be exposed and the test fails.
Test plan:
Run t/db_dependent/Templates.t without second patch. The two tests in the
last subtest should fail.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This patch is the Koha part of the Hea v2 project.
You can find the (testing) code for the server at
hea-ws - https://github.com/joubu/hea-ws/commits/v2
hea-app - https://github.com/joubu/hea-app/commits/v2
They contain the different pull requests made over the last 6 months.
More information on Hea at https://wiki.koha-community.org/wiki/KohaUsageStat_RFC
The goal of this commit message is to provide an overview of what could
be a new version of Hea.
Prior to these changes, the Hea database was filled with 1 line per Koha
installation. System preferences were filled by the libraries and a
cronjob (share_usage_with_koha_community.pl) collected these values to send
them to a webservice (hea-ws/upload.pl).
With the need to collect more data we would want to collect data at the library
level (branch) and not at the installation level.
For instance the geolocation, the url or the country can be different from one
library to another, even if managed from the same Koha installation.
The Hea DB has been upgraded to reflect that change (see hea-app/sql/schema.sql).
The hidden goal of this patch is to make Hea sexier and explain
better to libraries how it can be useful to share their information
with the Koha community. I guess the main problem is the lack of
communication and explanations about what we are doing we these data.
To fill this gap I'd like to (TODO)
1. Communicate on the ML about this new version of Hea (once it got
pushed and backported)
2. Link the Privacy_Policy.md from the Hea interface
3. Get help from a native English speaker to add
popup/help/info/whatever on "Home › Administration › Usage statistics",
to clearly explain what happens (and what will not happen!) when an option or
another is set.
You can find screenshot of this whole enhancement on bug 18066, comment 2.
What this patch does:
- Create a new branches.geolocation DB field
- Add 3 new sysprefs:
* UsageStatsGeolocation
* UsageStatsLibrariesInfo
* UsageStatsPublicID
- Integrate the Leaflet JS library to get a fancy map to pick
geolocations
How does it works:
On the new administration page where statistics to share are configured,
there are several new things. It is now possible to share information either
per Koha installation or libraries. If UsageStatsLibrariesInfo is set,
the info at library level (url, name, country, geolocation) will be
sent to the Hea webservice. If it is not set, you can decide to fill
UsageStatsLibraryUrl, UsageStatsLibraryName, UsageStatsCountry,
UsageStatsGeolocation to share these information. Note that even if the
data are retrieved at installation level, it's better to fill the prefs
as well: On the Hea website the different libraries defined for a given
Koha installation could be displayed on the same page.
This page is a public page which will be attributed to every
installation (with the pref UsageStatsPublicID). On this page all the
info available publicly will be displayed.
TODO later:
- Add a button on the administration page to delete the info shared
publicly. It will be easy to show that the info are no longer displayed
on the public page.
- Add an icon per Koha installation to get a better "public page"
- Any suggestions?
Test plan:
We will need to test hea-ws, hea-app and the Koha-side code to test the
whole enhancement.
1/ To start, clone the hea-ws and hea-app project and checkout the
'master' branch (*not* 'v2')
2/ Create the hea database and user
CREATE DATABASE hea
CREATE USER 'hea'@'localhost' IDENTIFIED BY 'hea';
GRANT ALL PRIVILEGES ON hea.* TO 'hea'@'localhost';
FLUSH PRIVILEGES;
3/ Fill the DB with some data
mysql hea < hea-app/sql/schema.sql
mysql hea < hea-app/sql/sql/mock-data.sql
4/ Checkout the 'v2' branch for both hea-ws and hea-app
5/ Execute the upgrade DB script
% cd hea-app
% perl -p -i -e 's/REPLACE_ME/hea/' sql/upgrade.pl # Fill the DB info
% perl sql/upgrade.pl
Now the DB is using the v2 structure. That means we have 1 installation
row per library previously defined. 1 library row has also been created.
5/ Configure hea-ws
% echo '192.168.50.1 hea.koha-community.org' >> /etc/hosts
<VirtualHost *:80>
DocumentRoot "/path/to/hea-ws"
ServerName "hea.koha-community.org"
<Directory "/">
Options +ExecCGI
Require all granted
AddHandler cgi-script .pl
</Directory>
</VirtualHost>
And enable it with a2ensite, then restart apache.
The copy the database.yml.sample to database.yml and edit it to fill the
DB info.
6/ Launch the hea-app
% cd hea-app
% edit README.md # to install the missing modules
% cp environments/config.yml environments/development.yml
% edit environments/development.yml # to fill the DB info
% perl bin/app.pl
Then hit localhost:3000
You should see a local version of Hea with sample data
7/ Back to Koha side
A. We will test that the webservice still works with previous version of Koha (without v2)
a. Do not configure Hea
% perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
Then hit localhost:3000
=> Nothing added
b. Configure Hea on admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> New library added
c. Modify the Hea configuration
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Info are modified
B. Not we will test that it works with the new version (much more fun ;))
% git checkout hea-v2 # koha
a. Configure Hea using /admin/usage_statistics.pl
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
=> Check the result on localhost:3000
b. Share libraries's info
perl misc/cronjobs/share_usage_with_koha_community.pl -f -v
c. Continue to play a bit and share the info.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
C4::Items::GetItemsCount can be replaced with Koha::Biblio->items->count
Test plan:
Create a bibliographic record with items attached
Try to delete the record from a basket (acquisition module), the detail
page and the batch item deletion tool.
=> You should not be able to delete it.
Remove the items and then try again to delete the record
=> Now you must be able to delete it.
Followed test plan, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
- Tests are now in t/db_dependent/Search/History.t
- There were 2 differents sysprefs in sysprefs.sql and in atomicupdate => fixed
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Added a syspref LoadHistory addSearchHistoryToTheFirstLoggedUser to select if you want the system to add the history of searches performed without session when you log in as registered user.
TEST PLAN
1 - Search in the catalogue, check you are not logged
2 - Log in : your last history should appear
4 - Log out
5 - Apply the patch
6 - Repeat 1 and 2
7 - Desactivate the syspref addSearchHistoryToTheFirstLoggedUser
8 - Repeat 1 and 2 : your last history shouldn't appear
The Unit test doesn't rollback but delete the added lines : the function get_template_and_user allway sets the autocommit to 1.
https://bugs.koha-community.org/show_bug.cgi?id=8010
Tested 3 patches together, works as expected.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Fix use of 'gstrate' for 'tax_rate'
Signed-off-by: Matthias Meusburger <matthias.meusburger@biblibre.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
There is a deletedbiblio_metadata table but it is not populated when a
biblio is deleted. Since we have a ON DELETE constraint on
biblio_metadata.biblionumber, the row is deleted when the biblio entry
is deleted => data lost!
Test plan:
- Create a biblio
- Delete it
=> Without this patch the deletedbiblio_metadata table is not populated
with the biblio_metadata row related to the biblio
=> With this patch applied you should see that the row has been moved.
Followed test plan, behaves as expected
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.coml>
root@koha_ci_1:/home/koha/Koha# KOHA_CONF=$KOHA_CONF perl t/Koha_Template_Plugin_Koha.t
1..3
ok 1 - use Koha::Template::Plugin::Koha;
ok 2
1..2
ok 1 - Correct development version
ok 2 - Correct maintenance version
ok 3 - Koha::Template::Plugin::Koha::Version tests
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>
To test:
1 - Make sure you don't have WebService::ILS installed
2 - prove t/00-load.t
3 - You should get a failure to load module and tests are skipped
4 - Apply patch
5 - prove t/00-load.t
6 - Module is not loaded, test are skipped
Optional:
7 - Install WebService::ILS
5 - prove t/00-load.t
6 - Module is loaded, test passes
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To test:
1 - Make sure you don't have WebService::ILS::OverDrive::Patron
installed
2 - prove t/Koha_ExternalContent_OverDrive.t
3 - You should get a failure to load module and tests are skipped
4 - Apply patch
5 - prove t/Koha_ExternalContent_OverDrive.t
6 - Module is not loaded, test are skipped
Fixed small typo 'cannot filnd' -> 'cannot find'
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
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>
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>
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>
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>
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>
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>
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>
* 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>
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>
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>
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>
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>
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>
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>
C4::Koha::GetItemTypesByCategory can be easily replaced with
Koha::ItemTypes->search({ searchcategory => ? });
So let's replace it where it is used.
Test plan:
Make sure this patch does not break the test plan of bug 10937
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The code is duplicated, variable are not set ($_), code is hard to read,
not covered by tests and the subroutine has 2 completely different
behaviors depending on the presence of the "module" parameter.
No need more ti rewrite it.
Test plan:
- Use koha_perl_deps.pl with the different options (-u -m -a -i)
- Go on the about page, "Perl modules" tab
You should not see any differences from before and after this patch
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Items.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Test plan:
prove t/db_dependent/Koha/Checkouts.t
should return green
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To make ACQ_NOTIF_ON_RECEIV TT compatible, we need to expose data from
the aqorders table. We already have a package for it in the Koha
namespace but it is based on Koha::Object[s].
The other path creates dummy Koha::Tmp::Order[s] packages to make it
usable. Of course we should use a valid Koha::Acquisition::Order[s]
based on Koha::Object, but it's outside the scope of this bug report.
This notice template is quite simple, and it's a good one to start.
From C4::Acq::NotifyOrderUsers, GetPreparedLetter is called with 4
elements: the library, the patron to notify, the biblio and the order
information.
Note that prior to this patch aqorders was filled from GetOrder, which
retrieved a lot of information from the acquisition table (aqbasket,
aqbookseller). The idea with the TT syntax is to access the data from
where it really exists. So if a user wants to display the basket name,
[% order.basket.basketname %] should be used instead.
Note that this will not work at the moment, the basket method is not
defined in the order package.
However the basic template should work as before.
The test added to TemplateToolkit proves that.
Test plan:
Use the default ACQ_NOTIF_ON_RECEIV to notify a patron that an order has
been received.
That generated template should be exactly the same as prior to this
patch.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch does:
[1] Remove unused $dbh.
[2] Move all mocking statements to one sub.
[3] Promote a few lexicals to globals when used in the subtests.
Test plan:
Run t/db_dependent/Authorities/Merge.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Bug 17909 and 17913 added a quick fix for Merge.t on UNIMARC records.
This patch improves that fix with the sub compare_fields, a merge from
compare_field_count and compare_field_order.
Also it adds the option to test MARC21/UNIMARC by adding a command line
switch that triggers mocking the marcflavour preference.
The test on a cleared field 609 in strict mode has been broken up in two
tests: first a count without 609 and then counting 609s only.
Note: Could have mocked GetMarcBiblio too, but decided to go this way.
Test plan:
[1] Run perl t/db_dependent/Authorities/Merge.t
[2] (For UNIMARC users:) Run perl Merge.t -flavour MARC21
[3] (For others:) Run perl Merge.t -flavour UNIMARC
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch makes the 'Locations' facet work as expected (i.e. having the
same behaviour it has for Zebra: picking the 952$c in MARC21 and 995e
for UNIMARC).
It also adds the code to handle holding and home library settings for
facets and makes the facets show the library name instead of the branch
code.
The mappings are updated so the labels match what facets.inc expect to
work properly.
To test:
- On master, do a search that returns biblios with items having
homebranch set.
=> FAIL: Under the 'Locations' label on the facets you will notice
branchcodes are shown.
- Apply the patch
- Restart memcached and plack (just in case, it was tricky)
- Reset your mappings:
http://localhost:8081/cgi-bin/koha/admin/searchengine/elasticsearch/mappings.pl?op=reset&i_know_what_i_am_doing=1
- Restart memcached and plack (again, not sure if needed)
- Make sure this mappings are set:
homebranch => HomeLibrary
holdingbranch => HoldingLibrary
(Note: it might not be set due to the place the yaml file is being picked)
- Reindex your records:
$ sudo koha-shell kohadev
k$ cd kohaclone
k$ perl misc/search_tools/rebuild_elastic_search.pl -d -v
- Repeat the initial search
=> SUCCESS: 'Location' contains the right stuff, 'Home libraries' and
'Holding libraries' too.
- Run
k$ prove t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
=> SUCCESS: Tests pass!
- Sign off :-D
Note: play with the 'DisplayLibraryFacets' syspref options.
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>
If the cache has to be cleaned when marc structure is updated.
Before this patch, if the age restriction was not mapped, Reserves.t
failed with:
Tag "" is not a valid tag. at t/db_dependent/Reserves.t line 523.
Test plan:
Just confirm that the tests still pass and that the changes make sense.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
It seems that only removing the key MarcSubfieldStructure- should be
enough, but it does not hurt either to remove those four keys.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Liz Rea <liz@catalyst.net.nz>
Seems to work fine
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When a user gets an email, but doesn't act or visit it within two days,
attempting to create a new one causes a collision. We should just
delete the old one, assuming they still want to reset their
password.
To test:
create yourself a borrower with a userid and password.
Attempt a password recovery on the OPAC
update the entry in the database for that user to have an expired token
e.g. update borrower_password_recovery set valid_until = '2017-01-25
03:25:26' where borrowernumber = 12;
Attempt another password recovery operation - should error
apply the patch
Try it again - no error, new token is generated and additional email
with new link is sent.
Issue reproduced - is resolved by 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>
Without this patch get_age return actually 0 instead of
Signed-off-by: Dobrica Pavlinusic <dpavlin@rot13.org>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This patch does the following:
[1] Move some POD lines from Cache to Caches.
[2] Correct C4::Plugins to Koha::Plugins in POD line of Koha::Plugins
[3] POD Koha/AuthorisedValue.pm: lib_opac moved to opac_description
[4] The POD in Koha/Patron.pm uses head2 and head3 inconsistently.
Ran s/^=head2/=head3/ on those lines (7 substitutions on 7 lines)
[5] Correct a copied POD line from reports/issues_stats.pl in
reports/reserve_stats.pl.
[6] Correct a test description in t/db_dependent/Koha/Authorities.t.
You should never delete the library :)
[7] Correct typo shouild in a comment of rebuild_zebra.pl
Test plan:
[1] Read the patch. Does it make sense?
[2] Run perldoc Koha/Cache.pm and Koha/Caches.pm
[3] Run t/db_dependent/Koha/Authorities.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: David Cook <dcook@prosentient.com.au>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This test should fail without the patch fixing the uniq calls.
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>
Signed-off-by: Jenny Schmidt <jschmidt@switchinc.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Instead of using the MAX(authid)+1 logic, AddAuthority should just save
the record and get the new id. The authid column is an autoincrement.
This eliminates problems where a newly assigned authid also refers to a
previously deleted record. (And it will not cause problems when refining
the dontmerge functionality on report 9988.)
Note: ModAuthority also calls AddAuthority to update an existing record; in
that case we should not create a new record even if the record should not
be found any more (which should be exceptional).
This patch also simplifies handling of 001 in the authority record: in all
cases this field is updated now; no need to check its contents.
Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t
[2] Add a new authority record via the interface
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Since AddAuthority uses max(id)+1 logic to produce the next authid,
authority id's will be reused when you delete the last record. This may
be a source of problems and will be addressed on the next patch.
This patch add a test to expose the problem.
Test plan:
[1] Run t/db_dependent/AuthoritiesMarc.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Modern::Perl, Koha::Database, etc.
Test plan:
Run t/db_dependent/AuthoritiesMarc.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
to test patch...
1/ set date between 00:00 and 00:59
$ sudo date -s 'Sun Jan 29 00:41:55 NZDT 2017'
2/ run prove, see fail
$ prove -v t/db_dependent/Members/IssueSlip.t
...
t/db_dependent/Members/IssueSlip.t (Wstat: 65280 Tests: 1 Failed: 0)
Result: FAIL
3/ apply patch
4/ run prove, see pass
$ prove -v t/db_dependent/Members/IssueSlip.t
...
All tests successful.
Result: PASS
NOTE: for code obscurity you could have also done a modulus 24. ;)
Signed-off-by: Mark Tompsett <mtompset@hotmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This is a follow-up on the internal server error on 0000-00-00 in the items
column onloan. This patch deals with preventing to have such dates at all
in the date fields of items.
It is accomplished by:
[1] Adding a (private) subroutine _mod_item_dates. It takes an item hash
and replaces date values if needed.
[2] AddItem and ModItem call _koha_new_item resp. koha_modify_item. In these
routines a call to the new _mod_item_dates is inserted.
[3] Although the routine is actually private, I have added some unit tests
to Items.t.
Test plan:
[1] Add a new item. Fill a correct date in dateaccessioned and an invalid
date in Price effective from (=replacementpricedate).
[2] Verify that dateaccessioned is saved correctly and replacementpricedate
is still null (does not contain 0000-00-00).
[3] Edit the item again. Fill some text in dateaccessioned and put a correct
date in replacementpricedate. Verify the results.
[4] Run t/db_dependent/Items.t
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>
Fix tests
prove t/db_dependent/Koha/Patrons.t
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When a loan period is shortened due to using decreaseLoanHighHolds* the time is
always set to the current time in X days, even if the original loan period is
given in days and not in hours.
It should default to 23:59 as is normal for loan periods given in days.
As original due date time defaults to 23:59 when given in days, this patch
modifies the hours and minutes of shortened due date to be equal to original due
date.
To test:
1. prove t/db_dependent/DecreaseLoanHighHolds.t
Signed-off-by: Grace McKenzie <grace.mcky@gmail.com>
Signed-off-by: Jonathan Druart <jonathan.druart@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
This test will prevent regression on the lost of data when
items.itemcallnumber is linked with a plugin.
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Builds on top of commit:
Bug 17255 - Upgrade Elastic Search code to work with version 2.4+ - rebased wip
-Fix data type 'string' to 'keyword' and 'text'
-index: not_analyzed deprecated, replaced with type: keyword which is equivalent
-store: yes was deprecated, use store: true
TODO: Installer bindings to both the debian package install and the raw developer install.
A taster in Buug 17851
ZE TEST PLAN
0. Remove existing ES and reinstall ES 5.1
apt-get purge elasticsearch
Follow instruction here:
https://www.elastic.co/guide/en/elasticsearch/reference/current/deb.html
1. Reset Elasticsearch index since facets are hard coded to dynamic search_marc_mappings.
1a. perl -e 'use Koha::SearchEngine::Elasticsearch; Koha::SearchEngine::Elasticsearch->reset_elasticsearch_mappings();'
1b. If you get trouble, simply DELETE FROM [search_fields|search_marc_to_fields|search_marc_mapping];
and retry 1a.
1c. Destroy elasticsearch index
curl -XDELETE localhost:9200/koha_biblios
so it can be recreated
2. Recreate the index:
perl misc/search_tools/rebuild_elastic_search.pl
2a. Add something to index if your koha.biblio-table is empty
3. Fetch all indexed records and the facet for subject__facet
curl -XGET localhost:9200/koha_biblios/data/_search?pretty -d '{
"aggregations": {
"my_agg": {
"terms": {
"field": "subject__facet"
}
}
}
}'
4. Run the included tests:
perl t/db_dependent/Koha_Elasticsearch_Indexer.t
perl t/db_dependent/Koha_Elasticsearch.t
perl t/db_dependent/Koha_SearchEngine_Elasticsearch_Search.t
Have fun with your new ES 5.1 cluster!
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The field opac_news.new is very confusing and should be renamed.
If you want to access it via Koha::NewsItem you will have trouble:
use Koha::News;
my $news_item = Koha::News->next;
say $news_item->new;
=> Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78.
This patchset is going to rename this DB field to opac_news_content instead.
Since the opac_news.new can be used in notice templates, we need to warn the
user during the update DB process that some templates must be updated.
Test plan:
0/ Apply the first patch "Add a test to highlight the issue" and confirm that
the test fail
1/ Apply this second patch
2/ Execute the DB entry
3/ Confirm that you get a warning if at least one of your notice templates is
using opac_news.new
4/ Confirm that the test new pass
5/ Add/update and delete a news
6/ Confirm that the RSS new feed still works as expected
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
With only this patch applied, proving t/db_dependent/Koha/News.t
will return
"Attempt to bless into a reference at /home/vagrant/kohaclone/Koha/Object.pm line 78."
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To recreate:
/cgi-bin/koha/patroncards/edit-template.pl?op=edit&element_id=23%20and%201%3d2+union+all+select+1,user(),@@version+--%20
Look at the Profile dropdown list.
To fix this problem and to make sure it does not appears anywhere else
in the label and patroncards modules, I have refactored the way the
queries are built in C4::Creators::Lib
Now all of the subroutine takes a hashref in parameters with a 'fields'
and 'filters' parameters.
From these 2 parameters the new internal subroutine _build_query will
build the query and use placeholders.
Test plan:
1/ Make sure you do not recreate the vulnerability with this patch
applied.
2/ With decent data in the labels and patroncards modules, compare all
the different view (undef the New and Manage button groups) with and
without this patch applied.
=> You should not see any differences.
This vulnerability has been reported by MDSec.
Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
On previous bugs
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>
The GetMemberIssuesAndFines subroutine used to retrieve the issues,
overdues and fines for a given patron. Most of the time, only 1 or 2 of
these values were used.
This patch removes this subroutine and uses the new get_issues,
get_overdues and get_balance method from Koha::Patron and Koha::Account::Lines.
Test plan:
1/ Add overdues, issues and fines to different patrons
2/ On the checkout, checkin and patron search result and the patron
detail pages, these 3 informations, if displayed before this patch, must be
correctly displayed.
3/ Use the batch patron deletion tool and make sure that patrons with a
balance > 0 are not deleted
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>
This patch makes the following changes, as requested by QA:
[1] UploadedFile->delete always calls SUPER::delete.
The return value normally comes from SUPER::delete; if removing the
file failed, we return false. Two warns are kept.
Since delete does no longer return the filename, a few changes were
needed in tools/upload.pl.
[2] Method getCategories is moved to UploadedFiles. Script tools/upload.pl
now only contains one call. Added a use C4::Koha.
[3] Calls UploadedFiles->delete as class method. As a result I removed
method delete_errors for now; may be reconsidered on a new report.
[4] Adjusted three ->search calls for id and public to ->find calls.
[5] If you pass no id to upload.pl when deleting, you don't get an alert.
All by all, we got rid of 15 lines !
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>
This patch adds some documentation lines.
And mainly rearrangs the tests in Upload.t. The 'basic CRUD testing' is
not needed separately any more. A new test catches the "file missing"
warn.
Test plan:
[1] Run perldoc on UploadedFile[s].pm
[2] Run t/db_dependent/Upload.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
Why? Koha::Uploader now only contains the actual CGI upload. The new name
better reflects its handler status.
Pragmatically, the difference between Uploaded and Uploader makes it
easier to specifically search for them in the codebase.
Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Add an upload via the interface.
[3] Check the code:
git grep "Koha::Upload;"
git grep "Koha::Upload\->"
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
Class method getCategories has no strict binding to Upload.pm. While
Upload.pm is now restricted to the actual uploading process with CGI
hook, this routine fits better in the UploadedFile package.
Class method httpheaders can be moved as well for the same reason. Note
that it actually is an instance method. The parameter $name is dropped.
Test plan:
[1] Run t/db_dependent/Upload.t.
[2] Check the categories in the combo box of tools/upload.
[3] Check a download via tools/upload and opac-retrieve-file.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
The get routine actually returns records from uploaded_files. It should be
possible to replace its calls by direct calls of Koha::UploadedFiles.
This patch is the crux of this patch set. It deals with all scripts that
use Koha::Upload.
In the process we do:
[1] Add a file_handle method to Koha::UploadedFile. This was previously
arranged via the fh parameter of get.
[2] Add a full_path method to UploadedFile. Previously returned in the
path hash key of get. (Name is replaced by filename.)
[3] Add a search_term method too (implementing get({ term => .. }).
This logic came from _lookup.
[4] Add a keep_file parameter to delete method. Only used in test now.
Test plan:
[1] Run t/db_dependent/Upload.t
[2] Go to Tools/Upload. Add an upload, download and delete.
[3] Add another public upload , search for it.
Use the hashvalue to download via opac with URL:
cgi-bin/koha/opac-retrieve-file.pl?id=[hashvalue]
[4] Go to Tools/Stage MARC for import. Import a marc file.
[5] Go to Tools/Upload local cover image. Import an image file.
Enable OPACLocalCoverImages to see result.
[6] Test uploading a offline circulation file:
Enable AllowOfflineCirculation, and create a koc file (plain text):
Line1: Version=1.0\tA=1\tB=2
Line2: 2016-11-23 16:00:00 345\treturn\t[barcode]
Note: Replace tabs and barcode. The number of tabs is essential!
Checkout the item with your barcode.
Go to Circulation/Offline circulation file upload.
Upload and click Apply directly.
Checkout again. Repeat Offline circulation file upload.
Now click Add to offline circulation queue.
[7] Connect the upload plugin to field 856$u.
Enable HTML5MediaEnabled.
Upload a webm file via the plugin. Click Choose to save the URL,
and put 'video/webm' into 856$q. Save the biblio record.
Check if you see the media tab with player on staff detail.
(See also: Bug 17673 about empty OPACBaseURL.)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
Since delete is not part of the upload process, we will move it now
to Koha::UploadedFile[s].
Deleting the file will be done in UploadedFile.
The (multiple) delete method in UploadedFiles refers to the single delete.
Test plan:
[1] Run t/db_dependent/Upload.t
The warning ("but file was missing") in the last subtest is fine;
the file did not exist. Will be addressed in a follow-up.
[2] Search for uploads on Tools/Upload. Clone this tab (repeat search on
a new tab in your browser).
[3] Delete an existing upload on the first tab.
[4] Try to delete it again on the second tab. Error message?
[5] Bonus points:
Add an upload. Mark the file immutable with chattr +i. Try to delete
the file. You should see a "Could not be deleted"-message.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
In the next set of patches we will start using these new classes in
Koha::Upload, and scripts using it.
This is just the starting point of that migration.
Test plan:
[1] Run t/db_dependent/Upload.t
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
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>
The directory it attempts to create an xml file may not be writable for
the user running the test. By changing the directory from the current
directory to a temporary one, the test runs. After all 'chmod 777
t/db_dependent' is a bad idea.
TEST PLAN
---------
1) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- fails
2) apply patch
3) sudo koha-shell "prove t/db_dependent/Sitemapper.t" kohadev
-- succeeds
4) run koha qa test tools
Tested without qa tools
Signed-off-by: Claire Gravely <claire_gravely@hotmail.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Adding another delete for field 100.
Will mock GetMarcBiblio on a new report.
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Remove some commented warnings
Remove the commented old code at the end of sub merge
Explicitly set merge mode in the first subtest
Move the return to loose mode from the second subtest to the third
Test plan:
Run t/db_dependent/Authorities/Merge.t
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>
