Upgrade Simplenews to Drupal 7

Subscribe.
And yikes. We should get a move on with #536620: Subscriber API: separate subscribers from newsletter management and allow other modules to define subscribers!

@jurgenhaas: do you want to make a start with porting Simplenews to D7? I can support, but don't have the time now to do it myselves. I can create a 6.x-2.x branch to mark the new functions already added to HEAD. The port can then be build in HEAD.

I've created a 6.x-2.x branch. We can now start with the port to D7. I like to join the #D7UX and am looking for more developers who want to yoin us to port Simplenews. Please state your intention here. PM me for more info.

Attached patch is now committed and adds todo notes based on http://drupal.org/node/224333. Anyone who wants to get their hands dirty ...

First upgrade by Coder is now committed too!

Apparently I can't do a straight port, many thing have changed since I started upgrading Simplenews. Lets start with the terminology.

New terminology:

Newsletter

A node, to be sent as email

Newsletter category

A collection of newsletters. Each newsletter can be tagged with one categories.

Mailing list

Where people can subscibe to. Usually the collection of addresses the newsletters of one category get sent to.

Subscriber

Someone with an email address that is (or was) subscribed to a mailing list.

Subscription

The subscription of a subscriber to a mailing list

The terminology is reflected in the schema.

The schema

At some point I did introduce a table simplenews_mailing_list. The subscriber was not subscribed to the category, but to a mailing list. The mailing list in its turn was related to the newsletter category. This was to enable future changes where newsletters can be send to arbitrary lists. For example a views generated list of subscribers filtered by postal code. Further this would allow the subscription part of simplenews to be separated from the mail building part and live as a separate module. But it was too heavy lifting and I reverted the changes.

Another major change I reverted was about taxonomy.

Taxonomy

Simplenews uses taxonomy terms to identify the newsletter categories. Each newsletter node must contain one taxonomy term of a selected vocabulary. There a several negative side effects to this choice: Incompatibility of Categories module; In combination with Taxonomy Access Control modules uses must have permission to view the term in order to be able to subscribe to the related category/mailing list; The relation between a taxonomy term and a node of newsletter content type fails in certain configurations (e.g. #368183).
To replace the taxonomy terms, new ids were introduced for Category (scid) and mailing list (slid), a simplenews_access module was written to replace the taxonomy access control option and parts of views integration were developed. But also this failed due to lifting capacity. A lot of code needed to be written to replace all that comes free if taxonomy is used. The code can still be found in HEAD, but the latest dev release uses taxonomy again.

What else

These are the major changes that occured. Further a number of database function are introduced to give simplenews (and related modules) a uniform interface to the simplenews_* tables.

• simplenews_category_load()
• simplenews_categories_load_multiple()
• simplenews_category_save()
• simplenews_category_delete()
• simplenews_subscriber_load()
• simplenews_subscribers_load_multiple()
• simplenews_subscriber_save()
• simplenews_subscriber_delete()
• simplenews_subscription_save()
• simplenews_subscription_update()
• simplenews_subscription_delete()

A number of hooks are introduced by simplenews. Documentation will be in simplenews.api.php

• hook_simplenews_recipients_alter()
• hook_simplenews_category_insert()
• hook_simplenews_category_update()
• hook_simplenews_category_delete()
• hook_simplenews_mailing_list_insert()
• hook_simplenews_subscriber_update()
• hook_simplenews_subscriber_insert()
• hook_simplenews_subscriber_delete()

That's enough for now ;) Any questions?

That's great news.

The current terminology is somehow misleading (newsletter, list, ? issue) and even tables are named differently in database.

Some parts of this unification might be subject of the 6--2 feature branch.

The new API will make life easyer. :-)

subscribing.

I'm still a student, so maybe I'm waaaay offbase, but isn't having a _load and load_multiple function a case of introducing unnecessary complexity? Why not just a load function? I don't mean to be a pest by asking :)

Relating to the suggestions from Sutharsan about function names:
Regarding simplification of naming, we should always use singular namings.

# simplenews_category_load()
# simplenews_category_load_multiple()
...
# simplenews_subscriber_load()
# simplenews_subscriber_load_multiple()

We shouldn't switch category to categories and subscriber to subscribers. Always remain on the entity naming - no (natural) language specific rules.
In DB-Design it is always 100% clear that a table should be named singular (e.g. user instead of users). Drupal has many such misdesign relicts. I suggest to also follow these guidelines for api design.

Is there any help needed to port the module to D7 ? I need it for my business so I can give a hand on tasks.

And Drupal 7 is out ! Its time to release

Let me know ASAP :)

We started porting D6.x-2.x to D7 recently.
However there's much work remaining.

While we're currently working on a clean D7 base functionality, there are e.g. many tasks to do.
Simply start checking our port queue or try the code and provide us patches for issues found.
Also taking the D7 active queue and start providing patches would make us happy

