I needed to send a notification on comment replies to everyone but the user who made the reply, so I needed a little more than what nodejs_actions provided, therefor I added a rules action to send a notification.

I added the rules code inside the nodejs_notify project, although I didn't know if perhaps it should go in the main nodejs project being that that is where nodejs_broadcast_message() lives.

Also, I added a parameter to nodejs_broadcast_message() to take in an array of uids to ignore when broadcasting the message.

Thanks for all the hard work, this project is awesome!

CommentFileSizeAuthor
#9 nodejs_notify_rules.patch1.07 KBinolen
#6 client.socket.id_.patch3.26 KBAnonymous (not verified)
#5 nodejs_rules_action_and_ajax_sessionids.patch4.08 KBinolen
nodejs_notify_rules_support.patch2.6 KBinolen
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

inolen’s picture

inolen CreditAttribution: inolen commented

Actually, ignoring a UID for the purpose of this action isn't exactly what I need to.

It would be more proper, if possible, to ignore the session id responsible for triggering the broadcast.

inolen’s picture

inolen CreditAttribution: inolen commented
Status: Needs review » Needs work
Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

i think being able to broadcast to everyone except the broadcaster is a feature i'd be ok with in the core nodejs module.

inolen’s picture

inolen CreditAttribution: inolen commented

Is it possible to ignore just the clientId/sessionId responsible for sending it though (I don't know what is the proper term, it seems sessionId and clientId are used for different/the same meaning some times).

What I mean is, if we have two browser instances open, signed into the same account, is it possible to know which one is responsible for the call and exclude only it, or are we limited to excluding based on uid/authToken?

Edit: Driving to work and I realized some context may help. I think typically when posting a comment which sends a broadcast, the broadcaster may not receive the notification due to it being sent as the page reloads. However, I'm posting the comment asynchronously and then updating comments on clients that are subscribed to the page's unique channel. I'm not by the code now, but perhaps I can hook into the AJAX request and pass in the current sessionId?

inolen’s picture

inolen CreditAttribution: inolen commented
Status: Needs work » Needs review
FileSize
nodejs_rules_action_and_ajax_sessionids.patch4.08 KB

I'm still not sure if this is the best way, unfortunately this is my first time working with your project.

To preface, I don't think ignoring the browser window responsible for triggering the message matters in non-AJAX environments. In those instances, the user's browser is going to refresh and the message probably sent to everyone before he reconnects to the server.

However, for AJAX environments it's helpful to be able to exclude the window that triggered the message from receiving the resulting messages. I.E. when a user posts a comment asynchronously, we don't need to then send a notification saying "you posted a comment," we just want to let others know.

Anyways, to do this, I monkey patched Drupal.ajax.prototype.beforeSerialize to pass in the current socket id into the request data. This wasn't my favorite solution, but it didn't seem anything else would work.
- We can't set it as a cookie, because all tabs share the same cookies and therefor overwrite each other
- We can't use persistantStorage as that isn't sent to the server on a subsequent page request
- We can only universally hook and modify HTTP headers in Firefox, Chrome may have support but IE definitely does not. I was really hoping we could do this and set a X-Nodejs-SessionId flag.

Moving on, I modified nodejs_broadcast_message() to take in a boolean telling it to ignore the active session id or not.

Finally, I made some modifications in server.js to respect the new exclude_sessionid flag.

Also, moved the new .rules.inc into the main project, it didn't make sense being in the nodejs_notify project.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
FileSize
client.socket.id_.patch3.26 KB

here's the current state of my work on this, let me know if this will work for your use case.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Status: Needs review » Active

ok, committed the plumbing related to the session id and ajax, and making the module and server.js code use it.

so, what's left to do here?

inolen’s picture

inolen CreditAttribution: inolen commented

Everything worked great :)

I'll post a small patch with the actual rule action for nodejs_notify tonight.

inolen’s picture

inolen CreditAttribution: inolen commented
FileSize
nodejs_notify_rules.patch1.07 KB

And the final patch.

tizzo’s picture

tizzo CreditAttribution: tizzo commented
Status: Active » Needs work

Somethign is blowing up for me in Chrome but not Safari and console is throwing the following error.

