Update comment notify for Drupal 7

I'm more and more open to the idea that this branch will be totally separate and I'm thinking it would be be better to apply this so that we can track the commit of other issues into this version because, for example, this whole thing doesn't apply any more :/

Any chance you have some magic way of re-rolling it? Or should I just branch 6.x, and take HEAD back to the date you created this on and commit from there and find individual stuff to merge in manually? I'm fine with doing that, just want to make sure it's not unnecessary.

I think this patch will apply cleanly against CVS HEAD if you want to branch and then apply it. Note that there is 1 new file to be added to CVS.

it includes one more minor fix in function comment_notify_user_update() to avoid notices.

Awesome, thanks pwolanin and Jacob for this great work on the module.

• comment_notify_set_user_notification_setting and comment_notify_get_user_notification_setting need to say in the doxygen that they @throw exceptions
• on a brand new site after creating a page and not having a preference set I get Notice: Trying to get property of non-object in comment_notify_get_user_comment_notify_preference() (line 58 of /var/www/7d/sites/all/modules/comment_notify/comment_notify.lib.inc).
• Posting a comment we get # Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load() (line 109 of /var/www/7d/includes/entity.inc).
  # Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->cacheGet() (line 272 of /var/www/7d/includes/entity.inc).
• Can we better translate $this->fail('Unable to load role: ' . $role_name); ?
• The tests don't show up for me. I tried to merge in the changes to the tests that you made with the additional tests I've written in the last few days, so maybe I merged them incorrectly? I attached my

@greggles - can you commit now? that would make follow-up patches easier.

I did some really basic testing but not full testing. At this point the tests are more complete than manual testing I do, so I'd like to rely on them to be the decider of whether or not to commit this.

I've committed this http://drupal.org/cvs?commit=290756 - now time for final fixups. Thanks, Jacob!

It turns out the missing thing for the tests was to add the file to comment_notify.info file. The tests no longer pass for me, but I can look into that later. I think it's related to my local environment more than anything.

I think this is missing the comment_notify.lib file (also, why not comment_notify.inc) as I'm getting a bunch of undefined function errors on D7. Plus, it's also included in global context instead of only when necessary.

details details. http://drupal.org/cvs?commit=332192 ;)

So, I'm reviewing and fixing up a handful of things today and noticed that comment_notify_variable_registry_set doesn't seem to actually get called anywhere. Is that right? Is there a reason for that?

Ok, I just committed an update: http://drupal.org/cvs?commit=350198

1. The tests pass again. Yay. Though there are still lots of exceptions about flipping elements. #761878: Warning: array_flip(): Can only flip STRING and INTEGER values! in DrupalDefaultEntityController->load()
2. I moved comment_notify.lib.inc to just comment_notify.inc and I am not now loading it using the more robust module_load_include('inc', 'comment_notify', 'comment_notify'); and only doing so in places where it's necessary.
3. The validate function wasn't actually getting called since the hook_comment_validate doesn't exist (anymore?)
4. Some array elements/object properties got re-shuffled and needed to be updated
5. Updated the url for modifying node settings

I think this is pretty close to done so I'm marking this "fixed".

Though I'd still love to know why comment_notify_variable_registry_set was added and if there is anything that actually uses it.

Edited to add reference to the array flip issue and change a not to now since now is what I meant.