Commit graph

236 commits

Author SHA1 Message Date
a9e3db201e Bug 26922: Better error handling in SendAlerts
This patch makes SendAlerts display a better error message when sending
fails.

To test:
1. Set KohaAdminEmailAddress to admin@example.org
2. Edit a vendor, set a valid email address
3. Create a new basket, a new order. Send the basket
=> FAIL: As you did not configure a valid SMTP server, the email is not sent and logs displayed "unable to establish SMTP connection to (localhost) port 25", with the stracktrace.
4. Apply this patch and reload all
5. Repeat 3
=> SUCCESS: A simpler message is displayed, the stacktrace remains in
the logs
6. Sign off :-D

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-20 15:44:20 +01:00
Joonas Kylmälä
4415fdb0f4 Bug 26948: Remove double encoding from sent emails
The sent emails from Koha were garbled / double encoded because
Koha::Email now after the refactoring and moving to use Email::Stuffer
handles the encoding itself. Previously the Koha::Email callee had to
handle the encoding. This patch removes the double encoding in the
email subject and content.

To test:
 1) Put some UTF-8 characters like ä in the HOLD template subject and body
 2) Create hold and check-in an item to make it Waiting and cause the
 email to be send
 3) Notice the email comes garbled / double encoded
 4) Apply patch
 5) Repeat step 2) and notice the email comes without any issues in
    encoding and look OK.
 6) Make sure "prove t/Koha/Email.t" and "prove
    t/db_dependent/Letters.t" and passes

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-12 17:09:52 +01:00
Andrew Isherwood
afd13b6a9b Bug 22818: (follow-up) Respond to feedback
This commit fixes this bug, it was broken in a number of ways.

Fixes include:

- Added necessary config block in C4::Letters to enable the TT notice
syntax introduced in an earlier commit to work
- Changed template variables to refer to singular objects rather than
multiple e.g. borrowers -> borrower
- Fixed missing / misnamed variables

This commit also implements the additional syspref checks suggested by
Katrin in comment #87

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-11 08:35:10 +01:00
Andrew Isherwood
ef07ad1044 Bug 22818: Add support for ILL notices
Here we're just adding ILL notices to the existing "letters"
infrastructure so notices can be displayed and edited

Sponsored-by: PTFS Europe
Signed-off-by: Niamh Walker-Headon <Niamh.Walker-Headon@it-tallaght.ie>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-11 08:35:10 +01:00
cb3753d292 Bug 26935: Don't use basket for claimacquisition and claimissues
Incorrect basketid is sent for claimacquisition and claimissues.

The basket info are not used in the default template of these notices, but if it
is used, a wrong basket may be passed to generate the notice.

This has been caught by a test failure after bug 26745.

 #   Failed test 'SendAlerts - claimissue'
 #   at t/db_dependent/Letters.t line 642.
 C4::Letters::_substitute_tables(): DBI Exception: DBD::mysql::st execute failed: called with 2 bind variables when 1 are needed [for Statement "SELECT * FROM aqbasket WHERE       basketno = ?" with ParamValues: 0=53] at /kohadevbox/koha/C4/Letters.pm line 679

This is a quick patch to fix the failing test, but more work would needed.

We are removing the basket info from those 2 notices and keeping it only
for orderacquisition.

Technically: the $externalid is coming from the parameter list of
SendAlerts, but it is a basketno only for orderacquisition. We must not
pass it for other notices.

Test plan:
Confirm that the failing test is now passing and that the changes make
sense.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-05 15:24:20 +01:00
7c9e44b24e Bug 26745: Add TT support to 'title' in notices
This patch adds support for using TT syntax in the title element of the
notices system.

Test plan
1/ Edit an easily triggerable email notice such that the title contains
   some template toolkit markup.
   Example: Edit the 'CHECKOUT' notice to include [% today %] in the
   title field.
