Early Bird Registration for DrupalCon Portland 2024 is open! Register by 23:59 PST on 31 March 2024, to get $100 off your ticket.
I've added quite a lot of code to this module, in order to get it to do everything I need. The two main features I've added to it are:
- Backend UI for each mailing list, allowing individual name / email pairs to be listed and edited.
- Ability to import emails into a mailing list from a CSV file
I've also moved the admin area from 'admin/settings/mailing-list' to 'admin/build/mailing-list'.
Hope others find this patch useful - and if it's worth it, might even be good to get it committed back.
Note: this patch depends on these two other patches in the queue:
Comment | File | Size | Author |
---|---|---|---|
#6 | mailing_list_admin_ui_fixindices.patch | 4.55 KB | Jaza |
#4 | mailing_list_admin_ui_1.patch | 35.14 KB | Jaza |
mailing_list_admin_ui.patch | 21.49 KB | Jaza |
Comments
Comment #1
litwol CreditAttribution: litwol commentedHello Jaza,
Are you using this on production site ?
If you would give the two other issues you've mentioned a thorough review and set them to RTBC i would be willing to commit them soon.
Thanks, let me know.
Comment #2
Jaza CreditAttribution: Jaza commentedHi litwol,
I'm currently using this module on a site that's in beta, and that's scheduled to go live next week. I've reviewed the patches in the other two issues, and they both work and look reasonably well-written to me. I've set both of them to RTBC.
Comment #3
litwol CreditAttribution: litwol commentedPlease reroll this patch. It failed to apply after i commited the other two mentioned patches.
Comment #4
Jaza CreditAttribution: Jaza commentedNew patch, which has quite a few improvements over the old one:
mailing_list.admin.inc
.Comment #5
litwol CreditAttribution: litwol commentedThank you!
I really enjoyed the new functionality adition, good job. I've commited it.
Let me know if there is something more you wish to add. I think soon would be a very good time to make an official release.
Comment #6
Jaza CreditAttribution: Jaza commentedThanks for committing my big patch, litwol.
Mostly looking good... except, I discovered that I made one big mistake with the DB schema changes. I made a unique key index for the 'mail' field in the 'mailing_list_emails' table. However, the unique key should be for the combination of 'mlid' and 'mail', because an e-mail should only be unique in the context of the mailing list that it's in.
I've patched the same update function, rather than writing a new update function, as I doubt that anyone has upgraded in the 6 hours since this change was committed (plus this module currently has no stable releases).
Also, a few other minor fixes:
'#required' => TRUE
, so it's validated automatically)'#required' => TRUE
to the 'mail' field in subscription block formI agree, an official release should be made soon.
Comment #7
litwol CreditAttribution: litwol commented@Jaza
I dont think @name field should be required. My point about disagreement is to point out that some one else may also have similar opinion. therefore i think it is a good idea to make name field optional. Each mailing list could have a checkbox that says 'require name'.
What do you think of that ?
In the meanwhile i'm commiting your patch :)
Comment #8
Jaza CreditAttribution: Jaza commented@litwol
The 'name' field is not required. I agree that it's best to allow site admins to choose whether or not to have this field for each of their mailing lists. That's why my main patch in this thread added a checkbox to the configuration form for each mailing list's subscription block, called 'Show name field in subscription form'.
When this checkbox is ticked, the 'name' field is shown and is required in the subscription block form. When this checkbox is un-ticked, the 'name' field is hidden (and is obviously not required). I hope that fits with how you want it to work too. Or... are you saying that this checkbox should be in the mailing list add/edit form, rather than in the config form for a mailing list's subscription block? That sounds fine to me as well.
Anyway, thanks for committing the patch so fast!