MDL-41945 mod_assign: Properly check if submission is empty

Previous empty submission checks required the submission to
be saved to the database. This patch adds a new method to
submission plugins that lets them report whether the submission
is empty before it is saved.

mod/assign/locallib.php

@@ -6371,6 +6371,22 @@ class assign {
         return $allempty;
     }
 
+    /**
+     * Determine if a new submission is empty or not
+     *
+     * @param stdClass $data Submission data
+     * @return bool
+     */
+    public function new_submission_empty($data) {
+        foreach ($this->submissionplugins as $plugin) {
+            if ($plugin->is_enabled() && $plugin->is_visible() && $plugin->allow_submissions() &&
+                    !$plugin->submission_is_empty($data)) {
+                return false;
+            }
+        }
+        return true;
+    }
+
     /**
      * Save assignment submission for the current user.
      *

mod/assign/submission/file/locallib.php

@@ -479,6 +479,20 @@ class assign_submission_file extends assign_submission_plugin {
         return $this->count_files($submission->id, ASSIGNSUBMISSION_FILE_FILEAREA) == 0;
     }
 
+    /**
+     * Determine if a submission is empty
+     *
+     * This is distinct from is_empty in that it is intended to be used to
+     * determine if a submission made before saving is empty.
+     *
+     * @param stdClass $data The submission data
+     * @return bool
+     */
+    public function submission_is_empty(stdClass $data) {
+        $files = file_get_drafarea_files($data->files_filemanager);
+        return count($files->list) == 0;
+    }
+
     /**
      * Get file areas returns a list of areas this plugin stores files
      * @return array - An array of fileareas (keys) and descriptions (values)

mod/assign/submission/file/tests/locallib_test.php

@@ -0,0 +1,141 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for mod/assign/submission/file/locallib.php
+ *
+ * @package   assignsubmission_file
+ * @copyright 2016 Cameron Ball
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/mod/assign/tests/base_test.php');
+
+/**
+ * Unit tests for mod/assign/submission/file/locallib.php
+ *
+ * @copyright  2016 Cameron Ball
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class assignsubmission_file_locallib_testcase extends advanced_testcase {
+
+    /** @var stdClass $user A user to submit an assignment. */
+    protected $user;
+
+    /** @var stdClass $course New course created to hold the assignment activity. */
+    protected $course;
+
+    /** @var stdClass $cm A context module object. */
+    protected $cm;
+
+    /** @var stdClass $context Context of the assignment activity. */
+    protected $context;
+
+    /** @var stdClass $assign The assignment object. */
+    protected $assign;
+
+    /**
+     * Setup all the various parts of an assignment activity including creating an onlinetext submission.
+     */
+    protected function setUp() {
+        $this->user = $this->getDataGenerator()->create_user();
+        $this->course = $this->getDataGenerator()->create_course();
+        $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign');
+        $params = [
+            'course' => $this->course->id,
+            'assignsubmission_file_enabled' => 1,
+            'assignsubmission_file_maxfiles' => 12,
+            'assignsubmission_file_maxsizebytes' => 10,
+        ];
+        $instance = $generator->create_instance($params);
+        $this->cm = get_coursemodule_from_instance('assign', $instance->id);
+        $this->context = context_module::instance($this->cm->id);
+        $this->assign = new testable_assign($this->context, $this->cm, $this->course);
+        $this->setUser($this->user->id);
+    }
+
+    /**
+     * Test submission_is_empty
+     *
+     * @dataProvider submission_is_empty_testcases
+     * @param string $data The file submission data
+     * @param bool $expected The expected return value
+     */
+    public function test_submission_is_empty($data, $expected) {
+        $this->resetAfterTest();
+
+        $itemid = file_get_unused_draft_itemid();
+        $submission = (object)['files_filemanager' => $itemid];
+        $plugin = $this->assign->get_submission_plugin_by_type('file');
+
+        if ($data) {
+            $data += ['contextid' => context_user::instance($this->user->id)->id, 'itemid' => $itemid];
+            $fs = get_file_storage();
+            $fs->create_file_from_string((object)$data, 'Content of ' . $data['filename']);
+        }
+
+        $result = $plugin->submission_is_empty($submission);
+        $this->assertTrue($result === $expected);
+    }
+
+    /**
+     * Test new_submission_empty
+     *
+     * @dataProvider submission_is_empty_testcases
+     * @param string $data The file submission data
+     * @param bool $expected The expected return value
+     */
+    public function test_new_submission_empty($data, $expected) {
+        $this->resetAfterTest();
+
+        $itemid = file_get_unused_draft_itemid();
+        $submission = (object)['files_filemanager' => $itemid];
+
+        if ($data) {
+            $data += ['contextid' => context_user::instance($this->user->id)->id, 'itemid' => $itemid];
+            $fs = get_file_storage();
+            $fs->create_file_from_string((object)$data, 'Content of ' . $data['filename']);
+        }
+
+        $result = $this->assign->new_submission_empty($submission);
+        $this->assertTrue($result === $expected);
+    }
+
+    /**
+     * Dataprovider for the test_submission_is_empty testcase
+     *
+     * @return array of testcases
+     */
+    public function submission_is_empty_testcases() {
+        return [
+            'With file' => [
+                [
+                    'component' => 'user',
+                    'filearea' => 'draft',
+                    'filepath' => '/',
+                    'filename' => 'not_a_virus.exe'
+                ],
+                false
+            ],
+            'Without file' => [null, true]
+        ];
+    }
+
+
+}

