diff --git a/.github/workflows/phpunit.yml b/.github/workflows/phpunit.yml index 6eb43420..02e2474d 100644 --- a/.github/workflows/phpunit.yml +++ b/.github/workflows/phpunit.yml @@ -57,8 +57,9 @@ jobs: OC_PASS: test run: | mkdir data - ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password ./occ user:add --password-from-env --display-name="Test Displayname" test + ./occ user:add --password-from-env --display-name="User No. 1" user1 ./occ app:enable --force ${{ env.APP_NAME }} php -S localhost:8080 & @@ -141,8 +142,9 @@ jobs: OC_PASS: test run: | mkdir data - ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password ./occ user:add --password-from-env --display-name="Test Displayname" test + ./occ user:add --password-from-env --display-name="User No. 1" user1 ./occ app:enable --force ${{ env.APP_NAME }} php -S localhost:8080 & @@ -209,8 +211,9 @@ jobs: OC_PASS: test run: | mkdir data - ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password + ./occ maintenance:install --verbose --database=${{ matrix.databases }} --database-name=nextcloud --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=root --database-pass=rootpassword --admin-user admin --admin-pass password ./occ user:add --password-from-env --display-name="Test Displayname" test + ./occ user:add --password-from-env --display-name="User No. 1" user1 ./occ app:enable --force ${{ env.APP_NAME }} php -S localhost:8080 & @@ -273,9 +276,10 @@ jobs: run: | mkdir data ./occ maintenance:install --verbose --database=oci --database-name=XE --database-host=127.0.0.1 --database-port=$DB_PORT --database-user=autotest --database-pass=owncloud --admin-user admin --admin-pass admin - php -f index.php ./occ user:add --password-from-env --display-name="Test Displayname" test + ./occ user:add --password-from-env --display-name="User No. 1" user1 ./occ app:enable --force ${{ env.APP_NAME }} + php -S localhost:8080 & - name: PHPUnit working-directory: apps/${{ env.APP_NAME }} diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index da2958f2..c728afc9 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -267,6 +267,8 @@ class ApiController extends OCSController { ]); $form->setSubmitMultiple(false); $form->setShowExpiration(false); + $form->setExpires(0); + $form->setIsAnonymous(false); $this->formMapper->insert($form); diff --git a/lib/Db/Answer.php b/lib/Db/Answer.php index 84083b4e..0e5c2e91 100644 --- a/lib/Db/Answer.php +++ b/lib/Db/Answer.php @@ -56,7 +56,7 @@ class Answer extends Entity { 'id' => $this->getId(), 'submissionId' => $this->getSubmissionId(), 'questionId' => $this->getQuestionId(), - 'text' => htmlspecialchars_decode($this->getText()), + 'text' => (string)$this->getText(), ]; } } diff --git a/lib/Db/Form.php b/lib/Db/Form.php index 5b67b3d6..cb0dc445 100644 --- a/lib/Db/Form.php +++ b/lib/Db/Form.php @@ -89,15 +89,15 @@ class Form extends Entity { return [ 'id' => $this->getId(), 'hash' => $this->getHash(), - 'title' => $this->getTitle(), - 'description' => $this->getDescription(), + 'title' => (string)$this->getTitle(), + 'description' => (string)$this->getDescription(), 'ownerId' => $this->getOwnerId(), 'created' => $this->getCreated(), 'access' => $this->getAccess(), - 'expires' => $this->getExpires(), - 'isAnonymous' => $this->getIsAnonymous(), - 'submitMultiple' => $this->getSubmitMultiple(), - 'showExpiration' => $this->getShowExpiration() + 'expires' => (int)$this->getExpires(), + 'isAnonymous' => (bool)$this->getIsAnonymous(), + 'submitMultiple' => (bool)$this->getSubmitMultiple(), + 'showExpiration' => (bool)$this->getShowExpiration() ]; } } diff --git a/lib/Db/Option.php b/lib/Db/Option.php index 6b586311..e3495a0e 100644 --- a/lib/Db/Option.php +++ b/lib/Db/Option.php @@ -53,7 +53,7 @@ class Option extends Entity { return [ 'id' => $this->getId(), 'questionId' => $this->getQuestionId(), - 'text' => htmlspecialchars_decode($this->getText()), + 'text' => (string)$this->getText(), ]; } } diff --git a/lib/Db/OptionMapper.php b/lib/Db/OptionMapper.php index 34ebb9c6..9f68689a 100644 --- a/lib/Db/OptionMapper.php +++ b/lib/Db/OptionMapper.php @@ -54,7 +54,8 @@ class OptionMapper extends QBMapper { ->from($this->getTableName()) ->where( $qb->expr()->eq('question_id', $qb->createNamedParameter($questionId)) - ); + ) + ->orderBy('id'); return $this->findEntities($qb); } diff --git a/lib/Db/Question.php b/lib/Db/Question.php index cf5383eb..61cb9b72 100644 --- a/lib/Db/Question.php +++ b/lib/Db/Question.php @@ -81,11 +81,11 @@ class Question extends Entity { return [ 'id' => $this->getId(), 'formId' => $this->getFormId(), - 'order' => $this->getOrder(), - 'type' => htmlspecialchars_decode($this->getType()), - 'isRequired' => $this->getIsRequired(), - 'text' => htmlspecialchars_decode($this->getText()), - 'description' => htmlspecialchars_decode($this->getDescription()), + 'order' => (int)$this->getOrder(), + 'type' => $this->getType(), + 'isRequired' => (bool)$this->getIsRequired(), + 'text' => (string)$this->getText(), + 'description' => (string)$this->getDescription(), 'extraSettings' => $this->getExtraSettings(), ]; } diff --git a/lib/Db/Share.php b/lib/Db/Share.php index c4861499..1273f17e 100644 --- a/lib/Db/Share.php +++ b/lib/Db/Share.php @@ -57,7 +57,7 @@ class Share extends Entity { return [ 'id' => $this->getId(), 'formId' => $this->getFormId(), - 'shareType' => $this->getShareType(), + 'shareType' => (int)$this->getShareType(), 'shareWith' => $this->getShareWith(), ]; } diff --git a/lib/Migration/Version010200Date20200323141300.php b/lib/Migration/Version010200Date20200323141300.php index 634f3a0a..ba9d345d 100644 --- a/lib/Migration/Version010200Date20200323141300.php +++ b/lib/Migration/Version010200Date20200323141300.php @@ -86,7 +86,7 @@ class Version010200Date20200323141300 extends SimpleMigrationStep { 'length' => 64, ]); $table->addColumn('title', Types::STRING, [ - 'notnull' => true, + 'notnull' => false, 'length' => 256, ]); $table->addColumn('description', Types::TEXT, [ @@ -131,7 +131,7 @@ class Version010200Date20200323141300 extends SimpleMigrationStep { 'notnull' => true, ]); $table->addColumn('order', Types::INTEGER, [ - 'notnull' => true, + 'notnull' => false, 'default' => 1, ]); $table->addColumn('type', Types::STRING, [ @@ -143,7 +143,7 @@ class Version010200Date20200323141300 extends SimpleMigrationStep { 'default' => 0, ]); $table->addColumn('text', Types::STRING, [ - 'notnull' => true, + 'notnull' => false, 'length' => 2048, ]); $table->setPrimaryKey(['id']); @@ -159,7 +159,7 @@ class Version010200Date20200323141300 extends SimpleMigrationStep { 'notnull' => true, ]); $table->addColumn('text', Types::STRING, [ - 'notnull' => true, + 'notnull' => false, 'length' => 1024, ]); $table->setPrimaryKey(['id']); @@ -198,7 +198,7 @@ class Version010200Date20200323141300 extends SimpleMigrationStep { 'notnull' => true, ]); $table->addColumn('text', Types::TEXT, [ - 'notnull' => true, + 'notnull' => false, 'length' => 4096, ]); $table->setPrimaryKey(['id']); diff --git a/lib/Migration/Version030000Date20211206213004.php b/lib/Migration/Version030000Date20211206213004.php index 2e79e406..613c83c4 100644 --- a/lib/Migration/Version030000Date20211206213004.php +++ b/lib/Migration/Version030000Date20211206213004.php @@ -72,7 +72,7 @@ class Version030000Date20211206213004 extends SimpleMigrationStep { 'notnull' => true, ]); $table->addColumn('share_type', Types::SMALLINT, [ - 'notnull' => true, + 'notnull' => false, ]); $table->addColumn('share_with', Types::STRING, [ 'length' => 256, diff --git a/lib/Migration/Version030100Date20230115214242.php b/lib/Migration/Version030100Date20230115214242.php new file mode 100644 index 00000000..e61eb6b6 --- /dev/null +++ b/lib/Migration/Version030100Date20230115214242.php @@ -0,0 +1,67 @@ + + * + * @author Joas Schilling + * @author Jonas Rittershofer + * + * @license AGPL-3.0-or-later + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU Affero General Public License as + * published by the Free Software Foundation, either version 3 of the + * License, or (at your option) any later version. + * + * This program 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 Affero General Public License for more details. + * + * You should have received a copy of the GNU Affero General Public License + * along with this program. If not, see . + * + */ + +namespace OCA\Forms\Migration; + +use Closure; +use OCP\DB\ISchemaWrapper; +use OCP\Migration\IOutput; +use OCP\Migration\SimpleMigrationStep; + +class Version030100Date20230115214242 extends SimpleMigrationStep { + + /** + * @param IOutput $output + * @param Closure $schemaClosure The `\Closure` returns a `ISchemaWrapper` + * @param array $options + * @return null|ISchemaWrapper + */ + public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { + /** @var ISchemaWrapper $schema */ + $schema = $schemaClosure(); + + $result = $this->ensureColumnIsNullable($schema, 'forms_v2_answers', 'text'); + $result |= $this->ensureColumnIsNullable($schema, 'forms_v2_forms', 'title'); + $result |= $this->ensureColumnIsNullable($schema, 'forms_v2_options', 'text'); + $result |= $this->ensureColumnIsNullable($schema, 'forms_v2_questions', 'order'); + $result |= $this->ensureColumnIsNullable($schema, 'forms_v2_questions', 'text'); + $result |= $this->ensureColumnIsNullable($schema, 'forms_v2_shares', 'share_type'); + + return $result ? $schema : null; + } + + protected function ensureColumnIsNullable(ISchemaWrapper $schema, string $tableName, string $columnName): bool { + $table = $schema->getTable($tableName); + $column = $table->getColumn($columnName); + + if ($column->getNotnull()) { + $column->setNotnull(false); + return true; + } + + return false; + } +} diff --git a/tests/Integration/Api/ApiV2Test.php b/tests/Integration/Api/ApiV2Test.php index 166ff11e..b49c2cea 100644 --- a/tests/Integration/Api/ApiV2Test.php +++ b/tests/Integration/Api/ApiV2Test.php @@ -41,117 +41,146 @@ class ApiV2Test extends TestCase { private $formMapper; /** @var Array */ - private $testForms = [ - [ - 'hash' => 'abcdefg', - 'title' => 'Title of a Form', - 'description' => 'Just a simple form.', - 'owner_id' => 'test', - 'access_json' => [ - 'permitAllUsers' => false, - 'showToAllUsers' => false - ], - 'created' => 12345, - 'expires' => 0, - 'is_anonymous' => false, - 'submit_multiple' => false, - 'show_expiration' => false, - 'questions' => [ - [ - 'type' => 'short', - 'text' => 'First Question?', - 'isRequired' => true, - 'order' => 1, - 'options' => [] + private $testForms; + + /** + * Store Test Forms Array. + * Necessary as function due to object type-casting. + */ + private function setTestForms() { + $this->testForms = [ + [ + 'hash' => 'abcdefg', + 'title' => 'Title of a Form', + 'description' => 'Just a simple form.', + 'owner_id' => 'test', + 'access_json' => [ + 'permitAllUsers' => false, + 'showToAllUsers' => false ], - [ - 'type' => 'multiple_unique', - 'text' => 'Second Question?', - 'isRequired' => false, - 'order' => 2, - 'options' => [ - [ - 'text' => 'Option 1' + 'created' => 12345, + 'expires' => 0, + 'is_anonymous' => false, + 'submit_multiple' => false, + 'show_expiration' => false, + 'questions' => [ + [ + 'type' => 'short', + 'text' => 'First Question?', + 'description' => 'Please answer this.', + 'isRequired' => true, + 'order' => 1, + 'options' => [], + 'extraSettings' => (object)[] + ], + [ + 'type' => 'multiple_unique', + 'text' => 'Second Question?', + 'description' => '', + 'isRequired' => false, + 'order' => 2, + 'options' => [ + [ + 'text' => 'Option 1' + ], + [ + 'text' => 'Option 2' + ], + [ + 'text' => '' + ] ], - [ - 'text' => 'Option 2' + 'extraSettings' => (object)[ + 'shuffleOptions' => true + ] + ] + ], + 'shares' => [ + [ + 'shareType' => 0, + 'shareWith' => 'user1', + ], + [ + 'shareType' => 3, + 'shareWith' => 'shareHash', + ], + ], + 'submissions' => [ + [ + 'userId' => 'user1', + 'timestamp' => 123456, + 'answers' => [ + [ + 'questionIndex' => 0, + 'text' => 'This is a short answer.' + ], + [ + 'questionIndex' => 1, + 'text' => 'Option 1' + ] + ] + ], + [ + 'userId' => 'user2', + 'timestamp' => 12345, + 'answers' => [ + [ + 'questionIndex' => 0, + 'text' => 'This is another short answer.' + ], + [ + 'questionIndex' => 1, + 'text' => 'Option 2' + ] + ] + ], + [ + 'userId' => 'user3', + 'timestamp' => 1234, + 'answers' => [ + [ + 'questionIndex' => 0, + 'text' => '' + ] ] ] ] ], - 'shares' => [ - [ - 'shareType' => 0, - 'shareWith' => 'user1', + [ + 'hash' => 'abcdefghij', + 'title' => 'Title of a second Form', + 'description' => '', + 'owner_id' => 'someUser', + 'access_json' => [ + 'permitAllUsers' => true, + 'showToAllUsers' => true ], - [ - 'shareType' => 3, - 'shareWith' => 'shareHash', + 'created' => 12345, + 'expires' => 0, + 'is_anonymous' => false, + 'submit_multiple' => false, + 'show_expiration' => false, + 'questions' => [ + [ + 'type' => 'short', + 'text' => 'Third Question?', + 'description' => '', + 'isRequired' => false, + 'order' => 1, + 'options' => [], + 'extraSettings' => (object)[] + ], ], - ], - 'submissions' => [ - [ - 'userId' => 'user1', - 'timestamp' => 123456, - 'answers' => [ - [ - 'questionIndex' => 0, - 'text' => 'This is a short answer.' - ], - [ - 'questionIndex' => 1, - 'text' => 'Option 1' - ] - ] + 'shares' => [ + [ + 'shareType' => 0, + 'shareWith' => 'user2', + ], ], - [ - 'userId' => 'user2', - 'timestamp' => 12345, - 'answers' => [ - [ - 'questionIndex' => 0, - 'text' => 'This is another short answer.' - ], - [ - 'questionIndex' => 1, - 'text' => 'Option 2' - ] - ] - ] + 'submissions' => [] ] - ], - [ - 'hash' => 'abcdefghij', - 'title' => 'Title of a second Form', - 'description' => '', - 'owner_id' => 'someUser', - 'access_json' => [ - 'permitAllUsers' => true, - 'showToAllUsers' => true - ], - 'created' => 12345, - 'expires' => 0, - 'is_anonymous' => false, - 'submit_multiple' => false, - 'show_expiration' => false, - 'questions' => [ - [ - 'type' => 'short', - 'text' => 'Third Question?', - 'isRequired' => false, - 'order' => 1, - 'options' => [] - ], - ], - 'shares' => [ - [ - 'shareType' => 0, - 'shareWith' => 'user2', - ], - ], - 'submissions' => [] - ] - ]; + ]; + } /** * Set up test environment. @@ -159,6 +188,7 @@ class ApiV2Test extends TestCase { */ public function setUp(): void { parent::setUp(); + $this->setTestForms(); $qb = TestCase::$realDatabase->getQueryBuilder(); @@ -189,7 +219,9 @@ class ApiV2Test extends TestCase { 'order' => $qb->createNamedParameter($question['order'], IQueryBuilder::PARAM_INT), 'type' => $qb->createNamedParameter($question['type'], IQueryBuilder::PARAM_STR), 'is_required' => $qb->createNamedParameter($question['isRequired'], IQueryBuilder::PARAM_BOOL), - 'text' => $qb->createNamedParameter($question['text'], IQueryBuilder::PARAM_STR) + 'text' => $qb->createNamedParameter($question['text'], IQueryBuilder::PARAM_STR), + 'description' => $qb->createNamedParameter($question['description'], IQueryBuilder::PARAM_STR), + 'extra_settings_json' => $qb->createNamedParameter(json_encode($question['extraSettings']), IQueryBuilder::PARAM_STR), ]); $qb->execute(); $questionId = $qb->getLastInsertId(); @@ -326,7 +358,8 @@ class ApiV2Test extends TestCase { 'results', 'submit' ], - 'partial' => true + 'partial' => true, + 'submissionCount' => 3 ]] ] ]; @@ -431,6 +464,7 @@ class ApiV2Test extends TestCase { ], 'questions' => [], 'shares' => [], + 'submissionCount' => 0, ] ] ]; @@ -489,7 +523,9 @@ class ApiV2Test extends TestCase { 'text' => 'First Question?', 'isRequired' => true, 'order' => 1, - 'options' => [] + 'options' => [], + 'description' => 'Please answer this.', + 'extraSettings' => [] ], [ 'type' => 'multiple_unique', @@ -502,7 +538,14 @@ class ApiV2Test extends TestCase { ], [ 'text' => 'Option 2' + ], + [ + 'text' => '' ] + ], + 'description' => '', + 'extraSettings' => [ + 'shuffleOptions' => true, ] ] ], @@ -510,7 +553,7 @@ class ApiV2Test extends TestCase { [ 'shareType' => 0, 'shareWith' => 'user1', - 'displayName' => '' + 'displayName' => 'User No. 1' ], [ 'shareType' => 3, @@ -518,6 +561,7 @@ class ApiV2Test extends TestCase { 'displayName' => '' ], ], + 'submissionCount' => 3 ] ] ]; @@ -559,6 +603,7 @@ class ApiV2Test extends TestCase { // Compared to full form expected, update changed properties $fullFormExpected['title'] = 'Title of a Form - Copy'; $fullFormExpected['shares'] = []; + $fullFormExpected['submissionCount'] = 0; // Compared to full form expected, unset unpredictable properties. These will be checked logically. unset($fullFormExpected['id']); unset($fullFormExpected['hash']); @@ -682,7 +727,21 @@ class ApiV2Test extends TestCase { 'type' => 'short', 'isRequired' => false, 'text' => 'Already some Question?', - 'options' => [] + 'options' => [], + 'description' => '', + 'extraSettings' => [], + ] + ], + 'emptyQuestion' => [ + 'expected' => [ + // 'formId' => 3, // Checked during test + // 'order' => 3, // Checked during test + 'type' => 'short', + 'isRequired' => false, + 'text' => '', + 'options' => [], + 'description' => '', + 'extraSettings' => [], ] ] ]; @@ -697,7 +756,7 @@ class ApiV2Test extends TestCase { 'json' => [ 'formId' => $this->testForms[0]['id'], 'type' => 'short', - 'text' => 'Already some Question?' + 'text' => $expected['text'] ] ]); $data = $this->OcsResponse2Data($resp); @@ -984,7 +1043,7 @@ class ApiV2Test extends TestCase { [ // 'formId' => Checked dynamically 'userId' => 'user1', - 'userDisplayName' => 'user1', + 'userDisplayName' => 'User No. 1', 'timestamp' => 123456, 'answers' => [ [ @@ -1016,6 +1075,19 @@ class ApiV2Test extends TestCase { 'text' => 'Option 2' ] ] + ], + [ + // 'formId' => Checked dynamically + 'userId' => 'user3', + 'userDisplayName' => 'user3', + 'timestamp' => 1234, + 'answers' => [ + [ + // 'submissionId' => Checked dynamically + // 'questionId' => Checked dynamically + 'text' => '' + ] + ] ] ], 'questions' => $this->dataGetFullForm()['getFullForm']['expected']['questions'] @@ -1068,9 +1140,10 @@ class ApiV2Test extends TestCase { return [ 'exportSubmissions' => [ 'expected' => ' - "User display name","Timestamp","First Question?","Second Question?" - "user1","Friday, January 2, 1970 at 10:17:36 AM GMT+0:00","This is a short answer.","Option 1" - "user2","Thursday, January 1, 1970 at 3:25:45 AM GMT+0:00","This is another short answer.","Option 2"' + "User ID","User display name","Timestamp","First Question?","Second Question?" + "user1","User No. 1","Friday, January 2, 1970 at 10:17:36 AM GMT+0:00","This is a short answer.","Option 1" + "","Anonymous user","Thursday, January 1, 1970 at 3:25:45 AM GMT+0:00","This is another short answer.","Option 2" + "","Anonymous user","Thursday, January 1, 1970 at 12:20:34 AM GMT+0:00","",""' ] ]; } diff --git a/tests/phpunit.integration.xml b/tests/phpunit.integration.xml index 0441e6b5..cccf71d1 100644 --- a/tests/phpunit.integration.xml +++ b/tests/phpunit.integration.xml @@ -4,8 +4,7 @@ convertDeprecationsToExceptions="true" verbose="true"> - - + ./Integration