Bug 10076 - Add Bcc syspref for claimacquisition and clamissues
Summary: Add Bcc syspref for claimacquisition and clamissues
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Notices (show other bugs)
Version: Main
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Charles Farmer
QA Contact:
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-04-18 18:52 UTC by Maxime Pelletier
Modified: 2017-06-14 22:11 UTC (History)
14 users (show)

See Also:
Change sponsored?: Sponsored
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:


Attachments
Patch to add the syspref (2.03 KB, patch)
2013-04-30 19:06 UTC, Maxime Pelletier
Details | Diff | Splinter Review
Bug 10076 - Fixed merge conflicts. (1.92 KB, patch)
2014-08-08 15:23 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for claimacquisition and claimissues (2.05 KB, patch)
2014-08-28 15:28 UTC, Paola Rossi
Details | Diff | Splinter Review
syspref.sql and updatedatabase.pl Added (2.31 KB, patch)
2014-09-05 13:29 UTC, simith.doliveira
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for claimacquisition and claimissues (1.99 KB, patch)
2015-07-08 17:51 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
syspref.sql and updatedatabase.pl Added (1.38 KB, patch)
2015-07-08 17:51 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 10076 - Atomic Update (907 bytes, patch)
2015-07-08 17:51 UTC, Maxime Beaulieu
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues (3.66 KB, patch)
2015-11-26 14:58 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues (3.94 KB, patch)
2015-11-26 17:35 UTC, Frédéric Demians
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues (3.93 KB, patch)
2015-12-01 13:58 UTC, Charles Farmer
Details | Diff | Splinter Review
Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues (4.07 KB, patch)
2015-12-03 09:20 UTC, Frédéric Demians
Details | Diff | Splinter Review
[PASSED QA] Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues (4.14 KB, patch)
2015-12-03 15:29 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Maxime Pelletier 2013-04-18 18:52:53 UTC
There is already a syspref called "OverdueNoticeBcc" for sending Bcc copies of mails sent for overdues and other notices. It would be useful to have a new preference to bcc the claimacquisition and clamissues alerts.

I will work on a patch if this is something that could be wanted.
Comment 1 Maxime Pelletier 2013-04-30 19:06:23 UTC Comment hidden (obsolete)
Comment 2 Srdjan Jankovic 2013-06-04 03:52:10 UTC
SendAlerts() is not used only from serials/claims.pl, but from members/memberentry.pl as well, which would cause unwanted Bcc. I suggest type is examined for Bcc as well
Comment 3 Maxime Beaulieu 2014-08-08 15:23:37 UTC Comment hidden (obsolete)
Comment 4 Paola Rossi 2014-08-28 15:25:26 UTC
I've applied the patch against master 3.17.00.016

I set the pref ClaimsBccCopy to "Send".

Everything is OK as required.

So I pass the patch to "Signed Off" status.
Comment 5 Paola Rossi 2014-08-28 15:28:21 UTC Comment hidden (obsolete)
Comment 6 Kyle M Hall 2014-08-29 14:35:54 UTC
This patch does not add the new system preference to updatedatabase.pl or sysprefs.sql.
Comment 7 simith.doliveira 2014-09-05 13:29:50 UTC Comment hidden (obsolete)
Comment 8 Amit Gupta 2015-07-05 06:12:50 UTC
simith any plan to rebase this patch on master?
Comment 9 Amit Gupta 2015-07-08 16:46:33 UTC
Hi maxime.beaulieu@inlibro.com,

Need this patch one of our client. Can i take on this bug?
Comment 10 Maxime Beaulieu 2015-07-08 17:51:23 UTC Comment hidden (obsolete)
Comment 11 Maxime Beaulieu 2015-07-08 17:51:32 UTC Comment hidden (obsolete)
Comment 12 Maxime Beaulieu 2015-07-08 17:51:44 UTC Comment hidden (obsolete)
Comment 13 Maxime Beaulieu 2015-07-08 18:06:02 UTC
Patch has been rebased to current master.
Comment 14 Frédéric Demians 2015-11-16 16:30:42 UTC
Since it's a small patch, it would be better to package it in a single patch. And could you use the capitalized version of the new syspref in SQL? ClaimsBccCopy rather than claimsbcccopy. Here:

VALUES ('claimsbcccopy','0','','bcc the claimacquisition and clamissues alerts','YesNo');

('claimsbcccopy','0','','bcc the claimacquisition and clamissues alerts','YesNo'),
Comment 15 Katrin Fischer 2015-11-17 13:15:56 UTC
Please make the small fixes suggested by Frederic! :)
Comment 16 Charles Farmer 2015-11-26 14:58:29 UTC Comment hidden (obsolete)
Comment 17 Frédéric Demians 2015-11-26 17:35:47 UTC Comment hidden (obsolete)
Comment 18 Katrin Fischer 2015-11-30 23:30:03 UTC
Just a request for a small follow-up here: 

The system preference is not nicely formatted and I think the wording could be a bit more clear. 

Currently it reads:
Send a copy in bcc of the ClaimAcquisition and ClaimIssues letters to the active user [Send|Don't send]

My suggestion after discussing with a native speaker (thx Liz :) ) would be:
[Send|Don't send] blind copy (BCC) to logged in user when sending serial or acquisitions claims notices.
Comment 19 Charles Farmer 2015-12-01 13:58:39 UTC Comment hidden (obsolete)
Comment 20 Katrin Fischer 2015-12-01 14:31:26 UTC
Hi Charles,
for a small change request like this doing a follow-up patch and keep the 'Signed-off' would be ok next time!
I just noticed comment#2 from Srdjan - can you comment on that?
Comment 21 Charles Farmer 2015-12-01 14:55:54 UTC
(In reply to Katrin Fischer from comment #20)
> Hi Charles,
> for a small change request like this doing a follow-up patch and keep the
> 'Signed-off' would be ok next time!
> I just noticed comment#2 from Srdjan - can you comment on that?

Hi Katrin,

Maxime already took care of it. This problem is avoided in the patch by first checking if the letter type is 'claimacquisition' or 'claimissues'. Never should a BCC be sent otherwise.

I unfortunately cannot unit test this part though, because the email is never queued anywhere. But from the code, it looks like no unwanted BCC can be sent.
Comment 22 Katrin Fischer 2015-12-02 21:47:09 UTC
Thx, I can see it now! So all we need is another sign-off now - or the old signed patch + a new follow-up dealing with the string changes.
Comment 23 Frédéric Demians 2015-12-03 09:20:53 UTC Comment hidden (obsolete)
Comment 24 Katrin Fischer 2015-12-03 15:29:52 UTC
Created attachment 45361 [details] [review]
[PASSED QA] Bug 10076 - Add Bcc syspref for ClaimAcquisition and ClaimIssues

There is already a syspref called "OverdueNoticeBcc" for sending Bcc
copies of mails sent for overdues and other notices. This patch add a
new syspref ClaimsBccCopy to bcc the claimacquisition and clamissues
alerts.

Changed the wording of the system preference to:

[Send|Don't send] blind copy (BCC) to logged in user when sending
serial or acquisitions claims notices.

Signed-off-by: Frédéric Demians <f.demians@tamil.fr>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 25 Brendan Gallagher 2015-12-31 20:40:40 UTC
Pushed to master - Thanks