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 feature: Make invitation/reminder sending rate configurable #1636

Merged

Conversation

ViliusS
Copy link
Contributor

@ViliusS ViliusS commented Nov 2, 2020

Dev: This patch makes currently hardcoded 20 second automatic email sending rate configurable.

Dev: Most SMTP providers limit sending rates on their side. This patch, combined with "Email batch size" parameter, now allows for a very flexible solution to the problem. For example, you can set LimeSurvey to send 30 emails per 1 minute OR you can set it to 1 email per 2 seconds which could be a big improvement on some SMTP platforms to not being blocked by spam filters.

Dev: While at it, convert related strings to be translatable, allow for plural translation forms and make notice to the survey administrator more sensical when a "Cancel automatic sending" button is clicked.

With small adjustments in _email.php (<span> -> <p>) the patch could be merged to LS3.x branch too.

P.S. This patch is against master because https://github.com/LimeSurvey/LimeSurvey/blob/master/CONTRIBUTING.md says it should be so, but only after I pulled a request github PR template said it should be in develop?

P.P.S. I could not create a ticket on bugs.limesurvey.org because I'm using my github account to login to limesurvey.org, but it seems Mantis doesn't support this?

Dev: This patch makes currently hardcoded 20 second automatic email sending rate configurable.

Dev: Most SMTP providers limit sending rates on their side. This patch, combined with "Email batch size" parameter, now allows for a very flexible solution to the problem. For example, you can set LimeSurvey to send 30 emails per 1 minute OR you can set it to 1 email per 2 seconds which could be a big improvement on some SMTP platforms to not being blocked by spam filters.

Dev: While at it, convert related strings to be translatable, allow for plular translation forms and make notice to the survey administrator more sensical when a "Cancel automatic sending" button is clicked.
@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 2, 2020

I like it !

@Shnoulle Shnoulle self-requested a review November 2, 2020 12:11
@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

I make some test and review it again :)

@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

Did you kow if you can move to develop branch ? Seems new feature must go to develop branch (see my pull request)

@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 3, 2020

It should be compatible with develop branch however I don't know how to transfer to it now. Should I create a new pull request for develop branch or can someone change it from your side?

P.S. CONTRIBUTING.md and other documentations really needs to be updated, because now there are mismatches all over the place. I'm still not sure which branch I need to use.

@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

It should be compatible with develop branch however I don't know how to transfer to it now. Should I create a new pull request for develop branch or can someone change it from your side?

Unsure : personaly i merge locally develop … ans start fixing …

P.S. CONTRIBUTING.md and other documentations really needs to be updated, because now there are mismatches all over the place. I'm still not sure which branch I need to use.

Yes … sure … i still fix my pull request … b4f74e8

@ViliusS ViliusS changed the base branch from master to develop November 3, 2020 14:56
@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 3, 2020

Should be OK now.

@Shnoulle Shnoulle self-assigned this Nov 3, 2020
@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

Really like this !!
Mail one by one each second :)

Peek 03-11-2020 16-41

@Shnoulle Shnoulle merged commit f774b94 into LimeSurvey:develop Nov 3, 2020
@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

Should be OK now.

Thanks a lot !

@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 3, 2020

Could you merge this to LS3.x too? I know that there is a consensus to not add new features to LS3.x anymore, but since LS4 is still not stable and this is more a change to existing feature than a completely new feature, it would be great to have it in LS3.x We are still using it extensively.

@Shnoulle
Copy link
Collaborator

Shnoulle commented Nov 3, 2020

Not me … @cdorin93 can answer

@ViliusS
Copy link
Contributor Author

ViliusS commented Nov 10, 2020

@c-schmitz , @cdorin93 any chance to have this merged to LS3.x? I would really hate to have to manage this patch on my side for such simple and useful change until LS4 becomes stable.

@cdorin93
Copy link
Contributor

cdorin93 commented Dec 14, 2020

Hello @ViliusS , and thank you for the nice feature.
It will be merged to develop. We hope to bring LS4 to stable as soon as possible - 4.4 will be a an important release and we hope to have many of the issues solved with this release.

LE: corrected the last sentence.

@maziminke
Copy link
Collaborator

@cdorin93 Since it will surely take quite some time until LS 4.x is really stable, can you please consider merging this into LS3.x as well? Many, many users will benefit from this!

@ViliusS Thanks a lot for this great contribution!

@ViliusS ViliusS deleted the feature/configurable-SMTP-sending-rate branch March 26, 2021 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants