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

NEW(mailing): Allow user to define the number of email sent by batch directly from mailing config page #32750

Open
wants to merge 8 commits into
base: develop
Choose a base branch
from

Conversation

BenjaminFlr
Copy link
Contributor

Add a parameter in /admin/mailing.php to allow user to define a personal value for the number of emails sent by each batch when done from web interface.
mailing_config

@hregis
Copy link
Contributor

hregis commented Jan 22, 2025

@BenjaminFlr you broken the limit defined in conf.php?

@BenjaminFlr
Copy link
Contributor Author

@hregis I didn't know this parameter was available in conf.php, not sure how it would work with my code.

I have to check in the /comm/mailing/card.php file.

@hregis
Copy link
Contributor

hregis commented Jan 22, 2025

@BenjaminFlr la constante "MAILING_LIMIT_SENDBYWEB" est la valeur de "$dolibarr_mailing_limit_sendbyweb" si elle est défini dans conf.php. En fait il faudrait griser ou masquer ton option si "$dolibarr_mailing_limit_sendbyweb" est plus grand que 0 car elle prendra le dessus sur ton option.

@BenjaminFlr
Copy link
Contributor Author

C'est ce que je me disais. Mais par défaut, si ce n'est pas présent dans le fichier conf.php, la valeur est à 25. Donc même si tu n'as pas cette variable définie dans le fichier, tu as quand même une valeur.

Il faudrait donc que je varie la présence de la valeur dans le fichier, et j'affiche ou non cette option en conséquence. C'est bien comme ça que tu le vois ? Parce que la constante existe dans tous les cas par contre.

@hregis
Copy link
Contributor

hregis commented Jan 22, 2025

@BenjaminFlr oui il faudrait faire en sorte que si il y a une valeur dans conf.php ce sera celle-ci qui prendra le dessus, et sinon avoir une valeur par défaut sauf si la constante MAILING_LIMIT_SENDBYWEB existe bel et bien dans llx_const

@hregis
Copy link
Contributor

hregis commented Jan 22, 2025

@BenjaminFlr il faut regarder dans la classe /core/class/conf.class.php

@eldy eldy added the Discussion Some questions or discussions are opened and wait answers of author or other people to be processed label Jan 23, 2025
@BenjaminFlr
Copy link
Contributor Author

BenjaminFlr commented Jan 23, 2025

Alors pour le coup, de ce que je vois, et tu me corrigeras si je me trompe, mais :

  • Dans le fichier htdocs/install/step1.php, on a le fichier conf.php qui est automatiquement alimenté par cette ligne :
    fwrite($fp, '$dolibarr_mailing_limit_sendbyweb=\'0\';');
  • C'est bien dans htdocs/core/class/conf.class.php qu'est définie la valeur de MAILING_LIMIT_SENDBYWEB en fonction du fichier conf.php :
    $this->global->MAILING_LIMIT_SENDBYWEB = $this->file->mailing_limit_sendbyweb;

Donc, si je me trompe pas, il faut que je vérifie si la valeur est = 0 dans le conf.php, et pas uniquement sa présence. Et je pense donc conditionner l'affichage de cette option à l'existence ET une valeur différente de 0. Si la valeur existe et que la valeur > 0, alors je n'affiche même pas l'option. Qu'en penses-tu ?

Edit: Si je comprends bien, la valeur de $conf->file->mailing_limit_sendbyweb suffit pour la vérification, je vais modifier ma PR dans ce sens.

Edit bis : Ne serait-il pas aussi pertinent de supprimer cette constante de la page admin/conf.php ?

@BenjaminFlr BenjaminFlr changed the title NEW(mailing): Allow user to define the number of email sent by batch NEW(mailing): Allow user to define the number of email sent by batch directly from mailing config page Jan 23, 2025
@BenjaminFlr BenjaminFlr requested a review from eldy January 24, 2025 08:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Discussion Some questions or discussions are opened and wait answers of author or other people to be processed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants