From 142099e2a647603eb3b17677a08337262606432b Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 5 Nov 2015 14:13:47 +0000 Subject: [PATCH 1/3] MDL-47065 notes: don't add blank notes Previously the logic was wrong and was proceeding to add a note when content was missing. Also add behat coverage for adding notes to participants (this test is not perfect, but better than zero coverage we had before). --- notes/tests/behat/participants_notes.feature | 41 ++++++++++++++++++++ user/addnote.php | 3 +- 2 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 notes/tests/behat/participants_notes.feature diff --git a/notes/tests/behat/participants_notes.feature b/notes/tests/behat/participants_notes.feature new file mode 100644 index 00000000000..50e35d9e433 --- /dev/null +++ b/notes/tests/behat/participants_notes.feature @@ -0,0 +1,41 @@ +@core @core_note +Feature: Add notes to course participants + In order to share information with other staff + As a teacher + I need to add notes from the course particpants list + + Scenario: An teacher can add multiple notes + Given the following "users" exist: + | username | firstname | lastname | email | + | teacher1 | Teacher | 1 | teacher1@example.com | + | student1 | Student | 1 | student1@example.com | + | student2 | Student | 2 | student2@example.com | + And the following "courses" exist: + | fullname | shortname | format | + | Course 1 | C1 | topics | + And the following "course enrolments" exist: + | user | course | role | + | teacher1 | C1 | editingteacher | + | student1 | C1 | student | + | student2 | C1 | student | + And I log in as "teacher1" + And I follow "Course 1" + And I follow "Participants" + And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 1')]//input[@type='checkbox']" to "1" + And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 2')]//input[@type='checkbox']" to "1" + And I set the field "With selected users..." to "Add a new note" + And I press "OK" + # Add a note to student 1, but leave student 2 empty. + When I set the field with xpath "//tr[contains(normalize-space(.), 'Student 1')]//textarea" to "Student 1 needs to pick up his game" + And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 2')]//textarea" to "" + And I press "Save changes" + And I follow "Student 1" + And I follow "Notes" + # Student 1 has note from Teacher + Then I should see "Teacher" in the "region-main" "region" + And I should see "Student 1 needs to pick up his game" + And I follow "Participants" + And I follow "Student 2" + And I follow "Notes" + # Terrible way to verify the absence of a note.. + And I should not see "Teacher" in the "region-main" "region" diff --git a/user/addnote.php b/user/addnote.php index 540423a9a88..8155496c2c5 100644 --- a/user/addnote.php +++ b/user/addnote.php @@ -54,7 +54,8 @@ if (!empty($users) && confirm_sesskey()) { $note->courseid = $id; $note->format = FORMAT_PLAIN; foreach ($users as $k => $v) { - if (!$user = $DB->get_record('user', array('id' => $v)) || empty($contents[$k])) { + $user = $DB->get_record('user', array('id' => $v)); + if (!$user || empty($contents[$k])) { continue; } $note->id = 0; From 2a4c2367045d700de0b83537c844af40356a4009 Mon Sep 17 00:00:00 2001 From: Jun Pataleta Date: Thu, 12 Nov 2015 09:43:26 +0000 Subject: [PATCH 2/3] MDL-47065 behat: improve verificaton of lack of note --- notes/tests/behat/participants_notes.feature | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/notes/tests/behat/participants_notes.feature b/notes/tests/behat/participants_notes.feature index 50e35d9e433..d0e82d119cf 100644 --- a/notes/tests/behat/participants_notes.feature +++ b/notes/tests/behat/participants_notes.feature @@ -37,5 +37,10 @@ Feature: Add notes to course participants And I follow "Participants" And I follow "Student 2" And I follow "Notes" - # Terrible way to verify the absence of a note.. - And I should not see "Teacher" in the "region-main" "region" + And I follow "Course 1" + And I follow "Participants" + And I follow "Notes" + Then I should see "Student 1" + And I should see "Student 1 needs to pick up his game" + # Verify Student 2 does not have a note added. + And I should not see "Student 2" From 00d70d79a4f73de8e4f0fae482be193aed80c68f Mon Sep 17 00:00:00 2001 From: Dan Poltawski Date: Thu, 12 Nov 2015 09:54:39 +0000 Subject: [PATCH 3/3] MDL-47065 notes: prevent empty spaces added as note (And updated behat test for this test case) --- notes/tests/behat/participants_notes.feature | 7 ++++++- user/addnote.php | 5 +++-- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/notes/tests/behat/participants_notes.feature b/notes/tests/behat/participants_notes.feature index d0e82d119cf..4f3dccba3cb 100644 --- a/notes/tests/behat/participants_notes.feature +++ b/notes/tests/behat/participants_notes.feature @@ -10,6 +10,7 @@ Feature: Add notes to course participants | teacher1 | Teacher | 1 | teacher1@example.com | | student1 | Student | 1 | student1@example.com | | student2 | Student | 2 | student2@example.com | + | student3 | Student | 3 | student3@example.com | And the following "courses" exist: | fullname | shortname | format | | Course 1 | C1 | topics | @@ -18,16 +19,19 @@ Feature: Add notes to course participants | teacher1 | C1 | editingteacher | | student1 | C1 | student | | student2 | C1 | student | + | student3 | C1 | student | And I log in as "teacher1" And I follow "Course 1" And I follow "Participants" And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 1')]//input[@type='checkbox']" to "1" And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 2')]//input[@type='checkbox']" to "1" + And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 3')]//input[@type='checkbox']" to "1" And I set the field "With selected users..." to "Add a new note" And I press "OK" - # Add a note to student 1, but leave student 2 empty. + # Add a note to student 1, but leave student 2 empty and student 3 with space. When I set the field with xpath "//tr[contains(normalize-space(.), 'Student 1')]//textarea" to "Student 1 needs to pick up his game" And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 2')]//textarea" to "" + And I set the field with xpath "//tr[contains(normalize-space(.), 'Student 3')]//textarea" to " " And I press "Save changes" And I follow "Student 1" And I follow "Notes" @@ -44,3 +48,4 @@ Feature: Add notes to course participants And I should see "Student 1 needs to pick up his game" # Verify Student 2 does not have a note added. And I should not see "Student 2" + And I should not see "Student 3" diff --git a/user/addnote.php b/user/addnote.php index 8155496c2c5..4072718bb2f 100644 --- a/user/addnote.php +++ b/user/addnote.php @@ -55,11 +55,12 @@ if (!empty($users) && confirm_sesskey()) { $note->format = FORMAT_PLAIN; foreach ($users as $k => $v) { $user = $DB->get_record('user', array('id' => $v)); - if (!$user || empty($contents[$k])) { + $content = trim($contents[$k]); + if (!$user || empty($content)) { continue; } $note->id = 0; - $note->content = $contents[$k]; + $note->content = $content; $note->publishstate = $states[$k]; $note->userid = $v; note_save($note);