mod/assign/submission/onlinetext/locallib.php

@@ -572,6 +572,22 @@ class assign_submission_onlinetext extends assign_submission_plugin {
         return empty($onlinetextsubmission->onlinetext);
     }
 
+    /**
+     * Determine if a submission is empty
+     *
+     * This is distinct from is_empty in that it is intended to be used to
+     * determine if a submission made before saving is empty.
+     *
+     * @param stdClass $data The submission data
+     * @return bool
+     */
+    public function submission_is_empty(stdClass $data) {
+        if (!isset($data->onlinetext_editor)) {
+            return true;
+        }
+        return !strlen((string)$data->onlinetext_editor['text']);
+    }
+
     /**
      * Get file areas returns a list of areas this plugin stores files
      * @return array - An array of fileareas (keys) and descriptions (values)

mod/assign/submission/onlinetext/tests/locallib_test.php

@@ -0,0 +1,116 @@
+<?php
+// This file is part of Moodle - http://moodle.org/
+//
+// Moodle is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+//
+// Moodle is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+//
+// You should have received a copy of the GNU General Public License
+// along with Moodle.  If not, see <http://www.gnu.org/licenses/>.
+
+/**
+ * Tests for mod/assign/submission/onlinetext/locallib.php
+ *
+ * @package   assignsubmission_onlinetext
+ * @copyright 2016 Cameron Ball
+ * @license   http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+
+defined('MOODLE_INTERNAL') || die();
+
+global $CFG;
+require_once($CFG->dirroot . '/mod/assign/tests/base_test.php');
+
+/**
+ * Unit tests for mod/assign/submission/onlinetext/locallib.php
+ *
+ * @copyright  2016 Cameron Ball
+ * @license    http://www.gnu.org/copyleft/gpl.html GNU GPL v3 or later
+ */
+class assignsubmission_onlinetext_locallib_testcase extends advanced_testcase {
+
+    /** @var stdClass $user A user to submit an assignment. */
+    protected $user;
+
+    /** @var stdClass $course New course created to hold the assignment activity. */
+    protected $course;
+
+    /** @var stdClass $cm A context module object. */
+    protected $cm;
+
+    /** @var stdClass $context Context of the assignment activity. */
+    protected $context;
+
+    /** @var stdClass $assign The assignment object. */
+    protected $assign;
+
+    /**
+     * Setup all the various parts of an assignment activity including creating an onlinetext submission.
+     */
+    protected function setUp() {
+        $this->user = $this->getDataGenerator()->create_user();
+        $this->course = $this->getDataGenerator()->create_course();
+        $generator = $this->getDataGenerator()->get_plugin_generator('mod_assign');
+        $params = ['course' => $this->course->id, 'assignsubmission_onlinetext_enabled' => 1];
+        $instance = $generator->create_instance($params);
+        $this->cm = get_coursemodule_from_instance('assign', $instance->id);
+        $this->context = context_module::instance($this->cm->id);
+        $this->assign = new testable_assign($this->context, $this->cm, $this->course);
+        $this->setUser($this->user->id);
+    }
+
+    /**
+     * Test submission_is_empty
+     *
+     * @dataProvider submission_is_empty_testcases
+     * @param string $submissiontext The online text submission text
+     * @param bool $expected The expected return value
+     */
+    public function test_submission_is_empty($submissiontext, $expected) {
+        $this->resetAfterTest();
+
+        $plugin = $this->assign->get_submission_plugin_by_type('onlinetext');
+        $data = new stdClass();
+        $data->onlinetext_editor = ['text' => $submissiontext];
+
+        $result = $plugin->submission_is_empty($data);
+        $this->assertTrue($result === $expected);
+    }
+
+    /**
+     * Test new_submission_empty
+     *
+     * @dataProvider submission_is_empty_testcases
+     * @param string $submissiontext The file submission data
+     * @param bool $expected The expected return value
+     */
+    public function test_new_submission_empty($submissiontext, $expected) {
+        $this->resetAfterTest();
+        $data = new stdClass();
+        $data->onlinetext_editor = ['text' => $submissiontext];
+
+        $result = $this->assign->new_submission_empty($data);
+        $this->assertTrue($result === $expected);
+    }
+
+    /**
+     * Dataprovider for the test_submission_is_empty testcase
+     *
+     * @return array of testcases
+     */
+    public function submission_is_empty_testcases() {
+        return [
+            'Empty submission string' => ['', true],
+            'Empty submission null' => [null, true],
+            'Value 0' => [0, false],
+            'String 0' => ['0', false],
+            'Text' => ['Ai! laurië lantar lassi súrinen, yéni únótimë ve rámar aldaron!', false]
+        ];
+    }
+}

