Skip to content
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

Open
wants to merge 31 commits into
base: do_NOT_USE_develop
Choose a base branch
from

Conversation

kalimerre
Copy link
Member

@kalimerre kalimerre commented Nov 26, 2017

https://development.invoiceplane.com/browse/IP-362?filter=11802

Need ToDo

  • The setup function to save the admin's email if old option 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

Kovah and others added 25 commits September 18, 2017 11:50
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
@kalimerre kalimerre changed the title [IP-362] - Setup the input and modify the language [IP-362] - Modify the way we can setup outgoing mails to BCC Nov 26, 2017
@@ -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',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe mentioning, comma seperated :)

Copy link
Member Author

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'));
Copy link
Contributor

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()

Copy link
Member Author

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

Copy link
Contributor

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.

Copy link
Member Author

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 :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@kalimerre
Copy link
Member Author

Fixed for the translation (comma separated) thanks @aalwash

@kalimerre kalimerre changed the base branch from v1.5.6 to v1.6.0 February 18, 2018 12:35
@Kovah Kovah added this to the 1.6.0 milestone Apr 9, 2018
@Kovah
Copy link
Contributor

Kovah commented Jun 28, 2018

@kalimerre What is the current status of this PR?

@kalimerre
Copy link
Member Author

The ToDo in my original post
The setup function to save the admin's email if old option bcc_mails_to_admin is set to "1"

@UnderDogg UnderDogg changed the base branch from v1.6.0 to develop January 23, 2021 12:41
@UnderDogg
Copy link
Contributor

Needs some work before we can merge. @kalimerre can you work on that Todo please?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants