Description

The Simplenews module lacks a way to dynamically filter lists of subscribers for newsletters. This module hopes to remedy that by using Views as filters.

Simplenews filter allows you to select any view of type simplenews_subscriber on which to filter newsletter subscriptions against. The recipients must already be subscribed to the newsletter.

If the selected view has exposed filters, they will be loaded on the newsletter's send page via AJAX and can be modified to alter the view's results. When the newsletter is sent, the view query is modified with details required by the Simplenews mail spool and the results are then inserted into the mail spool.

Similar projects

The Party Simplenews module which allows modifying Simplenews mailing lists by integrating the Party module instead of the Views module.

Link to project

https://drupal.org/sandbox/dieuwe/2179443

Links to git repository

git clone --branch 7.x-2.x http://git.drupal.org/sandbox/dieuwe/2179443.git simplenews_filter
Version compatible with Simplenews 7.x-2.x. This will be the primary version, as that branch of Simplenews allows for integration of what this module does.
git clone --branch 7.x-1.x http://git.drupal.org/sandbox/dieuwe/2179443.git simplenews_filter
Version compatible with Simplenews 7.x-1.x. This branch is a little bit hacky, as the branch of Simplenews it attempts to be compatible with does not allow for integration of what this module does.

Other project reviews
https://drupal.org/comment/8469085#comment-8469085
https://drupal.org/comment/8469667#comment-8469667
https://drupal.org/comment/8469775#comment-8469775

Comments

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Issue summary: View changes

Added link to a project review.

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Issue summary: View changes

Fixed links to git repository.

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Issue summary: View changes

Added another review link.

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Issue summary: View changes
Issue tags: +PAreview: review bonus

Added link to another project review and the review bonus tag.

gobinathm’s picture

gobinathm
he/him
Primary language English
Location Milton, Ontario
commented

PAREview : There is a PHP Error reported on your project. Please fix this.
http://pareview.sh/pareview/httpgitdrupalorgsandboxdieuwe2179443git

gobinathm’s picture

gobinathm
he/him
Primary language English
Location Milton, Ontario
commented
Status: Needs review » Needs work
dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Status: Needs work » Needs review

Forgot to double check that when making changes a few days back. Fixed now.

alinouman’s picture

alinouman commented
Status: Needs review » Needs work
Issue tags: -PAreview: review bonus

Hi dieuwe,
1- You are using form alter, if you are modifying one form you have to use hook_form_FORM_ID_alter.
2- when i go to newletter tab to send i get following fatal error,
Fatal error: Call to undefined function simplenews_issue_newsletter_id() in /var/www/ji/sites/all/modules/simplenews_filter/simplenews_filter.module on line 61
removing review tag you can add it again if you had done 3 more manual reviews of code. Thanks

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Status: Needs work » Needs review

Have changed the hook_form_alter to a hook_form_FORM_ID_alter().

I think the error you are seeing is caused by not having the latest 7.x-2.x version of Simplenews (as I have documented).

I have attempted to require a 2.x branch of Simplenews in my info file, but without much success.

Please take another look :)

stred’s picture

stred commented

Hi dieuwe,

I've reviewed your module, version 7.x-2.x. No errors, really nice coded and documented, works like a charm!

The only small suggestion I could do is why explicitly exclude default/master display in simplenews_filter_get_views_as_options() ? if this is by design, you should mention it on the README file. I didn't create any additional display and thought the module was broken...

This is not a blocking issue so I don't change the status... great job!

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented
Status: Needs review » Reviewed & tested by the community

Hope you don't mind me setting this to RTBC then.

I have followed your suggestion though, as I realise it makes more sense.
The reason I initially excluded the "master" display is because views normally hides it and it can often be redundant for view with normal displays. In this case though, users may want to just use a master display if there is no reason to have a page or block (e.g. they just want to use that view as a filter and nothing else).

I will consider adding an option to toggle "master" display on or off later, as for some this might just clog up the list with redundant options.

Thanks for your feedback :)

stborchert’s picture

stborchert
he/him
Primary language German
commented
Status: Reviewed & tested by the community » Needs review

Please do not set your own application to "Reviewed and tested by the community". This is done by other reviewers if there are no release-critical objections.
Have you thought about providing this feature as a patch for https://drupal.org/project/simplenews?

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented

This is intended to plugin to an API feature, for the 2.x branch anyway.

https://www.drupal.org/node/2179459

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs review » Fixed

Sorry for the delay. Reviewing more project applications and a new review bonus would have get this finished faster.

Review of the 7.x-2.x branch (commit 8cceefc):

  • Coder Sniffer has found some issues with your code (please check the Drupal coding standards).
    FILE: /home/klausi/pareview_temp/simplenews_filter.module
--------------------------------------------------------------------------------
FOUND 10 ERRORS AFFECTING 7 LINES
--------------------------------------------------------------------------------
   9 | ERROR | Doc comment short description must start with a capital letter
   9 | ERROR | Doc comment short description must end with a full stop
  11 | ERROR | Doc comment short description must be on a single line, further
     |       | text should be a separate paragraph
  78 | ERROR | Doc comment short description must start with a capital letter
  78 | ERROR | Doc comment short description must end with a full stop
  80 | ERROR | Doc comment short description must be on a single line, further
     |       | text should be a separate paragraph
 210 | ERROR | Doc comment short description must start with a capital letter
 210 | ERROR | Doc comment short description must end with a full stop
 212 | ERROR | Doc comment short description must be on a single line, further
     |       | text should be a separate paragraph
 261 | ERROR | Type hint "array" missing for $filters
--------------------------------------------------------------------------------
  • DrupalPractice has found some issues with your code, but could be false positives.
    
FILE: /home/klausi/pareview_temp/simplenews_filter.install
--------------------------------------------------------------------------------
FOUND 0 ERRORS AND 1 WARNING AFFECTING 1 LINE
--------------------------------------------------------------------------------
 12 | WARNING | Do not use DELETE queries with db_query(), use db_delete()
    |         | instead
--------------------------------------------------------------------------------
  • No automated test cases were found, did you consider writing Simpletests or PHPUnit tests? This is not a requirement but encouraged for professional software development.

This automated report was generated with PAReview.sh, your friendly project application review script. You can also use the online version to check your project. You have to get a review bonus to get a review from me.

manual review:

  1. So it looks like this specific use case is worth a separate module.
  2. "'#markup' => 'The recipient count needs to be refreshed.',": all user facing text must run through t() for translation.

Otherwise looks good to me. Since this has been reviewed already before and no problems were found it looks like the RTBC was justified.

Thanks for your contribution, dieuwe!

I updated your account so you can promote this to a full project and also create new projects as either a sandbox or a "full" project.

Here are some recommended readings to help with excellent maintainership:

You can find lots more contributors chatting on IRC in #drupal-contribute. So, come hang out and stay involved!

Thanks, also, for your patience with the review process. Anyone is welcome to participate in the review process. Please consider reviewing other projects that are pending review. I encourage you to learn more about that process and join the group of reviewers.

Thanks to the dedicated reviewer(s) as well.

dieuwe’s picture

dieuwe
Location Auckland, NZ
commented

Thank you! :) I'll address the issues you raised and publish my first module.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.