2/ Trigger the notice (For 'CHECKOUT' that means checking an item out to
   a user (Preferably with 'email' selected in their advanced messaging
   preferences for the notice)
3/ Either, navigate to the 'Notices' tab for the user, or run the
   message queue and inspect the subject line of the recieved email.
4/ Prior to the patch the TT syntax would remain 'as is'.. i.e
   '[% today %]'.
5/ After the patch, the TT syntax should have been properly replaced.
6/ Signoff

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 17:32:35 +01:00
96be711d03 Bug 21898: Add basket for TT syntax
Without this patch the basket's info won't be available for the TT
syntax.

With this patch you can now use [% basket.basketname %]

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 12:59:34 +01:00
98ab986392 Bug 21898: Add aqbasket to available tables for ACQORDER
This patch should allow <<aqbasket.$field>> replacements to work in both
email subject and email content fields for notices.

Test plan
1/ Add <<aqbasket.basketname>> into the ACQORDER notice (In the subject
line and content box)
2/ Trigger said notice to send (before applying the patch)
3/ Note that the replacements have not taken place..
<<aqbasket.basketname>> is present as is.
4/ Apply patch
5/ Trigger the notice again
6/ Note that the replacements are now made.
7/ Signoff

Signed-off-by: Séverine QUEUNE <severine.queune@bulac.fr>

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

JD amended patch: fix alignment in hashref, edit commit message to fix
letter code "AQORDER" vs "ACQORDER"

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-11-04 12:59:34 +01:00
cd7b54ab71 Bug 22343: (QA follow-up) Fix some comments
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-10-02 10:54:41 +02:00
3a80954aa3 Bug 22343: Make C4::Letters use the new SMTP server config
This patch makes the different methods in C4::Letters use:
- Koha::SMTP::Servers: to get the effective SMTP server for the library
  or the fallback default if no library in context.
- New Koha::Email->create method for crafting the email envelope for
  sending.

The tests are adapted so they behave the same way, but the trapped (in
the mock) $email object has the right type and its attributes are
accessed correctly.

To test:
1. Apply this patch
2. Run:
   $ kshell
  k$ prove t/db_dependent/Letters.t
=> SUCCESS: Tests pass. YAY!
3. Sign off :-D

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-10-02 10:54:40 +02:00
Julian Maurice
b168f4a2e9 Bug 21395: Make perlcritic happy
This patch adds a .perlcriticrc (copied from qa-test-tools) and fixes
almost all perlcrictic violations according to this .perlcriticrc
The remaining violations are silenced out by appending a '## no critic'
to the offending lines. They can still be seen by using the --force
option of perlcritic
This patch also modify t/00-testcritic.t to check all Perl files using
the new .perlcriticrc.
I'm not sure if this test script is still useful as it is now equivalent
to `perlcritic --quiet .` and it looks like it is much slower
(approximatively 5 times slower on my machine)

Test plan:
1. Run `perlcritic --quiet .` from the root directory. It should output
   nothing
2. Run `perlcritic --quiet --force .`. It should output 7 errors (6
   StringyEval, 1 BarewordFileHandles)
3. Run `TEST_QA=1 prove t/00-testcritic.t`
4. Read the patch. Check that all changes make sense and do not
   introduce undesired behaviour

Signed-off-by: Bernardo Gonzalez Kriegel <bgkriegel@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2020-06-29 12:37:02 +02:00
aa1f0fd73f
Bug 24612: Use the reserve_id to identify a reserve when building a notice
Now that we have the reserve_id PK on the reserves table we should use
it (instead of the couple borrowernumber, biblionumber)

Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-05-22 09:33:17 +01:00
6ac037f817
Bug 24840: Replace DateTime->now with dt_from_string
We should use Koha::DateUtils instead of Date::Time directly

This patch simplay replaces calls to now() with a call to dt_from_string()
which does effectively the same thing.

Probably reading the code and verifying changes is sufficient but...

To test:
1 - confirm the files all compile
2 - confirm all tests pass
3 - confirm Koha still works

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-08 11:54:23 +01:00
Aleisha Amohia
96883e69b5
Bug 4461: (follow-up) Creating sample notice and other fixes
- filters
- capitalisation
- atomic update

Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: David Nind <david@davidnind.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-04-06 11:16:38 +01:00
6070492462
Bug 18177: remove aqbooksellers.bookselleremail
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-27 12:15:51 +00:00
ccfc6572f7
Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttributes
This patch replace Koha::Patron->get_extended_attributes with
->extended_attributes
It's now a getter a setter method.

It permits to replace UpdateBorrowerAttribute and use
create_related from DBIx::Class

Notes:
* We face the same variable names difference than in a previous patch
(value vs attribute)

Bug 20443: Remove SetBorrowerAttributes

squash  + RM get_extended_attributes

 RM get_extended_attributes

SQUASH Bug 20443: Remove UpdateBorrowerAttribute and SetBorrowerAttribute

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 13:44:16 +00:00
9707167a48
Bug 20443: Move GetBorrowerAttributes to Koha::Patron->extended_attributes
The GetBorrowerAttributes subroutine return the attributes for a given
patron.

Using get_extended_attributes we can acchieve it easily. The problematic
here is to restore the method's name (value vs attribute,
value_description vs description of the authorised value, as well as
display_checkout that should not be a method of Attribute, but
Attribute::Type instead)

