Different user actions create different punctuation in watchdog messages.

User block/delete use arrows-

      watchdog('user', 'Blocked user: %name %email.', array('%name' => $account->name, '%email' => '<' . $account->mail . '>'), WATCHDOG_NOTICE);
      watchdog('user', 'Deleted user: %name %email.', array('%name' => $account->name, '%email' => '<' . $account->mail . '>'), WATCHDOG_NOTICE);

User creation uses parentheses -

  watchdog('user', 'New user: %name %email.', array('%name' => $form_state['values']['name'], '%email' => '<' . $form_state['values']['mail'] . '>'), WATCHDOG_NOTICE, l(t('edit'), 'user/' . $account->uid . '/edit'));
CommentFileSizeAuthor
#20 watchdog_extra_punctuation-985360-18.patch1.92 KBsuperspring
#20 watchdog_extra_punctuation-tidyup-985360-19.patch2.61 KBsuperspring
#17 watchdog_extra_punctuation-985360-17.patch4.59 KBsuperspring
#15 watchdog_extra_punctuation-985360-15.patch1007 bytessuperspring
#10 watchdog_extra_punctuation-985360-10.patch1.92 KBsuperspring
#8 watchdog_extra_punctuation-985360-8.patch1.89 KBsuperspring
#7 watchdog_extra_punctuation-985360-7.patch1.67 KBsuperspring
#4 985360-new-user-watchdog-punctuation-4.patch1.16 KBerikwebb
#1 985360-new-user-watchdog-punctuation.patch651 byteserikwebb
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented
Status: Active » Needs review
FileSize
985360-new-user-watchdog-punctuation.patch651 bytes

Attached patch converts "New user" message to arrows for consistency with the majority.

erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented

User creation snippet in OP was post-patch. Original -

    watchdog('user', 'New user: %name (%email).', array('%name' => $form_state['values']['name'], '%email' => $form_state['values']['mail']), WATCHDOG_NOTICE, l(t('edit'), 'user/' . $account->uid . '/edit'));

Status: Needs review » Needs work

The last submitted patch, 985360-new-user-watchdog-punctuation.patch, failed testing.

erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented
FileSize
985360-new-user-watchdog-punctuation-4.patch1.16 KB

Failed to run other modules tests.

erikwebb’s picture

erikwebb CreditAttribution: erikwebb commented
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 985360-new-user-watchdog-punctuation-4.patch, failed testing.

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review
FileSize
watchdog_extra_punctuation-985360-7.patch1.67 KB

Patches what is described above - removing extra puntuation.

superspring’s picture

superspring CreditAttribution: superspring commented
Version: 7.x-dev » 8.x-dev
FileSize
watchdog_extra_punctuation-985360-8.patch1.89 KB

Patches change above for D8.

lyricnz’s picture

lyricnz CreditAttribution: lyricnz commented
Status: Needs review » Needs work

I like having the visual markers around the email address, so while I agree that we should be consistent, I would prefer that we put them everywhere: AccountFormController.php has three without <>, user.module has two with <> and user.pages has one of each :)

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review
FileSize
watchdog_extra_punctuation-985360-10.patch1.92 KB

I have searched the code base for email addresses that append a name. These instances now all have %name <%email> markup. %email by itself has been left as is.

lyricnz’s picture

lyricnz CreditAttribution: lyricnz commented
Status: Needs review » Reviewed & tested by the community

Looks OK to me. Consistency is good.

Status: Reviewed & tested by the community » Needs work

The last submitted patch, watchdog_extra_punctuation-985360-10.patch, failed testing.

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review

#10: watchdog_extra_punctuation-985360-10.patch queued for re-testing.

Status: Needs review » Needs work

The last submitted patch, watchdog_extra_punctuation-985360-10.patch, failed testing.

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review
FileSize
watchdog_extra_punctuation-985360-15.patch1007 bytes

Removing the fix inside the tests. This seems to work now, not too sure why though...

Status: Needs review » Needs work

The last submitted patch, watchdog_extra_punctuation-985360-15.patch, failed testing.

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review
FileSize
watchdog_extra_punctuation-985360-17.patch4.59 KB

Attempt four

Status: Needs review » Needs work

The last submitted patch, watchdog_extra_punctuation-985360-17.patch, failed testing.

lyricnz’s picture

lyricnz CreditAttribution: lyricnz commented 
+++ b/core/modules/user/lib/Drupal/user/RegisterFormController.phpundefined
@@ -1,5 +1,4 @@
-

Unnecessary change

+++ b/core/modules/user/lib/Drupal/user/RegisterFormController.phpundefined
@@ -102,7 +101,7 @@ public function save(array $form, array &$form_state) {
-    if ($status =! SAVED_NEW) {
+    if ($account->status != SAVED_NEW) {

How is this related to the change in watchdog message?

+++ b/core/modules/user/lib/Drupal/user/RegisterFormController.phpundefined
@@ -118,7 +117,10 @@ public function save(array $form, array &$form_state) {
-      drupal_set_message(t('Created a new user account for <a href="@url">%name</a>. No e-mail has been sent.', array('@url' => url($uri['path'], $uri['options']), '%name' => $account->name)));
+      drupal_set_message(t('Created a new user account for <a href="@url">%name</a>. No e-mail has been sent.', array(
+          '@url' => url($uri['path'], $uri['options']),
+          '%name' => $account->name,
+      )));

Unnecessary cleanup. Just fix the issue, make another patch for cleanup later. Ditto for the other three below.

superspring’s picture

superspring CreditAttribution: superspring commented
Status: Needs work » Needs review
FileSize
watchdog_extra_punctuation-tidyup-985360-19.patch2.61 KB
watchdog_extra_punctuation-985360-18.patch1.92 KB

Hey @lyricnz,

I've split this into three patches which addresses your issue above. Here I will upload two for 'code tidyup' and one for the 'bug fix'.

lyricnz’s picture

lyricnz CreditAttribution: lyricnz commented

The first patch looks good (RTBC). The second, not sure I'd bother with.

chx’s picture

chx CreditAttribution: chx commented
Status: Needs review » Reviewed & tested by the community

The first one is good to go. As for AccountFormController it only contains an email address w/o a name so no need for arrows or parenthesis.

catch’s picture

catch
he/him
Primary language English
CreditAttribution: catch commented
Status: Reviewed & tested by the community » Fixed

Not really into argument wrapping at all. Committed/pushed the first patch from #20, thanks!

Status: Fixed » Closed (fixed)

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