jazzband / django-newsletter

An email newsletter application for the Django web application framework, including an extended admin interface, web (un)subscription, dynamic e-mail templates, an archive and HTML email support.
GNU Affero General Public License v3.0
848 stars 206 forks source link

Improve submission recipient selection widget #197

Open mullerivan opened 8 years ago

mullerivan commented 8 years ago

screenshot at 2016-03-23 14 20 56

How to redefine the receipts widget on submission because i have a very big list, and none server could render that in time. I would like to have another widget to fix my bug, any idea what would be the vest idea How is the best way to fix that?, I would like to push a pull request to improve the newsletter

cheers

dokterbob commented 8 years ago

Rethinking the submission interface is indeed something we ought to rethink, especially for large lists of recipients. I know some people have worked on simply selecting all recipients automatically, but I don't think it would be good to make this the default behaviour.

I see two viable options for this:

  1. Introduce a configuration option which hides manual selection of recipients entirely and selects all subscribers by default.
  2. Select all subscribers by default and create an optional AJAX-based manual refinement of subscribers.

What do you think?

mullerivan commented 8 years ago

number 2 is the best option fare away, how python say with battery included (sorry for the delay on may answer) at the moment my fast ugly solution was to change newsletter/admin.py raw_id_fields = ('subscriptions',)

dokterbob commented 8 years ago

It might be preferred from a user perspective, it does provide a higher maintenance load and is more complex to configure. If you would provide a pull request I would gladly review it and give feedback but I cannot implement this myself (due to time constraints).

mullerivan commented 8 years ago

I'll just im in holidays now :-)

dsanders11 commented 8 years ago

I've also run into this and tracked the major cause to be slow performance in the admin SelectBox.js code. After making some improvements to it and a few hacks to get better performance it was merged into Django and should perform much better once Django 1.10 is released. In my testing I was using a subscription list of 20,000 subscribed and after the improvements in Chrome it all worked well after about 5 seconds of load time when going to the submission page. It doesn't work quite as fast on Safari, but it's still usable now.

@mullerivan, how big is the subscription list you're dealing with?

I also got better server performance by tweaking the admin form for submissions to only select the fields needed in the database query, this better used indexes and dropped the database query time from about 500ms to 70ms in my testing. I'll clean up that tweak and try to get it into the codebase here.

@dokterbob, perhaps for a long-term solution the subscriptions field should automatically hide after a max size (configurable via a setting). The AJAX approach, while interesting, doesn't provide an easy way to remove someone who is automatically added to the subscription list. You'd need to be able to see the selected subscriptions for that, and then we're sort of back to square one. What if we punted the problem and added a changelist action for subscriptions which was 'Create submission for subscriptions...' which created a submission (and let you choose the message) for the selected subscriptions. Then any filtering of the subscriptions could be done on the changelist, and if that's not fine-grained enough there are several 3rd party apps which let you create more advanced filters for the changelist.

mullerivan commented 8 years ago

@dsanders11 hey there,.my list is for 60.000 users per list.... I really like the idea to have max size for to show the list limit 8000 or configurable by settings,

I.sort it out with

mullerivan commented 8 years ago

Sorry I close it for using the web app from my phone.

dokterbob commented 8 years ago

@dsanders11 Cool work! Perhaps we could temporarily include your patch in django-newsletter for a short-term fix until Django 1.9 is phased out?

With regards to the more structural solution: I don't think a changelist action would do, even though it seems to. Changelists are for actions on multiple items and I don't think we should encourage that for mass mailings; when sending email to a bunch of thousand people it really does seem better to confirm each and every one of them. Besides this, it really does seem better to leave the current confirmation mechanism in place; creating a submission first and then confirming it.

HOWEVER, I would really really really love to see the submission mechanism reworked. Not just for large mailingst but in general, this could be so much more human-friendly.

I guess for this, custom forms could be the way. After a Submission create form, which should really just be a selection of a Message, there could be a clear confirmation dialog with a big fat call to action for confirmation (rather than a small button on the top right). From this place, there could be a... small button on the side directing users to the pre-existing sluggish change form.

This is relatively little work and would make the life of all users of django-newsletter a lot easier IMHO. So, what do y'all think? :) @mullerivan

newearthmartin commented 8 years ago

Hi guys, what is the state of this issue? I m having the same problem with 3k subscribers only. IMO I see that sending to all recipients in a list is the norm and selecting one or two would be the exception. If that becomes the norm, then it is a new list ;) take care!

dokterbob commented 8 years ago

I still really really really would love to see the submission mechanism reworked.

Basically, the Submission create form should just select a Message and default to adding all subscribers. Similarly, the normal change form should only have a Message field. However, there should be an action button allowing explicit selection of recipients.

Also, preferably a BIG FAT call to action on the normal edit form for submitting the current message would be great.

adi- commented 6 years ago

I think the widget is ok. The problem is with query. For each e-mail address admin is making one additional query. So, to list 10k email, there are 10k queries. This is not acceptable. I would suggest to optimize query in submission add admin view (using select_related() and prefetch_related())

dokterbob commented 6 years ago

That sounds like a good addition but still, loading 35k of ANYTHING in the user agent is undesirable. There’s simply no way a user can navigate such a list.

You are welcome to experiment with further query optimization but I am really strongly in favor of a UI which does not require the full list of subscribers in the same HTML page.

Op 30 mei 2018, om 15:12 heeft adi notifications@github.com het volgende geschreven:

I thing the widget is ok. The problem is with query. For each e-mail address admin is making one additional query. So, to list 10k email, there are 10k queries. This is not acceptable. I would suggest to optimize query in submission add admin view (using select_related() and prefetch_related())

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

newearthmartin commented 6 years ago

@adi- if it is such a small fix can you test it and create a pull request?

adi- commented 6 years ago

Yes, I can create one.

But, I think the better option is not to select each mail one by one. The idea about newsletters and its subscriptions is, that ppl are signing up to newsletter. When new message is submitted, it should be sent to all users that are subscribed to that newsletter.

So, my point is why anyone would want to select those emails by hand? It could be an addition, but not a general workflow.

dokterbob commented 6 years ago

Well, one very concrete case example is testing submissions. Especially with large groups, it makes a lot of sense to send the same email to a much smaller group of test recipients - i.e. to check if things render well etc.

Anyways, any proposed solution should incorporate backwards compatibility, at least on a data level.

I'm open to suggestions for an improved interaction flow within the constraints above (but note that it would actually have to be implemented and have test coverage.)

adi- commented 6 years ago

@dokterbob in that case another newsletter should be created.

dokterbob commented 6 years ago

It's about testing a specific message within a specific newsletter, I'm afraid your suggestion will not suffice.

adi- commented 6 years ago

@dokterbob Everything is fine. When you want to test a newsletter, there should be an option to paste e-mail addresses manually for testing. You don't have to list all subscribed e-mail addresses for that reason.

dokterbob commented 6 years ago

Sure, you’re hereby invited to work out to further explore the concept and share the result with us for feedback. If other users like this as well, the code is nice and there is test coverage I’d be happy to merge.

In general, the submission interface has been an ugly part I’ve been hoping to fix over the course of years...