-
Notifications
You must be signed in to change notification settings - Fork 778
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[IP-362] - Modify the way we can setup outgoing mails to BCC #574
base: do_NOT_USE_develop
Are you sure you want to change the base?
Conversation
Master to 1.6
This fixes the 500 error on php5.6
Cuando se añade una cuenta de usuario de tipo invitado. Al añadir los clientes que puede ver, aparece una nueva opción que nos permite añadir todos los clientes automáticamente. También cuando se cree un nuevo cliente este se añadira a todos las cuentas de usuario que tengan esta opción habilitada. Muy útil para cuentas de usuario que usarán en contabilidad. --- When a user type user account is added. When you add the clients you can see, a new option appears that allows us to add all clients automatically. Also when a new client is created this will be added to all user accounts that have this option enabled. Very useful for user accounts that will be used in accounting.
Corregida la frase de la variable user_all_clients_text --- Corrected the phrase of the variable user_all_clients_text
The same custom fields will be available as what is available in a PDF template. Forum topic: https://community.invoiceplane.com/t/topic/4223/26
As $mdl_invoice_tax_rates is not available in the copy_invoice function we load the corresponding model first. Couldn't reproduce the error but this should work.
+ various smaller code corrections
Pull changes from v1.5.6
…s it's not needed anymore)
@@ -37,8 +37,7 @@ | |||
'back' => 'Back', | |||
'base_invoice' => 'Base Invoice', | |||
'bcc' => 'BCC', | |||
'bcc_mails_to_admin' => 'Send all outgoing emails as BCC to the admin account', | |||
'bcc_mails_to_admin_hint' => 'The admin account is the account that was created while installing InvoicePlane.', | |||
'bcc_mails_to_admin' => 'Enter one or multiple email(s) to send outgoing emails as BCC', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe mentioning, comma seperated :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup thanks, will modify it
$CI->db->where('user_id', 1); | ||
$admin = $CI->db->get('ip_users')->row(); | ||
$mail->addBCC($admin->user_email); | ||
$bcc_admin = (strpos(get_setting('bcc_mails_to_admin_email'), ',')) ? explode(',', get_setting('bcc_mails_to_admin_email')) : explode(';', get_setting('bcc_mails_to_admin_email')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please keep it backward compatible. So if bcc_mails_to_admin == 1, then bcc_admin contains the email of the admin user.
If bcc_to_mails_to_admin_email != empty (and the new setting is leading), use that value.
Another thing, either it should support separated by comma or semi-colon, so don't check if comma is in the string.
Plus, you can do explode(',', $var) and still get an array, if even there is no comma in it, so no need of strpos()
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the backward compatible it's in the ToDo, the setup function that has to be made to save the field with the email of the admin user if bcc_mails_to_admin == 1 (function that will be run with the setup) but it has to be done.
Hmm so what do you suggest for the comma/semi-colon problem ?
To be honest I re-use the same way it was done for the $to variable, $cc, or $bcc (https://github.com/InvoicePlane/InvoicePlane/blob/master/application/modules/mailer/helpers/phpmailer_helper.php#L101) to use the same code at each part and not have any difference between two similar lines but maybe this has to be changed even for $to
$cc
$bcc
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the BC, sorry I didn't read your original comment with the PR. You already mentioned that.
With setup, you mean the upgrade setup? Not the "new-setup-wizard"? If yes, then that's fine with me
About the strpos(), in that case, keep it as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No problem mate ;)
Yeah the upgrade setup like this function (https://github.com/InvoicePlane/InvoicePlane/blob/master/application/modules/setup/models/Mdl_setup.php#L238),
Like a public function upgrade_029_1_5_6()
and write the code to check if bcc_mails_to_admin == 1
if that is the case then insert the bcc_mails_to_admin_email
field in the DB with the email of the admin user, if not just leave the function. If I don't do it, it's just because I don't want to fucked all my dev env on my computer (delete all, re-install all to run the upgrade function) but yeah the goal is to add a function which will be launched during the upgrade process :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Fixed for the translation (comma separated) thanks @aalwash |
IP-612 Fixed Mollie payments
Update 1.6.0 from 1.5.6
# Conflicts: # application/modules/guest/controllers/Payment_handler.php
@kalimerre What is the current status of this PR? |
The ToDo in my original post |
Needs some work before we can merge. @kalimerre can you work on that Todo please? |
https://development.invoiceplane.com/browse/IP-362?filter=11802
Need ToDo
bcc_mails_to_admin
is set to "1"I remove the hint in the language as it's not needed anymore, if you really want it then I can re-add it
Not merge before the setup function has been made or users won't have his mails as BCC anymore