I've extended the nodejs_notify module that came with the node.js integration module. I was having fun doing the drush command to get broadcast messages to show up, but noticed there was no way for a non-drush drupal admin to do this, so I built an interface for an admin to do a system-wide broadcast message to all users. I also built in a form with one config setting so far: notification lifetime, with the idea that more config settings will be added in the future. Enjoy!

CommentFileSizeAuthor
#10 nodejs_notify_admin_extend-1138250-4394348.patch5.29 KBLSU_JBob
#8 nodejs_notify_admin_extend-1138250-4393848.patch5.33 KBLSU_JBob
#6 nodejs_notify_admin_extend-1138250-4392432.patch5.15 KBLSU_JBob
#3 nodejs_notify_admin_extend-1138250.patch4.9 KBLSU_JBob
nodejs_notify_admin_extend.patch5.83 KBLSU_JBob

Comments

LSU_JBob’s picture

LSU_JBob commented

Oh, I also added an install file to my local copy. Didn't know how to show it in my patch so here is the new install file I created:

nodejs_notify.install

<?php

/**
 * @file
 * Install, update and uninstall functions for the nodejs notify module.
 */

/**
 * Implements hook_install().
 */
function nodejs_notify_install() {
  variable_set('nodejs_notify_notification_lifetime_seconds', 3);
}

/**
 * Implements hook_uninstall().
 */
function nodejs_notify_uninstall() {
  variable_del('nodejs_notify_notification_lifetime_seconds');
}
Anonymous’s picture

Anonymous (not verified) commented

subscribe, review soon.

LSU_JBob’s picture

LSU_JBob commented
StatusFileSize
new nodejs_notify_admin_extend-1138250.patch4.9 KB

Here is the updated patch. Javascript validation is replaced with preg_match. Unneeded validation functions removed. Updated spacing.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Needs work

cool, nearly there.

+      'access arguments' => array('administer site configuration'),

can we change these two lines? the one for 'Node.js Notification Settings' to 'administer nodejs configuration', and the other one to 'send nodejs broadcast' ?

this will require a hook_permission() implementation...

+  $form['#validate'][] = 'nodejs_notify_settings_form_validate';

that validate bit is redundant, because you've used the standard naming, and Form API will pick it up automagically.

+  drupal_set_message("Message broadcast to all users");

please wrap the message in t(). there's another one like that.

LSU_JBob’s picture

LSU_JBob commented

on it

LSU_JBob’s picture

LSU_JBob commented
Status: Needs work » Needs review
StatusFileSize
new nodejs_notify_admin_extend-1138250-4392432.patch5.15 KB

I've addressed your notes in #4

LSU_JBob’s picture

LSU_JBob commented

I'll add the nodejs_notify config permission you mentioned to me.

LSU_JBob’s picture

LSU_JBob commented
StatusFileSize
new nodejs_notify_admin_extend-1138250-4393848.patch5.33 KB

Patch!

LSU_JBob’s picture

LSU_JBob commented

Ah, I forgot to remove the redundant '#executes_submit_callback' => TRUE, removing that now

LSU_JBob’s picture

LSU_JBob commented
StatusFileSize
new nodejs_notify_admin_extend-1138250-4394348.patch5.29 KB

hopefully last one :)

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs review » Reviewed & tested by the community

awesome, RTBC.

LSU_JBob’s picture

LSU_JBob commented
Status: Reviewed & tested by the community » Fixed

http://drupalcode.org/project/nodejs.git/commit/38a7095

Status: Fixed » Closed (fixed)

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

  • Commit 38a7095 on 6.x-1.x, 7.x-1.x, drupalconlondon, auth-refactor, 8.x-1.x, 8.x-1.x-head by LSU_JBob: 
    Issue #1138250 by LSU_JBob: Extend Node.js Notifications: Config...