drupal_set_message('Test', 'warning', FALSE);
drupal_set_message('Test', 'warning', FALSE);
drupal_set_message('Test', 'warning', FALSE);

The code above displays 'Test' three times.

CommentFileSizeAuthor
#12 drupal_set_message-2371671-12.patch2.32 KBpodarok
#5 tests_only_drupal_set_message-2371671-5.patch1.53 KBpodarok
#4 drupal_set_message-2371671-4.patch2.4 KBpodarok
#2 drupal_set_message-2371671-2.patch889 bytespodarok

Comments

dawehner’s picture

dawehner
Primary language German
commented
Issue tags: +Needs tests

Just tagging it.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Assigned: Unassigned » podarok
Status: Active » Needs review
StatusFileSize
new drupal_set_message-2371671-2.patch889 bytes

Yup
Here is a patch.
Figuring out where test for this function should be located.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Component: other » system.module
podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
StatusFileSize
new drupal_set_message-2371671-4.patch2.4 KB

added tests

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
StatusFileSize
new tests_only_drupal_set_message-2371671-5.patch1.53 KB

Uploading tests only . Should fail.

Status: Needs review » Needs work

The last submitted patch, 5: tests_only_drupal_set_message-2371671-5.patch, failed testing.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review

nice
#4 contains fix and tests. Needs review

m1r1k’s picture

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

#4 looks good

andypost’s picture

andypost
he/him
Primary language Russian
commented

I'd prefer to see here a DrupalSetMessageTest converted to KernelTestBase

+++ b/core/modules/system/tests/modules/system_test/src/Controller/SystemTestController.php
@@ -65,6 +65,13 @@ public function drupalSetMessageTest() {
+    drupal_set_message('Non Duplicated message', 'status', FALSE);
+    drupal_set_message('Non Duplicated message', 'status', FALSE);
...
+    drupal_set_message('Duplicated message', 'status', TRUE);
+    drupal_set_message('Duplicated message', 'status', TRUE);

integration tests already all over core within UI (form) tests

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented 
+++ b/core/includes/bootstrap.inc
@@ -703,12 +703,14 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE)
+    unset($new);

I don't see how this unset is necessary. The important change is from $message to $new in the !in_array check.

alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Needs work 
+++ b/core/includes/bootstrap.inc
@@ -703,12 +703,14 @@ function drupal_set_message($message = NULL, $type = 'status', $repeat = FALSE)
+    unset($new);

Yep this is unnecessary

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Status: Needs work » Needs review
StatusFileSize
new drupal_set_message-2371671-12.patch2.32 KB

I'd prefer to see here a DrupalSetMessageTest converted to KernelTestBase

Let's not mix bug fixing with code refactoring. If it is necessary - do that in follow-up

Here is a new patch.
Removed unset.

podarok’s picture

podarok
🇺🇦 he/him/his
Primary language Ukrainian
Location Ukraine
commented
Issue tags: -Needs tests

removed tag

andypost’s picture

andypost
he/him
Primary language Russian
commented
Status: Needs review » Reviewed & tested by the community
alexpott’s picture

alexpott
he/they
Primary language English
Location 🇪🇺🌍
commented
Status: Reviewed & tested by the community » Fixed

This issue is a normal bug fix, and doesn't include any disruptive changes, so it is allowed per https://www.drupal.org/core/beta-changes. Committed 7473eaf and pushed to 8.0.x. Thanks!

  • alexpott committed 7473eaf on 8.0.x 
    Issue #2371671 by podarok | Chi: Fixed drupal_set_message repeat...

Status: Fixed » Closed (fixed)

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