mod/assign/submissionplugin.php

@@ -125,4 +125,16 @@ abstract class assign_submission_plugin extends assign_plugin {
     public function add_attempt(stdClass $oldsubmission, stdClass $newsubmission) {
     }
 
+    /**
+     * Determine if a submission is empty
+     *
+     * This is distinct from is_empty in that it is intended to be used to
+     * determine if a submission made before saving is empty.
+     *
+     * @param stdClass $data The submission data
+     * @return bool
+     */
+    public function submission_is_empty(stdClass $data) {
+        return false;
+    }
 }

mod/assign/tests/locallib_test.php

@@ -687,6 +687,63 @@ class mod_assign_locallib_testcase extends mod_assign_base_testcase {
         $this->assertContains(get_string('submitassignment', 'assign'), $output, 'Can submit non empty onlinetext assignment');
     }
 
+    /**
+     * Test new_submission_empty
+     *
+     * We only test combinations of plugins here. Individual plugins are tested
+     * in their respective test files.
+     *
+     * @dataProvider test_new_submission_empty_testcases
+     * @param string $data The file submission data
+     * @param bool $expected The expected return value
+     */
+    public function test_new_submission_empty($data, $expected) {
+        $this->resetAfterTest();
+        $assign = $this->create_instance(['assignsubmission_file_enabled' => 1,
+                                          'assignsubmission_file_maxfiles' => 12,
+                                          'assignsubmission_file_maxsizebytes' => 10,
+                                          'assignsubmission_onlinetext_enabled' => 1]);
+        $this->setUser($this->students[0]);
+        $submission = new stdClass();
+
+        if ($data['file'] && isset($data['file']['filename'])) {
+            $itemid = file_get_unused_draft_itemid();
+            $submission->files_filemanager = $itemid;
+            $data['file'] += ['contextid' => context_user::instance($this->students[0]->id)->id, 'itemid' => $itemid];
+            $fs = get_file_storage();
+            $fs->create_file_from_string((object)$data['file'], 'Content of ' . $data['file']['filename']);
+        }
+
+        if ($data['onlinetext']) {
+            $submission->onlinetext_editor = ['text' => $data['onlinetext']];
+        }
+
+        $result = $assign->new_submission_empty($submission);
+        $this->assertTrue($result === $expected);
+    }
+
+    /**
+     * Dataprovider for the test_new_submission_empty testcase
+     *
+     * @return array of testcases
+     */
+    public function test_new_submission_empty_testcases() {
+        return [
+            'With file and onlinetext' => [
+                [
+                    'file' => [
+                        'component' => 'user',
+                        'filearea' => 'draft',
+                        'filepath' => '/',
+                        'filename' => 'not_a_virus.exe'
+                    ],
+                    'onlinetext' => 'Balin Fundinul Uzbadkhazaddumu'
+                ],
+                false
+            ]
+        ];
+    }
+
     public function test_list_participants() {
         global $CFG, $DB;
 

mod/assign/upgrade.txt

@@ -3,6 +3,9 @@ This files describes API changes in the assign code.
 === 3.2 ===
 * External function mod_assign_external::get_assignments now returns additional optional fields:
    - preventsubmissionnotingroup: Prevent submission not in group.
+* Proper checking for empty submissions
+* Submission modification time checking - this will help students working in groups not clobber each others'
+  submissions
 
 === 3.1 ===
 * The feedback plugins now need to implement the is_feedback_modified() method. The default is to return true