value_description was used when the attribute types were attached to an
authorised value category. To avoid the necessary test in template and
controller there is now a $attribute->description method that will
display either the attribute's value OR the value of the authorised
value when needed. We should certainly use this one from few other
places.

Notes:
* This patch rename Koha::Patron->attributes with Koha::Patron->get_extended_attributes.
It will be renamed with Koha::Patron->extended_attributes in ones of the next
patches when it will become a setter as well.
* GetBorrowerAttributes did not care about the library limits, we still
do not
* The opac_only flag was not used outside of test, we drop it off.
* To maintain the existing behavior we add a default order-by clause to
the search method [code, attribute]
* From C4::Letters::_parseletter we always display the staff description
of the AV, There is now a FIXME to warn about it
* FIXMEs are not regressions, existing behaviors must be kept
* TODO add a new check to bug 21010 to search for inconsistencies in
patron's attributes attached to non-existent authorised values
* One test has been updated in Modifications.t, order_by is now
by default set to ['code', 'attribute']

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-23 13:39:25 +00:00
c1129d2286
Bug 10269: Use 'reply-to' address if passed to EnqueueLetter
This patch updates the EnqueueLetter method to allow passing a reply_to
address. This allows for passing in arbitrary addresses for the reply
address which will not override the from address. This improves the
chances of your email getting past spam filters.

Signed-off-by: Myka Kennedy Stephens <mkstephens@lancasterseminary.edu>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-03-18 14:03:42 +00:00
Agustin Moyano
5ac7f3690d
Bug 23673: Add "Updated on" column to patron's notices
This patch adds "Updated on" column to patron's notices tab. It also adds logic to C4::Letters to retrieve updated_on column.

To test:

1. Apply patches.
2. Restart plack.
3. Choose a patron and add a purchase suggestion.
4. Change suggestion status.
5. Open patron's notifications.
   CHECK => Messages table has now "Updated on" and "Time created" columns, and "Time" column is gone.
   SUCCESS => There is a message with status pending, with a "time created" that equals "updated on"
6. Execute in the shell in Koha directory
   $ ./misc/cronjobs/process_message_queue.pl
7. Open patron's notifications one more time.
   SUCCESS => The message changed status. Time created remained the same, and now "updated on" has the current timestamp.
8. Resend the message and repeat sep 6.
   SUCCESS => Every time you change the status, time created remains the same and updated on updates.
9. Run `prove t/db_dependant/Letters.t`
10. Sign off

Signed-off-by: Kelly McElligott <kelly@bywatersolutions.com>
Signed-off-by: Andrew Fuerste-Henry <andrew@bywatersolutions.com>
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2020-02-05 14:22:43 +00:00
86f999812e
Bug 23765: Do not display localized templates if TranslateNotices is off
To test:
1. Enable multi-languages
2. Set the preference 'TranslateNotices' on  'Allow'
3. Go to: tools==>Notices & slips==>Edit, make sure it has multilingual
email templates.
4. Set the preference TranslateNotices on 'Don't allow'.
5. Go to: tools==>Notices & slips==>Edit, the template shows several tab
for the same transport type.
6. Apply the patch.
7. Repeat the steps 4 and 5
8. Success. It only shows the default template when TranslateNotices is
'Dont allow'.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
2019-11-13 13:36:45 +00:00
fa643aa253 Bug 5770: (QA follow-up) Revert change to GetQueuedMessages
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-05-10 17:29:31 +00:00
Nazlı Çetin
a59003bea1 Bug 5770: Unit test
Signed-off-by: Liz Rea <wizzyrea@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-05-10 17:29:31 +00:00
c5a98ed3e1 Bug 8000: (QA follow-up) Two lastminute fixes
[1] Number of tests Letters.t
[2] Resolving uninitialized warn on Letters, L1327

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-29 15:08:47 +00:00
Maryse Simard
26a61c97f0 Bug 8000: (QA follow-up) Update the to_address in the message queue
Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-29 15:08:47 +00:00
968eea155e Bug 8000: (QA follow-up) Centralise Bcc too
Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-29 15:08:46 +00:00
David Bourgault
d51054c8b4 Bug 8000: Redirect all emails to SendAllEmailsTo
Rebased and squashed after changes to master.
Only difference from previous patches are small adjustements to conflicts in t/db_dependent/Letters.t

