Run perltidy pro=xt/perltidyrc on the file.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
From C4::Templates::output
# add variables set via param to $vars for processing
for my $k ( keys %{ $self->{VARS} } ) {
$vars->{$k} = $self->{VARS}->{$k};
}
This loop is not necessary, we could do the same with
$vars = { %$vars, %{ $self->{VARS} } };
After a quick benchmark, it gains 100 microseconds when we pass 170 vars
to the template.
Test plan:
Do some clicks on the interface, everything should be ok.
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Perl idiosyncratic way of merging hash, clearer, if not quicker (not
verified)
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
To retrieve a patron image, we can call Koha::Patron::Images->find or
Koha::Patrons->find->image
Both will return a Koha::Patron::Image object.
Test plan:
1/ From the patron/member module, open all tabs on the left (Checkouts,
detail, fines, etc.)
The image should be correctly displayed.
2/ At the OPAC, on the patron details page (opac-memberentry.pl) the
image should be displayed as well.
3/ Same on the sco module.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The C4::Members::RmPatronImage just removed a row in patronimage.
This can be accomplished using the delete method of Koha::Patron::Image.
Test plan:
From the patron defail page, try to delete the image of a patron.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
The C4::Members::PutPatronImage inserted/updated the image of a patron.
This can be done easily with ->find->set->store or ->new->store
Test plan:
1/ Modify the image of a patron from the patron detail page
2/ Add an image to a new patron
3/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of a patron
4/ Use the "Upload patron images" tools (tools/picture-upload.pl) to add
or modify the image of several patrons, using a zip file.
Stress the script trying to get as many errors as possible (wrong
cardnumber, wrong mimetype, file does not exist, etc.)
With this patch, if the cardnumber does not exist, you will get a
specific error "Image not imported because this patron does not exist in
the database"
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
There are 3 subroutines in C4::Members to get, add and delete patron
images:
- GetPatronImage
- PutPatronImage
- RmPatronImage
By creating these 2 Koha::Patron::Image[s] classes, we could remove them easily.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
When closing a basket in Acquisitions the confirmation dialog should be
styled using Font Awesome icons. This patch adds them.
The cancel button has been moved into its own form so that we can get
rid of several problematic JavaScript issues: The use of the
"javascript:" pseudo-protocol, the use of an "onclick" attribute, and
the use of a JS redirect where a GET action will do.
To test, apply the patch and locate an open basket in Acquisitions which
can be closed.
- Click the 'Close this basket' button and confirm that the icons in the
dialog look correct.
- Test the cancel operation to make sure you are redirected back to the
page showing the basket details.
- Test the confirm operation and verify that the basket is closed.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Please note that the confirmation message display depends on
BasketConfirmations.
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
In Acquisitions, when you delete (cancel) an order, a dialog asks you to
confirm. This dialog should be styled with Font Awesome icons.
Other minor edit: Changing page grid style for centered main content
area.
To test, apply the patch and locate a basket in Acquisitions with an
order.
- View the details for the basket.
- Click the 'Delete' link next to the order you want to delete.
- Confirm that the dialog is correctly styled.
- Test the cancel operation and verify that you are correctly redirected
back to where you were.
- Test the confirmation operation and verify that your order is deleted.
- Also test the deletion process from Acquisitions -> Vendor -> Invoices
-> Invoice -> Go to receipt page -> Delete order.
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: Brendan A Gallagher <brendan@bywatersolutions.com>
When attaching an existing item to a record, the confirmation and error
dialogs should be styled using Font Awesome icons. This patch revises
the buttons and revises the global CSS file.
To test, apply the patch and clear your cache if necessary.
- Locate a record in the catalog to which you can attach an item.
- Select "Attach item" from the "Edit" menu.
- Submit a barcode for an exising item.
- Verify that the confirmation dialog is correctly styled.
- Test the operation of both the "OK" and the "Attach another item"
buttons.
- Submit an invalid barcode.
- Verify that the error dialog is correctly styled.
- Test the operation of both the "OK" and the "Try again"
buttons.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This patch makes some markup changes to the patron lists interface in
order to make them more consistent with other areas of Koha.
Change to look for when testing:
Tools -> Patron lists:
- Wrap "New patron list" button in standard <div id="toolbar">
- Place toolbar above page heading.
Tools -> Patron lists -> New patron list:
- Use built-in form validation for required fields
- Clarify page title and breadcrumbs on list add and edit
Validation errors have been corrected on all three templates, including
removal of an extra </div>.
Revision: I was wrong about the frequency of use of "Modify." There's no
clear-cut reason to replace it with "Edit" so I have undone that change.
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Works as expected. No regression. Better texts. Consistent validation.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The authorities MARC subfield structure template contains some unused
JavaScript, "function displayMoreConstraint()" This patch removes it.
To test, apply the patch and go to Administration -> Authority types ->
MARC structure -> Subfields -> Edit subfields and confirm that there are
no JavaScript errors and tab switching works correctly.
A search of the source code should show no instances of
"displayMoreConstraint."
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
No regression, no JS warning.
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Bug 15354 did not fix correctly the issue with the item type display.
The default item type '*' should be "All", at the moment nothing is
displayed in the column.
Test plan:
1/ Define a rule for default item types (All/*) and for specific item
types.
2/ Translate some item type descriptions used in previous step
3/ Default item type should be "All" and correct translations should be
displayed.
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
When the SMSSendDriver system preference is set to "Email" a link to the
SMS cellular providers management page should appear in the left-hand
sidebar menu on Administration pages. This patch adds the link to
admin-menu.inc.
To test, apply the patch set your SMSSendDriver system preference to
"Email." View various Administration pages which show the left-hand
sidebar menu and confirm that "SMS cellular providers" is in the list.
Empty the SMSSendDriver preference and confirm that the link no longer
appears.
Signed-off-by: Nicole C Engard <nengard@bywatersolutions.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This patch sorts the modification templates when getting them from the
database This affects the pages below:
tools/batch_record_modification.pl
tools/marc_modification.pl
tools/stage-marc-import.pl
To test:
1 - Add some marc modification templates in a non alpha order (Shoes,
Hats, Cats)
2 - visit the pages above and note the templates are in order
added
3 - apply patch
4 - visit the three pages and note correct order
5 - verify all tools continue to work as expected
Signed-off-by: Frédéric Demians <f.demians@tamil.fr>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
perlcritic -5 failed.
Attempt to clean up to a higher level:
-- use English to address use of $@ variable
-- perltidy on the code
-- substitute q{} for ''
-- expand out single line hacky goodness (... s/\.pm$//) to more code
-- remove parenthesis on functions that don't need it
-- add x, s, and m as needed to regexps
-- change double quotes to single quotes where no variable involved
-- tweaked eval destroy test to check return value and use $EVAL_ERROR
-- renamed $processor to $record_processor in the subtest to avoid
lexical warnings
TEST PLAN
---------
$ perlcritic -5 t/RecordProcessor.t
Don't modify $_ in list functions at line 43, column 25. See page 114 of PBP. (Severity: 5)
$ perlcritic -2 t/RecordProcessor.t
No package-scoped "$VERSION" variable found at line 1, column 1. See page 404 of PBP. (Severity: 2)
Quotes used with a string containing no non-whitespace characters at line 34, column 36. See page 53 of PBP. (Severity: 2)
Quotes used with a string containing no non-whitespace characters at line 34, column 39. See page 53 of PBP. (Severity: 2)
Quotes used with a string containing no non-whitespace characters at line 36, column 33. See page 53 of PBP. (Severity: 2)
Quotes used with a string containing no non-whitespace characters at line 36, column 36. See page 53 of PBP. (Severity: 2)
Don't modify $_ in list functions at line 43, column 25. See page 114 of PBP. (Severity: 5)
Regular expression without "/s" flag at line 43, column 33. See pages 240,241 of PBP. (Severity: 2)
Regular expression without "/x" flag at line 43, column 33. See page 236 of PBP. (Severity: 3)
Regular expression without "/m" flag at line 43, column 33. See page 237 of PBP. (Severity: 2)
Regular expression without "/s" flag at line 43, column 66. See pages 240,241 of PBP. (Severity: 2)
Regular expression without "/x" flag at line 43, column 66. See page 236 of PBP. (Severity: 3)
Regular expression without "/m" flag at line 43, column 66. See page 237 of PBP. (Severity: 2)
Expression form of "grep" at line 47, column 8. See page 169 of PBP. (Severity: 4)
Expression form of "grep" at line 50, column 20. See page 169 of PBP. (Severity: 4)
Regular expression without "/s" flag at line 50, column 26. See pages 240,241 of PBP. (Severity: 2)
Regular expression without "/m" flag at line 50, column 26. See page 237 of PBP. (Severity: 2)
Return value of eval not tested at line 73, column 1. You can't depend upon the value of $@/$EVAL_ERROR to tell whether an eval failed. (Severity: 3)
Magic punctuation variable $@ used at line 78, column 5. See page 79 of PBP. (Severity: 2)
Reused variable name in lexical scope: $processor at line 84, column 5. Invent unique variable names. (Severity: 3)
Subroutine "new" called using indirect syntax at line 87, column 18. See page 349 of PBP. (Severity: 4)
Subroutine "new" called using indirect syntax at line 93, column 18. See page 349 of PBP. (Severity: 4)
Quotes used with a string containing no non-whitespace characters at line 96, column 40. See page 53 of PBP. (Severity: 2)
Subroutine "new" called using indirect syntax at line 99, column 18. See page 349 of PBP. (Severity: 4)
Subroutine "new" called using indirect syntax at line 106, column 18. See page 349 of PBP. (Severity: 4)
$ prove -v t/RecordProcessor.t
t/RecordProcessor.t .. ok
All tests successful.
Files=1, Tests=13, 0 wallclock secs ( 0.01 usr 0.00 sys + 0.22 cusr 0.02 csys = 0.25 CPU)
Result: PASS
$ prove -v t/RecordProcessor.t
...
$ git bz apply 15871
Repeat perlcritic level 2, and only $VERSION warning should exist.
Retest with the prove.
Run koha qa test tools.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as advertised
Signed-off-by: Jesse Maseto <jesse@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@unc.edu.ar>
I don't really care about perlcritic as long as it involves changing '' into qw{} (WTF?)
Anyway, I'd do this kind of things as we go, for example, if we were adding more tests. In that
case it would just be a followup for this, after you provided a patch for an enh/bugfix.
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The 'borrower' should not be used anymore, especially for new code.
This patch move files and rename variables newly pushed (i.e. in the Koha
namespace).
Test plan:
1/
git grep Koha::Borrower
should not return code in use.
2/
Prove the different modified test files
3/ Do some clicks in the member^Wpatron module to be sure there is not
an obvious error.
Signed-off-by: Hector Castro <hector.hecaxmmx@gmail.com>
Works as described. Tested with Circulation, Members/Patrons, Discharge,
Restrictions modules and the must common functionalities
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Jesse Weaver <jweaver@bywatersolutions.com>
The messages related tests in t/db_dependent/Members.t are not
deprecated and can be removed.
Test plan:
git grep AddMessage
git grep DeleteMessage
git grep GetMessagesCount
git grep GetMessages
should not return any result.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This subroutine just retrieved the messages given some parameters.
Some job should not have been done in this subroutine.
It was called only 3 times, in circ/circulation.pl and opac-user.pl.
Basically it was used to retrieved the message to displaye for a given
patron ($borrowernumber) at the OPAC (B) or Staff (L).
For the 3 calls, the 2 parameters $borrowernumber and $type
(message_type) were passed, the "%" trick at the beginning of the
subroutine was useless.
Moreover, the date formatting should be done on the TT side, not in
subroutine.
The can_delete flag was set if the branchcode given in parameter was the
same as the one of the message. This has been delegated to the template.
Indeed the can_delete was not valid, since it must depend on the
AllowAllMessageDeletion pref.
The test is now:
IF message.branchcode == branch OR
Koha.Preference('AllowAllMessageDeletion'')
There is not specific test plan for this patch, the changes have already
been tested in previous patches.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The GetMessageCount subroutine was only used once, in opac-user.pl, to
know if some messages will be displayed.
Test plan:
1/ Create messages to display at the OPAC for a patron
2/ Logged this patron in at the OPAC, you should see the messages
displayed.
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The DeleteMessage just deleted and logged if needed.
This is now be done by Koha::Patron::Message->delete.
Test plan:
1/ Go on the "check out" page of a patron
2/ Delete some messages
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The AddMessage subroutine just
1. checked if all the parameters were passed
2. inserted the message in the DB
3. logged the action if needed
These 3 jobs are now done by the overloaded store method of
Koha::Patron::Message.
Test plan:
1/ Go on the "check out" page of a patron
2/ Add different new messages
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
If the pref BorrowersLog is on, the store and delete method should log
into the action_logs table.
Easy to do by overloading them.
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The following 4 CRUD subroutines in C4::Members:
- AddMessage
- DeleteMessage
- GetMessagesCount
- GetMessages
could be replaced with a new package based on Koha::Objets.
This patchset will add the 2 Koha::Patron::Message[s] classes, then use
it to replace the different calls to these subroutine.
It will slightly reduce the size of C4::Members
Signed-off-by: Marc Véron <veron@veron.ch>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Test currency left behind will cuase next run of the unit test to fail
as well as leaving db cruft.
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
See the comment for details.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
This subroutine is unused and can be removed.
There is no trace left of currency in C4::Budgets.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Most part of the code here is unnecessary complex. We should selected
the currency if it is selected, that's all :)
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
On inserting an active currency, others should be marked as inactive.
We can only have 1 active currency at the same time.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The first step of this patch set is to move the business logic code from
admin/currencies.pl to Koha::Acquisition::Currenc[y|ies].
Then a foreign key will be created on aqorders.currency as we want to
assure data integrity. Note that a aqbooksellers.currency also exists
but is never used. It could be removed in another bug report.
This update has to care about possible breaking relation. For instance
an old order has been made using a currency which is now deleted. To be
sure the update process won't break a new column currency.archived is
created and leave open the door for a further improvement (marked a
currency as archived from the interface: this won't be provided by this
enhancement).
These archived currencies won't appear on the interface for newly
created items (order/suggestion).
Once this is done it becomes easy to remove the subroutine from
C4::Budgets: GetCurrencies and GetCurrency.
ConvertCurrency will also be removed but is not used anymore.
Note that none of these subroutines were covered by tests. Now they are.
Test plan:
0/ Don't apply this patch
1/ Create a temporary currency and an order using it
2/ Remove the currency from your CLI
3/ Apply the patch and execute the DB entry
4/ Note that the currency is inserted and marked as archived.
5/ Edit the previous order and confirm that the correct currency is
still selected.
It won't never be displayed anywhere else.
6/ Test the admin script: on admin/currency.pl add/remove/edit
currencies.
You could not have 2 active currencies at the same time.
7/ Then on the different scripts of the acquisition module, focus on the
currencies values and create a new order, receive it.
Create/edit a suggestion
Other changes must be checked by the QAer.
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
Bug 12803 [QA Followup] - Remove use of C4::Dates
C4::Dates was being included, but not used in the code!
Bug 12803 [QA Followup] - Fix koha-qa.pl errors
Bug 12803 [QA Followup] - Update unit tests due to changes in master
Bug 12803 [QA Followup] - Fix to stop failing unit tests
Bug 12803 [QA Followup] - Remove duplicate 'use' lines
Bug 12803 [QA Followup] - Remove NO_CACHE
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
The holds queue is typically generated many times a day in order to
select items to fill holds. Often these items are to be sent to a
different library. However, if the library whose item is picked to fill
a hold is closed, that hold will remain unfilled even if there are other
open libraries who own that item. It would be helpful if we could skip
closed libraries for the purpose of selecting items to fill holds.
Test Plan:
1) Apply this patch
2) Run updatedatabase.pl
3) Create a record with two items on it, one at Branch A, and one at
Branch B
4) Place a hold for pickup at Branch C
5) Generate the holds queue
6) Note which branch's item is selected for the hold
7) Enable the new system preference HoldsQueueSkipClosed
8) Add today as a holiday for that branch noted in step 6
9) Regenerate the holds queue
10) View the holds queue, notice the item selected is not from
the closed branch!
11) prove t/db_dependent/HoldsQueue.t
Signed-off by: Jason Robb <jrobb@sekls.org>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
When resending an email from the Notices tab in Patrons, we would like
to use the recent email address.
Test plan:
[1] Look up a patron with some notices sent.
[2] Adjust the patron email address (watch AutoEmailPrimaryAddress).
[3] Resend the notice. Verify if the new address was used.
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Brendan A Gallagher <brendan@bywatersolutions.com>
