search

emencia / emencia-django-newsletter

An app for sending newsletter by email to a contact list.
189 stars 72 forks source link

fixed 999 sqlite3 variable limitation crashing the newsletter creation #51

Closed danielsokolowski closed 12 years ago

danielsokolowski commented 12 years ago

By default all available newsletter contacts are selected during mailing list creation which causes a sqlite3 variable limitation crash, to avoid this the code has been modified so no contacts are automatically added and hence the sqlite3 limitation is only triggered when attempting to save.

Fantomas42 commented 12 years ago

Hi danois,

thank you for your pull request, but if I understand well your patch, the mailing lists created will be empty of contacts ?

If so, I don't see the interest of creating a mailing list from the contact list admin view.

Does I miss something ?

Regards

danielsokolowski commented 12 years ago

Hi Fache,

No you are perfectly correct --- the mailing list starts empty --- however this allows to create mailing lists at least up to 999 whereas before an error would be thrown right away as the code greedtly tries to select all available contacts. This change allowed the client to split his contacts into A-H, H-Z etc.

So rather then critical failure, the limitation only creeps up if the limit is exceeded; in my mind an acceptable compromise to porting the site to Portage SQL or MySQL

Thank you

On 05/01/2012 5:21 AM, Fache Julien wrote:

Hi danois,

thank you for your pull request, but if I understand well your patch, the mailing lists created will be empty of contacts ?

If so, I don't see the interest of creating a mailing list from the contact list admin view.

Does I miss something ?

Regards

Reply to this email directly or view it on GitHub: https://github.com/Fantomas42/emencia-django-newsletter/pull/51#issuecomment-3367912

danielsokolowski commented 12 years ago

Hi,

If I improved the patch to only perform the work around for sqlite database would you consider merging it then ?

Fantomas42 commented 12 years ago

Hi,

yes it's may be a good idea. With a warning handle by the messages app, it can be a must.

Regards

danielsokolowski commented 12 years ago

Cherry pick this commit https://github.com/danols/emencia-django-newsletter/commit/af9d64a75e6c7aca9359b198b30793088d1623c3

Any further tweaks you require sir ? :)

Fantomas42 commented 12 years ago

Hi danols,

thank you for your work and suggestion !

Fantomas42/emencia-django-newsletter@fb6fefb985888a9bc0294ad40a31cb68a5abdb1f should fix the issue.

As you can see I have a little bit adapted your code, but the essential is still present.

Regards.

danielsokolowski commented 12 years ago

Wonderful and thank you.