Test plan:

1) Apply path
2) Run updatedatabase.pl
3) Clear all SendAllEmailsTo system preference
4) Send mail to a patron of your choosing, email will go to patron's
   email address as usual.
5) Set SendAllEmailsTo to a test email address
6) Send mail to the patron, email will be redirected to the email set
   in the systempreference.
7) Run prove -v t/db_dependent/Letters.t

It does not affect messages in the message_queue.

This patch obsoletes previous patches, because it achieves the same
functionality in a much more centralized way. (4 lines of code!)

Signed-off-by: Ed Veal <eveal@mckinneytexas.org>
Signed-off-by: BWS Sandboxes <ByWaterSandboxes@gmail.com>
Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Liz Rea <wizzyrea@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-04-29 15:08:46 +00:00
aae5e55846 Bug 21875: Handling subject line in Letters.pm
The variable name $utf8 is very misleading: it contains MIME-Header encoding.
$message->{subject} comes from the database and is in perl internal format;
it should NOT be decoded as a MIME-Header.
After encoding to MIME-Header, previously another (useless) encoding to
UTF-8 was done. Since the string is plain ASCII, this is useless and
theoretically wrong. We should stay in MIME-Header.

Test plan:
See Bugzilla comment5.

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: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-02-27 09:14:21 -05:00
5b160af2a0 Bug 21829: Correctly format dateexpiry in notices (date only)
dateexpiry is a date, we should not display the time
part in notice.

Test plan:
Create a notice with borrowers.dateexpiry, generate this notice.
The value should be displayed without the time part

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2019-02-11 18:32:40 +00:00
a200aafd3b Bug 21571: Make ACCTDETAILS translatable
This patch makes the notice "ACCTDETAILS" translatable.

Test plan:
1. Switch on TranslateNotices and AutoEmailOpacUser
2. Define templates for different languages for ACCTDETAILS
3. Create a new patron, define an email address, userid and password.
Also pick a different "preferred language" then the default value
4. You should receive the email with the correct, translated, email.

Signed-off-by: Hugo Agud <hagud@orex.es>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-12-26 20:47:06 +00:00
David Gustafsson
d659649a13 Bug 20356: Add EmailSMSSendDriverFromAddress system preference
Add EmailSMSSendDriverFromAddress system preference for overriding Email
SMS send driver from address.

How to test:
1) Run tests in t/db_dependent/Letters.t
2) All tests should pass

Sponsored-by: Gothenburg University Library

Signed-off-by: Michal Denar <black23@gmail.com>

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

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-11-13 14:36:55 +00:00
Katrin Fischer
a002d12888 Bug 15971: Add biblioitems to available fields for serial claim notices and fix notices editor display
This fixes 2 related problems:

1) The editor only offered 3 entries from biblio for adding to the notice:
title, author, serial
It was not clear that actually all fields from biblio can be used.

2) It was not possible to use fields from biblioitems in the notices
which left out important fields like the ISSN.

The patch adds the biblioitems table and changes the editor to show
all entries from biblio and biblioitems table are shown on the left.

To test:
- Create a subscription
- Generate next issue a few times to get late issues
- Create a new notice in the "Claim serial issue" module
  - Use fields from different/all tables
- Make sure an email address is set for
  - the vendor (also check for 'receives claims for late issues')
  - your staff user
- Go to serials > claims
- Claim multiple issues
- Verify the email is generated and contains the correct information

Example notice:

Title: <<biblio.title>>
Author: <<biblio.author>>
ISSN: <<biblioitems.issn>>
ISBN: <<biblioitems.isbn>>
Issue: <<serial.serialseq>>
--

Signed-off-by: Owen Leonard <oleonard@myacpl.org>
Signed-off-by: Chris Cormack <chrisc@catalyst.net.nz>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
2018-10-01 13:56:34 +00:00
5c7ff786d5 Bug 19855: Move getalert, addalert and delalert to Koha::Subscription
This patch removes 3 subroutines from C4::Letters:
- getalert
- addalert
- delalert

And add 3 methods to Koha::Subscription:
- subscribers
- add_subscriber
- remove_subscriber

It makes the code cleaner for future cleanup.
TODO - we should remove alert.alertid and alert.type, and rename
alert.externalid with alert.subscriptionid
That way alert will be renamed borrowers_subscriptions (or similar) and
will become a simple join table between borrowers and subscriptions.
We will need to deal with FK that could not be satisfied.
Let's do that after this patch is pushed.

Test plan:
Subscribe and unsubscribe to email notifications sent when a new issues
is available.
Make sure everything works as before and you receive the emails.

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
0bd1f30c8c Bug 19855: Remove $type from the alerts
It looks like this feature has never been finished. It has been
developed with more flexibility in mind, but only 'issue' is used for
this parameter. Apparently it could have been 'virtual', for virtual shelves.

Let remove this parameter and clean the code a bit.
TODO: Remove the DB column

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
8a81a4adab Bug 19855: Remove C4::Letters::findrelatedto
This subroutine is called only once. It only concat firstname and
surname for subscribers.
It can be easily replaced with Koha::Patron

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-23 14:22:15 -03:00
38132cabbb Bug 18725: (QA follow-up) Use make_column_dirty instead of status change
Moving the status to the invalid 'processing' might well have unwanted
side-effects. (The status column will be set to empty string and we have
a problem if it is not processed.)

This patch allows pass-through of DBIX's make_column_dirty in
Koha::Object (simple tests included) and uses it to force an update.
If the update does not return true, it still exits.

Test plan:
[1] Read the changes.
[2] Run t/db_dependent/Koha/Object.t

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-16 13:07:22 -03:00
8ad4547432 Bug 18725: Prevent process_message_queue to send duplicate emails if message_queue is not writable.
Last week, we had a database server whose disk filled, causing database writes to fail.

This meant that messages in message_queue marked 'pending' were not marked as 'sent' when they were added to the postfix mail queue; messages were sent every 15 minutes (as specified in the cron job) until the disk space issues were cleared.

I would suggest adding a token write to the start of process_message_queue.pl as a 'canary in the coal mine'. If the database write fails, process_message_queue should stop, because it's not safe to proceed sending emails that may not be marked 'sent'.

Test Plan:
1) Apply this patch
2) Make the message_queue table unwriteable somehow
3) Run process_message_queue.pl
4) Script should die with error

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-16 13:07:22 -03:00
a236b684fd Bug 17981: Add a preview mode for notice templates
This patch is a first step to provide a preview mode for notice
templates.

CHECKIN, CHECKOUT and HOLD_SLIP are supported so far.
Maybe more, but I have not tested yet and the interface will not allow
you to generate the preview.

The idea is to provide an idea of how will render the messages generated
from a notice template.

A new "Preview" button is added close to each textarea on the editing
notice templates view.
For each notice template code (letter_code), we will need some input
data to produce the preview.

For instance, for CHECKIN we need an barcode. From the barcode we
can guess all the other data.
For CHECKOUT we will need the borrowernumber and the barcode.
Note that the way to enter the data for the preview is not really
user-friendly, for CHECKOUT you will have to fill
'borrowernumber|barcode', but the placeholder will help you to know how
and what to fill.
In the modal window, you will see 4 blocks:
1/ the content of the letter (with the placeholds << >>)
2/ the generated message (with the data filled)
3/ if the letter contained historical syntax markers, the screen will
try to generate a notice template using the TT syntax
4/ the generated message from this TT syntax
=> You will be able to compare the 2 generated messages.

