Enable and fix PHPUnit integration tests

Signed-off-by: Christian Hartmann <chris-hartmann@gmx.de>

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

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);
 

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(),
 		];
 	}
 }

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()
 		];
 	}
 }

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(),
 		];
 	}
 }

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);
 	}

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(),
 		];
 	}

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(),
 		];
 	}

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']);

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,

lib/Migration/Version030100Date20230115214242.php

@@ -0,0 +1,67 @@
+<?php
+
+declare(strict_types=1);
+/**
+ * @copyright Copyright (c) 2023 Jonas Rittershofer <jotoeri@users.noreply.github.com>
+ *
+ * @author Joas Schilling <coding@schilljs.com>
+ * @author Jonas Rittershofer <jotoeri@users.noreply.github.com>
+ *
+ * @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 <http://www.gnu.org/licenses/>.
+ *
+ */
+
+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;
+	}
+}

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","",""'
 			]
 		];
 	}

tests/phpunit.integration.xml

@@ -4,8 +4,7 @@
 		 convertDeprecationsToExceptions="true"
 		 verbose="true">
 	<testsuite name="Forms integration tests">
-		<!-- Disabling the tests until they exist. Error otherwise.-->
-		<!-- <directory>./Integration</directory> -->
+		<directory>./Integration</directory>
 	</testsuite>
 	<coverage>
 		<!-- filters for code coverage -->