It comes up many times in projects that the user profile form elements need shifted around according to design specifications. pm_email_notify.module defines an option for the user to choose if they want an email notification or not when they receive a PM. However, the way that it is included currently does not make it easy for a site builder to move this around as desired.

For instance, if I create a new profile category, and alter the 'account' category to not show this form field, then alter the new profile category to show this form field, the submit case within pm_email_notify's hook_user() implementation is still executed when I hit submit from the 'account' category. And since the field doesn't exist, it ends up setting turning off the option.

A simple resolution to this is to just do a simple check to make sure that this field even exists within that submit case.

CommentFileSizeAuthor
#5 check_actual_fields_not_category.patch1.74 KBberdir
#1 pm_email_notify-user_submit_check.patch1.89 KBsirkitree
pm_email_notify-user_submit_check.patch2.79 KBsirkitree

Comments

sirkitree’s picture

sirkitree commented
StatusFileSize
new pm_email_notify-user_submit_check.patch1.89 KB

Sorry, had a little cruft at the top of that patch.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Needs work

Can't you just extend the existing check? (&& isset($edit['pm_send_notifications'])) Then you only need to change a single line.

Because your patch would still trigger an E_NOTICE as it does always access $edit['pm_send_notifications'] even if it does not exist.

I recently opened the same issue but there is no patch yet, so I'll close that one as a duplicate.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented

Actually, if is moved to a *different* category, then we can replace the $category == 'account' check with what I suggested above.

sirkitree’s picture

sirkitree commented
Status: Needs work » Needs review

Totally makes sense.

Index: pm_email_notify/pm_email_notify.module
===================================================================
RCS file: /cvs/drupal-contrib/contributions/modules/privatemsg/pm_email_notify/pm_email_notify.module,v
retrieving revision 1.1.2.15
diff -u -p -r1.1.2.15 pm_email_notify.module
--- pm_email_notify/pm_email_notify.module	28 Apr 2010 20:19:52 -0000	1.1.2.15
+++ pm_email_notify/pm_email_notify.module	30 Jul 2010 12:41:13 -0000
@@ -169,7 +169,7 @@ function pm_email_notify_user($op, &$edi
       return $form;
 
      case 'submit':
-      if ($category == 'account' && privatemsg_user_access('read privatemsg')) {
+      if (isset($edit['pm_send_notifications']) && privatemsg_user_access('read privatemsg')) {
         $pm_email_enabled = $edit['pm_send_notifications'];
         unset($edit['pm_send_notifications']);
         // Update database entry with user preference.

I'm not working off of 2.x-dev locally (1.2 actually), so i don't expect the automated testing to apply - but it should apply with an offset as it doesn't look like this has changed.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
StatusFileSize
new check_actual_fields_not_category.patch1.74 KB

Attaching a patch for 2.x that also removes the category check for the (2.x only feature) enable privatemsg setting.

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Needs review » Fixed

Commited to all branches, thanks for reporting.

Status: Fixed » Closed (fixed)

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