What is the goal of this first patchset:
- Show this first POC and get feedback from other developpers
- Add a way to easily visualise the differences between the 2 syntaxes
- Confort users with the TT syntax and the migration step from the
historical syntax.

I'd like to get opinions before going further.

The possibilities:
- Mock data to get fully working generated messages for any notice
templates. For instance, for CHECKIN and CHECKOUT, the item is not
checked in/out yet. So we cannot access the issue's information.
(I have no idea how to do that)
- Browse the data to get the ones we want to use for the preview (big).

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-12 10:51:50 -03:00
f6155d98e0 Bug 17981: Add a 'letter' param to GetPreparedLetter
This will allow us to call GetPreparedLetter with a letter content that
does not exist in the DB

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

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-12 10:51:50 -03:00
b1f2c84e50 Bug 19578: Remove MARC punctuation in notices (TT syntax)
Jenkins fails (run 287) on a test in t/db_dependent/Letters/TemplateToolkit.t:

With the historical syntax:
        # Your request for an article from tQYRS (c3Av58O0P5xkkIGu) has been canceled for the following reason:

With the TT syntax:
        # Your request for an article from tQYRS_ (c3Av58O0P5xkkIGu) has been canceled for the following reason:

The last character of the biblio's title has been removed because it's a punctuation character.
It comes from: C4::Letters::_parseletter
 893         $val =~ s/\p{P}$// if $val && $table=~/biblio/;

The same replacement is done for patron's attributes too.

Test plan:
- Confirm that the new tests pass. That should be enough to confirm this change make sense.

Test plan (manual):
- Create a biblio with a title ending with a punctuation (like "with_punctuation_"), or any other fields of biblio/biblioitem
- Generate a notice which will display this field (CHECKIN for instance)

Use the historical syntax and the TT syntax, both should display the title without the punctuation character at the end

CHECKIN historical:
The following items have been checked in:
----
<<biblio.title>>
----

CHECKIN TT syntax:
The following items have been checked in:
----
[% biblio.title %]
----

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

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-04-04 15:45:43 -03:00
Mark Tompsett
b8a94b0a9a Bug 20474: Explicitly call sendmail to make mocking work
Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-27 12:19:31 -03:00
Kyle M Hall
830b5a7d7d Bug 19955: Add ability to process only one 'type' of message ( sms, email, etc ) for a given run of process_message_queue.pl
It would be nice to allow emails to be sent overnight, but limit the sending of SMS messages to hours when people are awake. Adding a type limit to process_message_queue.pl would allow this to be accomplished easily.

Signed-off-by: Charles Farmer <charles.farmer@inLibro.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-26 17:31:16 -03:00
Mark Tompsett
5b2e491415 Bug 18570: (QA follow-up) Improved POD
This tweaks the perldoc for SendQueuedMessages and adds
some for _get_unsent_messages.

TEST PLAN
---------

perldoc C4::Letters
-- look at _add_attachments (ugly), _get_unsent_messages (non-existent),
   SendQueuedMessages (no reference to borrowernumber or letter_code).
apply patch
perldoc C4::Letters
-- confirm that SendQueuedMessages and _get_unsent_messages
   have reasonable POD information.
-- notice how _add_attachments' POD is now readable

run koha qa test tools

Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-19 13:55:37 -03:00
Mark Tompsett
70941cde0e Bug 18570: Send Password Reset Emails immediately
Run through the password reset process, and your server
should send the message immediate, not waiting for the
cronjob.

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-03-19 13:55:37 -03:00
666cbed5e1 Bug 20144: [sql_modes] Fix GROUP BY clause in GetLetters
This subroutine is wrong and must be rewritten using
Koha::Notice::Templates.
Mainly because the DB structure is bad.
Meanwhile we remove the branchcode from the SELECT to get correct
results, it was not used by callers anyway.

Fix for:
'koha_kohadev.letter.module' isn't in GROUP BY

t/db_dependent/Letters.t

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>
Signed-off-by: Julian Maurice <julian.maurice@biblibre.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-02-13 13:58:54 -03:00
3a5534fcf5 Bug 19304: Move C4::Members::GetNoticeEmailAddress to Koha::Patron->notice_email_address
This subroutine is quite trivial and can be replaced easily with a new
method of Koha::Patron

