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

Comments

erikwebb’s picture

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

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

erikwebb’s picture

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 commented
StatusFileSize
new 985360-new-user-watchdog-punctuation-4.patch1.16 KB

Failed to run other modules tests.

erikwebb’s picture

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 commented
Status: Needs work » Needs review
StatusFileSize
new watchdog_extra_punctuation-985360-7.patch1.67 KB

Patches what is described above - removing extra puntuation.

superspring’s picture

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

Patches change above for D8.

lyricnz’s picture

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 commented
Status: Needs work » Needs review
StatusFileSize
new 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 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 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 commented
Status: Needs work » Needs review
StatusFileSize
new 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 commented
Status: Needs work » Needs review
StatusFileSize
new 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 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 commented
Status: Needs work » Needs review
StatusFileSize
new watchdog_extra_punctuation-tidyup-985360-19.patch2.61 KB
new 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 commented

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

chx’s picture

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
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.