There will be daily progress on -dev. So make sure you're always using the latest work as a base.
In addition there are some functional unimplemented parts in D7.

I'm pretty sure we'll be able to fill months of work regarding D7.

The last -dev release on the project page is from Feb 2010, are you speaking about cvs ?

The date is incorrect, the last 7.x-1.x-dev is currently updated every day. (caution, you can download 7.x-1.x-dev.tar.gz or HEAD.tar.gz, I suspect there is something wrong with that, but I don't know why ;).

Ok thanks for the info, I didn't know about this date bug.

Right, the date on the project page is the creation date of the project node.
The provided snapshot is really up-to-date.

This is an issue of project.module (drupal.org engine) that should display the tar.gz date instead of the project node creation date for dev releases.

Info file:

Module .info files can have configure line
----------- htdocs/sites/all/modules/mail/simplenews/simplenews.info ----------
diff --git a/htdocs/sites/all/modules/mail/simplenews/simplenews.info b/htdocs/sites/all/modules/mail/simplenews/simplenews.info
index ab7267f..53cdb0f 100644
--- a/htdocs/sites/all/modules/mail/simplenews/simplenews.info
+++ b/htdocs/sites/all/modules/mail/simplenews/simplenews.info
@@ -16,6 +16,7 @@
 files[] = simplenews_handler_field_newsletter_s_status.inc
 files[] = simplenews_handler_filter_newsletter_s_status.inc
 files[] = tests/simplenews.test
+configure = admin/config/simplenews
 
 ; Information added by drupal.org packaging script on 2010-02-26
 version = "HEAD"

Fix php syntax
htdocs/sites/all/modules/mail/simplenews/simplenews_handler_field_newsletter_s_status.inc
diff --git a/htdocs/sites/all/modules/mail/simplenews/simplenews_handler_field_newsletter_s_status.inc b/htdocs/sites/all/modules/mail/simplenews/simplenews_handler_field_newsletter_s_status.inc
index 99bf42a..8795b5c 100644
--- a/htdocs/sites/all/modules/mail/simplenews/simplenews_handler_field_newsletter_s_status.inc
+++ b/htdocs/sites/all/modules/mail/simplenews/simplenews_handler_field_newsletter_s_status.inc
@@ -12,7 +12,7 @@
  */
 class simplenews_handler_field_newsletter_s_status extends views_handler_field {
   function render($values) {
-    switch ($values->[$this->field_alias]) {
+    switch ($values->{$this->field_alias}) {
       case SIMPLENEWS_STATUS_SEND_NOT:
       default:
         return t('Not sent');

Can you work with this kind of patch ? I'm not sure about the file path to give.

Just checked out contributions/modules/simplenews from the HEAD branch but it's very different from the version I get from drupal project home page. Can't find the D7 branch.

I'll apply later.
It's the MAIN branch!
Right, your files[] lines refer not to the latest version.

@ Sylvain Lecoy

The best habit to into for patches is to patch from the individual mymodule dir (not the modules dir that might contain mymodule). I think there are more reasons, but if for nothing else, this allows people using drush_make to apply your patches from a make file. Anyone using drush_make will need to reroll your patches otherwise :)

So instead of htdocs/sites/all/modules/mail/simplenews/simplenews.info, you want mail/simplenews/simplenews.info.

Cheers,

@miro_dietiker: Double check the archive, this is not the cvs version then.

See
http://drupalcode.org/viewvc/drupal/contributions/modules/simplenews/sim...

As stated: the updates of the dev have some delay - this happens automated. So please use a cvs checkout if you participate development and like to submit patches.
Also CVS will help you creating those patches.

Here is a patch which fixe some typo issues, fixe the simplenews.js and make the tests working (not necessary validating).

in .info file you don't need to include every files. According to http://drupal.org/node/224333#registry, only php files which include a class definition need to be written into the .info file. This is to automatically load the file concerned when you declare a class.

You can add a configure tag like this: configure = admin/config/simplenews

I found that the Simplenews block in admin panel is not shown but I don't know why.

Sylvain,
Thank you, this looks great in general.
However: Never mix codestyle fixes, documentation fixes and functional changes. Always separate them...

Could you please split the codestyle from the functions?
I'd like to commit them separately.

Also: This issue here is about general D7 discussions, not specific issues.
Please reopen single issues for every additional issue you find. we'll happily commit them separately and have a clean history of what we did.
(Please do so for the above patch parts.)

Note that submitting formatting corrections is very special as patches only apply within ~half a day. Applied later they'll produce conflicts due to continuous work.
So next time you submit the formatting patch, we'll need to push it immediately to the repo and hope we don't have too many conflicts with our local continuous development branches.

Thank you for your contribution!

Here is different patches.

Thank you! Committed to cvs.

For further inputs please open new issues.

Basic functions work now on Drupal 7. Please open new issues for further requests/reports.