Uncaught TypeError: Cannot read property 'prototype' of undefined nodejs.js:79

changing:

    Drupal.Nodejs.originalBeforeSerialize = Drupal.ajax.prototype.beforeSerialize;
    Drupal.ajax.prototype.beforeSerialize = function(element_settings, options) {
      options.data['nodejs_client_socket_id'] = Drupal.Nodejs.socket.socket.sessionid;
      return Drupal.Nodejs.originalBeforeSerialize(element_settings, options);
    };  
  });

to:

    if (Drupal.ajax != undefined) {
      Drupal.Nodejs.originalBeforeSerialize = Drupal.ajax.prototype.beforeSerialize;
      Drupal.ajax.prototype.beforeSerialize = function(element_settings, options) {
        options.data['nodejs_client_socket_id'] = Drupal.Nodejs.socket.socket.sessionid;
        return Drupal.Nodejs.originalBeforeSerialize(element_settings, options);
      };
    }

Made the error go away and got my node.js stuff working again but I haven't had time to look into what the deeper cause is.

inolen’s picture

inolen CreditAttribution: inolen commented

Hmm, that's the new monkey patching code that sends the client socket id with AJAX requests.

If Drupal.ajax is undefined we shouldn't need the functionality.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

#10 looks fine to me - tizzo, go ahead and commit.

if you don't get around to it, i'll do it tomorrow morning.

tizzo’s picture

tizzo CreditAttribution: tizzo commented
Status: Needs work » Fixed

true to his word, @beejeebus did, in fact, commit this.

supermouse’s picture

supermouse CreditAttribution: supermouse commented

i have this error need people to help.
Invalid login for uid " 0 "
warn - websocket connect invalid
Example extension got connection event for session
sent message to client
thanks.

supermouse’s picture

supermouse CreditAttribution: supermouse commented

help me install nodejs drupal server with centOS 6.
Thanks

Status: Fixed » Closed (fixed)

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

Remon’s picture

Remon CreditAttribution: Remon commented
Status: Closed (fixed) » Active

Seems that rules integration (supposedly at nodejs_notify/nodejs_notify.rules.inc) is not present any more in module's tarball or even the Git repository. Please advise.
Thanks.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

#17 - that file was never committed.

why do you need it? i'm not opposed to adding more rules integration, i just can't remember all the history here.

i'm open to a nodejs_rules module, or additions to existing modules to add more rules hooks, i just want some better reasoning.

Remon’s picture

Remon CreditAttribution: Remon commented
Status: Active » Closed (fixed)

It is supposed to be bundled as per #13. But yeah, it'b better having a contrib module just for rules integration.
I think there is one already http://drupal.org/sandbox/frega/1273074.

noahadler’s picture

noahadler CreditAttribution: noahadler commented
Status: Closed (fixed) » Needs work

Reopening this issue because I'm new to this module, but not Drupal nor node.js itself, and after spending some time tracking this down, I suggest that this is a design flaw. My situation is I'm using the editablefields module to alter fields via AJAX, and using nodejs_send_message() from hook_node_update to write a changelog globally in a sidebar block. Tracking this down I found:

  // It's possible that this message originated from an ajax request from the
  // client associated with this socket.
  if (message.clientSocketId == Drupal.Nodejs.socket.socket.sessionid) {
    return;
  }

at line 21 of nodejs.js. While it may be sometimes desirable not to broadcast changes back to the user who made it from an AJAX call, other times it might be preferable to do so. Is there a better way to design this which can accomodate both use cases?

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented

#20 is a fair question.

patches and suggestions welcome.

Anonymous’s picture

Anonymous (not verified) CreditAttribution: Anonymous commented
Issue summary: View changes

foobar

  • Commit e849501 on 7.x-1.x, auth-refactor, 8.x-1.x, 8.x-1.x-head by beejeebus: 
    #1280748 by inolen, beejeebus: monkey-patch Drupal.ajax.prototype....
  • Commit 92692db on 7.x-1.x, auth-refactor, 8.x-1.x, 8.x-1.x-head by beejeebus:
    #1280748 by tizzo, beejeebus: check for Drupal.ajax when monkeypatching...