Test plan:
Overdue notices and shelf sharing must be send the to an email address,
according to the value of the pref AutoEmailPrimaryAddress

Signed-off-by: David Bourgault <david.bourgault@inlibro.com>

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2018-01-02 11:46:40 -03:00
e46ab82e28 Bug 18990: Overdue Notices are not sending through SMS correctly
Bug 18478 fixed sms via email problems under the assumption that
to_address was either smsalertnumber or blank.

It seems overdues set the to_address to email. This patch changes the
code to enforce that an sms sent with emial driver will use the
smsalertnumebr and provider defined for the borrower, regardless of what
is set in the queue

To test:
 1 - Define a messaging prefs for a patron to recieve hold notices via
  SMS
 2 - Ensure you have defined an SMS message for an overdue letter
 3 - Set an SMS alert number for patron
 4 - Set the SMS::Send driver to 'Email'
 5 - Checkout an item as overdue to trigger notice above
 6 - Run overdue_notices.pl
 6 - Check the db and note the address is email
 7 - run process_message_queue.pl
 8 - Check db - address is email followed by service provider
 9 - Apply patch
 10 - repeat
 11 - Message to_address should be populated with smsalertnumber

Signed-off-by: Josef Moravec <josef.moravec@gmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-12-07 09:37:11 -03:00
ecb65def5a Bug 18894: (QA follow-up) POD changes
Add limit parameter to POD section.

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-08 13:35:28 -03:00
58bc11a7d4 Bug 18894: Add ability to limit the number of messages sent by process_message_queue.pl at a time
Having the ability to limit the number of messages sent by process_message_queue.pl on a single run would be very useful for controlling home many messages are sent at a given time. This can help prevent too many messages being sent out at once and getting flagged as a spammer.

Test Plan:
1) Apply this patch
2) Generate some number of messages in the message queue
3) Run process_message_queue.pl with the new --limit option,
   set limit to a number smaller than the number of pending messages
4) After the script has run, check the database and note that only
   a number of pending messages were sent, and that the remaining amount
   of pending messages is the original amount less the number specified
   as the limit

Signed-off-by: Mark Tompsett <mtompset@hotmail.com>

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

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-11-08 13:35:17 -03:00
5a64723e6d Bug 19277: TT syntax - Display ordered data
This issue has been highlighted by a failing test: ISSUESLIP displays checkouts in random order

I thought it was because of dates comparison, but it comes from the following line in C4::Letters::_get_tt_params
  my $objects = $module->search( { $key => { -in => $values } } );

The DBMS will return data like there is ordered in the DB.
For instance:
  select borrowernumber from borrowers where borrowernumber in (5, 3, 1);
or
  select borrowernumber from borrowers where borrowernumber=5 or borrowernumber=3 or borrowernumber=1;

will return 1, 3, 5

I did not find a generic way to do that, so used "ORDER BY FIELD" which will not be portable.

Test plan:
If you do not apply this patch, the tests will sometime fail

Signed-off-by: Lee Jamison <ldjamison@marywood.edu>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-12 11:29:11 -03:00
3aeb1fc7e8 Bug 19256: Make Koha::Acq::Order using Koha::Object
At the moment we have 2 different modules for acquisition orders:
Koha::Tmp::Order[s] and Koha::Acquisition::Order
The later has been added before the creation of Koha::Object.
Koha::Tmp::Order[s] has been created to make the TT syntax for notices
works with acquisition order data.

This patch removes the temporary packages Koha::Tmp::Order[s] and adapt
the code of Koha::Acquisition::Order[s] to be based on Koha::Object[s].

It also overloads Koha::Object->new to add the trick that was done in
Koha::Acquisition::Order->insert. This is needed because acqui/addorder.pl
is called from several places and CGI->Vars is used to retrieved order's
attributes (and so much more). To avoid regression, the easiest (but not
cleanest) way to do is to filter on aqorders column's names.
This is *not* a pattern to follow!

Test plan:
Create basket and add orders from different ways, then continue a whole
acquisition process

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
2017-09-07 15:03:04 -03:00