search

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
846 stars 204 forks source link

longer name field for subscription #344

Closed newearthmartin closed 3 years ago

newearthmartin commented 3 years ago

The subscription name field is too short, only 30 chars. I'm upgrading it to 200, the same value of all the other fields that are names or title.

frennkie commented 3 years ago

The title is used in quite a few places. Both in the UI, Admin pages and the newsletters that are send out.

Simply changing this from 30 to 200 char might allow for unexpected negative effects in the user experience, or no?!

newearthmartin commented 3 years ago

I have users whose complete names are longer than 30 chars. So 30 chars is a really low limit. I dont think any UIs or Admin pages will be broken with a longer name (Newsletter, Message and Article titles are already 200 long). I dont know what others think, maybe not increase it to 200, maybe 100 or even 60.

frennkie commented 3 years ago

Oh - sorry.. I misread your request and PR. I thought you were referring to the Newsletter Title.

Regarding the subscription name field.. I don't see an issue with extending it! :-D

A remaining issue is that - as in #348 - I think having the migration number increasing continuously would be a good thing.

newearthmartin commented 3 years ago

so should I rename it to 0007 or 0008 ?

frennkie commented 3 years ago

I would suggest we wait for feedback from @dokterbob.

newearthmartin commented 3 years ago

I renamed it to 0008 but still the dependency is marked as 0006 When this is merged #348 we can update the dependency also Waiting for news from @dokterbob

dokterbob commented 3 years ago

@newearthmartin Yes, let's do it. 008 Let's hope anyone who cloned it while we had 2 007's knowns how to rewind it. ;)

newearthmartin commented 3 years ago

OK, now its 0008 and pointing to the 0007 dependency, ready to merge!