diff --git a/lib/BackgroundJob/DeleteQuestionFoldersJob.php b/lib/BackgroundJob/DeleteQuestionFoldersJob.php new file mode 100644 index 000000000..28c5123aa --- /dev/null +++ b/lib/BackgroundJob/DeleteQuestionFoldersJob.php @@ -0,0 +1,86 @@ +logger->debug('Deleting question folders for question {questionId} in form {formId}', [ + 'questionId' => $questionId, + 'formId' => $formId, + ]); + + $formFolders = $this->filePathHelper->getAllFormFoldersById($formId, $ownerId); + if (empty($formFolders)) { + $this->logger->notice('Form folder not found, nothing to delete', [ + 'formId' => $formId, + ]); + return; + } + + $questionFolderPrefix = $questionId . ' - '; + $deletedCount = 0; + + // Iterate through all form folders (handles form renames) + foreach ($formFolders as $formFolder) { + // Iterate through submission folders and delete matching question folders + foreach ($formFolder->getDirectoryListing() as $submissionFolder) { + if (!$submissionFolder instanceof Folder) { + continue; + } + foreach ($submissionFolder->getDirectoryListing() as $questionFolder) { + if (str_starts_with($questionFolder->getName(), $questionFolderPrefix)) { + $questionFolder->delete(); + $deletedCount++; + } + } + } + } + + $this->logger->info('Deleted {count} question folders for question {questionId}', [ + 'count' => $deletedCount, + 'questionId' => $questionId, + 'formId' => $formId, + ]); + } catch (NotFoundException) { + // Folder doesn't exist, do nothing + $this->logger->notice('Question folder not found, nothing to delete', [ + 'questionId' => $questionId, + 'formId' => $formId, + ]); + } catch (\Throwable $e) { + $this->logger->warning('Failed to delete question folders: {error}', [ + 'error' => $e->getMessage(), + 'questionId' => $questionId, + 'formId' => $formId, + ]); + } + } +} diff --git a/lib/Controller/ApiController.php b/lib/Controller/ApiController.php index cde1c4f8f..3ad8ae46b 100644 --- a/lib/Controller/ApiController.php +++ b/lib/Controller/ApiController.php @@ -7,6 +7,7 @@ namespace OCA\Forms\Controller; +use OCA\Forms\BackgroundJob\DeleteQuestionFoldersJob; use OCA\Forms\BackgroundJob\SyncSubmissionsWithLinkedFileJob; use OCA\Forms\Constants; use OCA\Forms\Db\Answer; @@ -45,6 +46,7 @@ use OCP\AppFramework\OCS\OCSNotFoundException; use OCP\AppFramework\OCSController; use OCP\BackgroundJob\IJobList; +use OCP\Files\Folder; use OCP\Files\IMimeTypeDetector; use OCP\Files\IRootFolder; use OCP\IL10N; @@ -743,6 +745,14 @@ public function deleteQuestion(int $formId, int $questionId): DataResponse { $question->setOrder(0); $this->questionMapper->update($question); + if ($question->getType() === Constants::ANSWER_TYPE_FILE) { + $this->jobList->add(DeleteQuestionFoldersJob::class, [ + 'formId' => $form->getId(), + 'questionId' => $question->getId(), + 'ownerId' => $form->getOwnerId(), + ]); + } + // Update all question-order > deleted order. $formQuestions = $this->questionMapper->findByForm($formId); foreach ($formQuestions as $question) { @@ -1589,7 +1599,7 @@ public function deleteSubmission(int $formId, int $submissionId): DataResponse { } // Delete submission (incl. Answers) - $this->submissionMapper->deleteById($submissionId); + $this->submissionMapper->deleteById($form, $submissionId); $this->formMapper->update($form); return new DataResponse($submissionId); @@ -1743,8 +1753,7 @@ public function uploadFiles(int $formId, int $questionId, string $shareHash = '' } else { $folder = $userFolder->newFolder($path); } - /** @var \OCP\Files\Folder $folder */ - + /** @var Folder $folder */ $fileName = $folder->getNonExistingName($uploadedFile['name']); $file = $folder->newFile($fileName, file_get_contents($uploadedFile['tmp_name'])); @@ -1819,8 +1828,7 @@ private function storeAnswersForQuestion(Form $form, $submissionId, array $quest } else { $folder = $userFolder->newFolder($path); } - /** @var \OCP\Files\Folder $folder */ - + /** @var Folder $folder */ $file = $userFolder->getById($uploadedFile->getFileId())[0]; $name = $folder->getNonExistingName($file->getName()); $file->move($folder->getPath() . '/' . $name); diff --git a/lib/Controller/ShareApiController.php b/lib/Controller/ShareApiController.php index 7323ab951..84954dfb8 100644 --- a/lib/Controller/ShareApiController.php +++ b/lib/Controller/ShareApiController.php @@ -14,6 +14,7 @@ use OCA\Forms\Db\FormMapper; use OCA\Forms\Db\Share; use OCA\Forms\Db\ShareMapper; +use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\ResponseDefinitions; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; @@ -62,6 +63,7 @@ public function __construct( private ISecureRandom $secureRandom, private CirclesService $circlesService, private IRootFolder $rootFolder, + private FilePathHelper $filePathHelper, private IManager $shareManager, ) { parent::__construct($appName, $request); @@ -273,7 +275,7 @@ public function updateShare(int $formId, int $shareId, array $keyValuePairs): Da if (in_array($formShare->getShareType(), [IShare::TYPE_USER, IShare::TYPE_GROUP, IShare::TYPE_USERGROUP, IShare::TYPE_CIRCLE], true)) { if (in_array(Constants::PERMISSION_RESULTS, $keyValuePairs['permissions'], true)) { $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); - $uploadedFilesFolderPath = $this->formsService->getFormUploadedFilesFolderPath($form); + $uploadedFilesFolderPath = $this->filePathHelper->getFormUploadedFilesFolderPath($form); try { /** @var \OCP\Files\Folder $folder */ $folder = $userFolder->get($uploadedFilesFolderPath); @@ -358,7 +360,7 @@ private function removeUploadedFilesShare(Form $form, Share $formShare): void { } $userFolder = $this->rootFolder->getUserFolder($form->getOwnerId()); - $uploadedFilesFolderPath = $this->formsService->getFormUploadedFilesFolderPath($form); + $uploadedFilesFolderPath = $this->filePathHelper->getFormUploadedFilesFolderPath($form); try { $folder = $userFolder->get($uploadedFilesFolderPath); } catch (NotFoundException $e) { diff --git a/lib/Db/FormMapper.php b/lib/Db/FormMapper.php index f7df9ddad..251dc429d 100644 --- a/lib/Db/FormMapper.php +++ b/lib/Db/FormMapper.php @@ -8,13 +8,16 @@ namespace OCA\Forms\Db; use OCA\Forms\Constants; +use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\Service\ConfigService; use OCP\AppFramework\Db\Entity; use OCP\AppFramework\Db\QBMapper; use OCP\Comments\ICommentsManager; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Folder; use OCP\IDBConnection; use OCP\Share\IShare; +use Psr\Log\LoggerInterface; /** * @extends QBMapper
@@ -32,6 +35,8 @@ public function __construct( private SubmissionMapper $submissionMapper, private ConfigService $configService, private ICommentsManager $commentsManager, + private FilePathHelper $filePathHelper, + private LoggerInterface $logger, ) { parent::__construct($db, 'forms_v2_forms', Form::class); } @@ -224,6 +229,33 @@ public function deleteForm(Form $form): void { $this->shareMapper->deleteByForm($formId); $this->questionMapper->deleteByForm($formId); $this->commentsManager->deleteCommentsAtObject('forms', (string)$formId); + $this->deleteFormFolder($form); $this->delete($form); } + + /** + * Delete the form folder from the file system + * @param Form $form The form instance + */ + private function deleteFormFolder(Form $form): void { + try { + $formsFolder = $this->filePathHelper->getFormsFolder($form->getOwnerId()); + if ($formsFolder === null) { + return; + } + $formFolderPrefix = $form->getId() . ' - '; + + // Iterate through form folders and delete matching folders + foreach ($formsFolder->getDirectoryListing() as $node) { + if (str_starts_with($node->getName(), $formFolderPrefix)) { + $node->delete(); + } + } + } catch (\Throwable $e) { + $this->logger->warning('Failed to delete form folder: {error}', [ + 'error' => $e->getMessage(), + 'formId' => $form->getId(), + ]); + } + } } diff --git a/lib/Db/SubmissionMapper.php b/lib/Db/SubmissionMapper.php index 367d94504..68736a566 100644 --- a/lib/Db/SubmissionMapper.php +++ b/lib/Db/SubmissionMapper.php @@ -7,10 +7,13 @@ namespace OCA\Forms\Db; +use OCA\Forms\Helper\FilePathHelper; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\QBMapper; use OCP\DB\QueryBuilder\IQueryBuilder; +use OCP\Files\Folder; use OCP\IDBConnection; +use Psr\Log\LoggerInterface; /** * @extends QBMapper @@ -20,10 +23,14 @@ class SubmissionMapper extends QBMapper { * SubmissionMapper constructor. * @param IDBConnection $db * @param AnswerMapper $answerMapper + * @param FilePathHelper $filePathHelper + * @param LoggerInterface $logger */ public function __construct( IDBConnection $db, private AnswerMapper $answerMapper, + private FilePathHelper $filePathHelper, + private LoggerInterface $logger, ) { parent::__construct($db, 'forms_v2_submissions', Submission::class); } @@ -179,22 +186,17 @@ protected function countSubmissionsWithFilters(int $formId, ?string $userId = nu /** * Delete the Submission, including answers. + * @param Form $form Form the submission belongs to. * @param int $id of the submission to delete */ - public function deleteById(int $id): void { - $qb = $this->db->getQueryBuilder(); - - // First delete corresponding answers. + public function deleteById(Form $form, int $id): void { $submissionEntity = $this->findById($id); - $this->answerMapper->deleteBySubmission($submissionEntity->getId()); - //Delete Submission - $qb->delete($this->getTableName()) - ->where( - $qb->expr()->eq('id', $qb->createNamedParameter($id, IQueryBuilder::PARAM_INT)) - ); + $this->deleteSubmissionFolder($form, $submissionEntity->getId()); - $qb->executeStatement(); + $this->answerMapper->deleteBySubmission($submissionEntity->getId()); + + $this->delete($submissionEntity); } /** @@ -218,4 +220,24 @@ public function deleteByForm(int $formId): void { $qb->executeStatement(); } + + /** + * Delete the submission folder from the file system + * @param Form $form The form instance + * @param int $submissionId The submission ID + */ + private function deleteSubmissionFolder(Form $form, int $submissionId): void { + try { + $submissionFolder = $this->filePathHelper->getSubmissionFolder($form, $submissionId); + if ($submissionFolder !== null) { + $submissionFolder->delete(); + } + } catch (\Throwable $e) { + $this->logger->warning('Failed to delete submission folder: {error}', [ + 'error' => $e->getMessage(), + 'submissionId' => $submissionId, + 'formId' => $form->getId(), + ]); + } + } } diff --git a/lib/Helper/FilePathHelper.php b/lib/Helper/FilePathHelper.php new file mode 100644 index 000000000..4a611de7b --- /dev/null +++ b/lib/Helper/FilePathHelper.php @@ -0,0 +1,115 @@ +normalizeFileName($form->getId() . ' - ' . $form->getTitle()), + ]); + } + + /** + * Get the full file path for a specific uploaded file + */ + public function getUploadedFilePath(Form $form, int $submissionId, int $questionId, ?string $questionName, string $questionText): string { + return implode('/', [ + $this->getFormUploadedFilesFolderPath($form), + $submissionId, + $this->normalizeFileName($questionId . ' - ' . ($questionName ?: $questionText)) + ]); + } + + /** + * Get all form folders matching the form ID prefix + * Useful for cleanup operations when form may have been renamed or deleted + * @param int $formId The form ID + * @param string $ownerId The owner user ID + * @return Folder[] + */ + public function getAllFormFoldersById(int $formId, string $ownerId): array { + $formsFolder = $this->getFormsFolder($ownerId); + if ($formsFolder === null) { + return []; + } + + $formFolderPrefix = $formId . ' - '; + $matchingFolders = []; + + // Collect all folders matching the form ID prefix + foreach ($formsFolder->getDirectoryListing() as $node) { + if (str_starts_with($node->getName(), $formFolderPrefix) && $node instanceof Folder) { + $matchingFolders[] = $node; + } + } + + return $matchingFolders; + } + + /** + * Get the forms folder for a user + */ + public function getFormsFolder(string $ownerId): ?Folder { + try { + $userFolder = $this->rootFolder->getUserFolder($ownerId); + $formsFolder = $userFolder->get(Constants::FILES_FOLDER); + + if (!$formsFolder instanceof Folder) { + return null; + } + + return $formsFolder; + } catch (NotFoundException) { + return null; + } + } + + /** + * Get the submission folder for a specific submission + * Searches across all form folders to handle form renames + */ + public function getSubmissionFolder(Form $form, int $submissionId): ?Folder { + $formFolders = $this->getAllFormFoldersById($form->getId(), $form->getOwnerId()); + + // Search for submission folder in all matching form folders + foreach ($formFolders as $formFolder) { + try { + $submissionFolder = $formFolder->get((string)$submissionId); + if ($submissionFolder instanceof Folder) { + return $submissionFolder; + } + } catch (NotFoundException) { + // Continue to next form folder + } + } + + return null; + } +} diff --git a/lib/Service/FormsService.php b/lib/Service/FormsService.php index 239ee94d9..454135faa 100644 --- a/lib/Service/FormsService.php +++ b/lib/Service/FormsService.php @@ -20,6 +20,7 @@ use OCA\Forms\Db\SubmissionMapper; use OCA\Forms\Events\FormSubmittedEvent; use OCA\Forms\Exception\NoSuchFormException; +use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\ResponseDefinitions; use OCP\AppFramework\Db\DoesNotExistException; use OCP\AppFramework\Db\IMapperException; @@ -63,6 +64,7 @@ public function __construct( private IUserManager $userManager, private ISecureRandom $secureRandom, private CirclesService $circlesService, + private FilePathHelper $filePathHelper, private IRootFolder $rootFolder, private IL10N $l10n, private LoggerInterface $logger, @@ -1034,36 +1036,24 @@ public function getFileName(Form $form, string $fileFormat): string { // TRANSLATORS Appendix for CSV-Export: 'Form Title (responses).csv' $fileName = $form->getTitle() . ' (' . $this->l10n->t('responses') . ').' . $fileFormat; - return self::normalizeFileName($fileName); + return $this->filePathHelper->normalizeFileName($fileName); } public function getFormUploadedFilesFolderPath(Form $form): string { - return implode('/', [ - Constants::FILES_FOLDER, - self::normalizeFileName($form->getId() . ' - ' . $form->getTitle()), - ]); + return $this->filePathHelper->getFormUploadedFilesFolderPath($form); } public function getUploadedFilePath(Form $form, int $submissionId, int $questionId, ?string $questionName, string $questionText): string { - - return implode('/', [ - $this->getFormUploadedFilesFolderPath($form), - $submissionId, - self::normalizeFileName($questionId . ' - ' . ($questionName ?: $questionText)) - ]); + return $this->filePathHelper->getUploadedFilePath($form, $submissionId, $questionId, $questionName, $questionText); } public function getTemporaryUploadedFilePath(Form $form, Question $question): string { return implode('/', [ Constants::UNSUBMITTED_FILES_FOLDER, microtime(true), - self::normalizeFileName($form->getId() . ' - ' . $form->getTitle()), - self::normalizeFileName($question->getId() . ' - ' . ($question->getName() ?: $question->getText())) + $this->filePathHelper->normalizeFileName($form->getId() . ' - ' . $form->getTitle()), + $this->filePathHelper->normalizeFileName($question->getId() . ' - ' . ($question->getName() ?: $question->getText())) ]); } - private static function normalizeFileName(string $fileName): string { - return trim(str_replace(Constants::FILENAME_INVALID_CHARS, '-', $fileName)); - } - } diff --git a/tests/Integration/Api/ApiV3Test.php b/tests/Integration/Api/ApiV3Test.php index ad900ea6d..6f8f2bb9c 100644 --- a/tests/Integration/Api/ApiV3Test.php +++ b/tests/Integration/Api/ApiV3Test.php @@ -717,6 +717,64 @@ public function testDeleteForm() { $this->assertEquals(404, $resp->getStatusCode()); } + public function testDeleteFormWithSubmissions() { + // Create a new form with a file question + $resp = $this->http->request('POST', 'api/v3/forms'); + $formData = $this->OcsResponse2Data($resp); + $formId = $formData['id']; + + // Add a file question + $resp = $this->http->request('POST', "api/v3/forms/$formId/questions", [ + 'json' => [ + 'type' => 'file', + 'text' => 'Upload a file', + ] + ]); + $questionData = $this->OcsResponse2Data($resp); + $questionId = $questionData['id']; + + // Upload a file + $uploadedFileResponse = $this->http->request('POST', + "api/v3/forms/$formId/submissions/files/$questionId", + [ + 'multipart' => [ + [ + 'name' => 'files[]', + 'contents' => 'test file content for form deletion', + 'filename' => 'test-form-delete.txt' + ] + ] + ]); + + $uploadedFileData = $this->OcsResponse2Data($uploadedFileResponse); + $uploadedFileId = $uploadedFileData[0]['uploadedFileId']; + + // Create a submission with the file + $resp = $this->http->request('POST', "api/v3/forms/$formId/submissions", [ + 'json' => [ + 'answers' => [ + $questionId => [['uploadedFileId' => $uploadedFileId]] + ] + ] + ]); + $this->assertEquals(201, $resp->getStatusCode()); + + // Delete the form + $resp = $this->http->request('DELETE', "api/v3/forms/$formId"); + $deletedId = $this->OcsResponse2Data($resp); + + $this->assertEquals(200, $resp->getStatusCode()); + $this->assertEquals($formId, $deletedId); + + // Verify form is deleted + try { + $this->http->request('GET', "api/v3/forms/$formId"); + } catch (ClientException $e) { + $resp = $e->getResponse(); + } + $this->assertEquals(404, $resp->getStatusCode()); + } + public static function dataCreateNewQuestion() { return [ 'newQuestion' => [ @@ -894,6 +952,25 @@ public function testDeleteQuestion(array $fullFormExpected) { $this->testGetFullForm($fullFormExpected); } + public function testDeleteFileQuestion() { + // Get the file question (index 2 in testForms[0]) + $fileQuestionId = $this->testForms[0]['questions'][2]['id']; + + // Delete the file question + $resp = $this->http->request('DELETE', "api/v3/forms/{$this->testForms[0]['id']}/questions/$fileQuestionId"); + $data = $this->OcsResponse2Data($resp); + + $this->assertEquals(200, $resp->getStatusCode()); + $this->assertEquals($fileQuestionId, $data); + + // Verify the question is deleted from the form + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}"); + $formData = $this->OcsResponse2Data($resp); + + $questionIds = array_map(fn ($q) => $q['id'], $formData['questions']); + $this->assertNotContains($fileQuestionId, $questionIds); + } + public function testCloneQuestion() { $resp = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/questions?fromId=" . $this->testForms[0]['questions'][0]['id']); $data = $this->OcsResponse2Data($resp); @@ -1254,6 +1331,81 @@ public function testExportSubmissions(string $expected) { $this->assertEquals($arr_txt_expected, $arr_txt_data); } + public function testDeleteSubmission() { + // Get submissions first to find a submission ID + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}/submissions"); + $data = $this->OcsResponse2Data($resp); + + $this->assertNotEmpty($data['submissions']); + $submissionId = $data['submissions'][0]['id']; + + // Delete the submission + $resp = $this->http->request('DELETE', "api/v3/forms/{$this->testForms[0]['id']}/submissions/$submissionId"); + $deletedId = $this->OcsResponse2Data($resp); + + $this->assertEquals(200, $resp->getStatusCode()); + $this->assertEquals($submissionId, $deletedId); + + // Verify submission is deleted + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}/submissions"); + $data = $this->OcsResponse2Data($resp); + + $this->assertCount(2, $data['submissions']); // Originally 3, now 2 + $submissionIds = array_map(fn ($s) => $s['id'], $data['submissions']); + $this->assertNotContains($submissionId, $submissionIds); + } + + public function testDeleteSubmissionWithFiles() { + // Upload a file for the file question + $uploadedFileResponse = $this->http->request('POST', + "api/v3/forms/{$this->testForms[0]['id']}/submissions/files/{$this->testForms[0]['questions'][2]['id']}", + [ + 'multipart' => [ + [ + 'name' => 'files[]', + 'contents' => 'test file content', + 'filename' => 'test-delete.txt' + ] + ] + ]); + + $uploadedFileData = $this->OcsResponse2Data($uploadedFileResponse); + $uploadedFileId = $uploadedFileData[0]['uploadedFileId']; + + // Create a new submission with the file + $resp = $this->http->request('POST', "api/v3/forms/{$this->testForms[0]['id']}/submissions", [ + 'json' => [ + 'answers' => [ + $this->testForms[0]['questions'][0]['id'] => ['Test Answer'], + $this->testForms[0]['questions'][2]['id'] => [['uploadedFileId' => $uploadedFileId]] + ] + ] + ]); + + $this->assertEquals(201, $resp->getStatusCode()); + + // Get the submission ID from the submissions list (newest submission is first) + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}/submissions"); + $data = $this->OcsResponse2Data($resp); + $this->assertNotEmpty($data['submissions']); + $newSubmissionId = $data['submissions'][0]['id']; + $this->assertNotEmpty($newSubmissionId); + + // Delete the submission + $resp = $this->http->request('DELETE', "api/v3/forms/{$this->testForms[0]['id']}/submissions/$newSubmissionId"); + $deletedId = $this->OcsResponse2Data($resp); + + $this->assertEquals(200, $resp->getStatusCode()); + $this->assertEquals($newSubmissionId, $deletedId); + + // Verify submission is deleted + $resp = $this->http->request('GET', "api/v3/forms/{$this->testForms[0]['id']}/submissions"); + $data = $this->OcsResponse2Data($resp); + + $submissionIds = array_map(fn ($s) => $s['id'], $data['submissions']); + $this->assertNotContains($newSubmissionId, $submissionIds); + } + public function testLinkFile() { $resp = $this->http->request('PATCH', "api/v3/forms/{$this->testForms[0]['id']}", [ 'json' => [ diff --git a/tests/Integration/BackgroundJob/DeleteQuestionFoldersJobTest.php b/tests/Integration/BackgroundJob/DeleteQuestionFoldersJobTest.php new file mode 100644 index 000000000..061af3d9c --- /dev/null +++ b/tests/Integration/BackgroundJob/DeleteQuestionFoldersJobTest.php @@ -0,0 +1,232 @@ + 'Test User', + ]; + + public function setUp(): void { + parent::setUp(); + + $this->rootFolder = \OCP\Server::get(IRootFolder::class); + $filePathHelper = \OCP\Server::get(\OCA\Forms\Helper\FilePathHelper::class); + $logger = \OCP\Server::get(\Psr\Log\LoggerInterface::class); + $timeFactory = \OCP\Server::get(\OCP\AppFramework\Utility\ITimeFactory::class); + + $this->job = new DeleteQuestionFoldersJob( + $timeFactory, + $filePathHelper, + $logger, + ); + } + + public function tearDown(): void { + // Clean up any created folders + $this->cleanupTestFolders(); + + parent::tearDown(); + } + + private function cleanupTestFolders(): void { + try { + $userFolder = $this->rootFolder->getUserFolder($this->testUserId); + $formsFolder = $userFolder->get(Constants::FILES_FOLDER); + if ($formsFolder instanceof \OCP\Files\Folder) { + $formsFolder->delete(); + } + } catch (NotFoundException $e) { + // Folder doesn't exist, nothing to clean up + } + } + + private function setupTestFolders(int $formId, int $questionId, int $submissionId, string $questionName): void { + $userFolder = $this->rootFolder->getUserFolder($this->testUserId); + + // Create Forms folder + $formsFolder = $userFolder->newFolder(Constants::FILES_FOLDER); + + // Create form folder + $formFolderName = $formId . ' - Test Form'; + $formFolder = $formsFolder->newFolder($formFolderName); + + // Create submission folder + $submissionFolder = $formFolder->newFolder((string)$submissionId); + + // Create question folder + $questionFolderName = $questionId . ' - ' . $questionName; + $submissionFolder->newFolder($questionFolderName); + } + + private function setupMultipleFormFolders(int $formId, int $questionId, array $submissionIds, string $questionName): void { + $userFolder = $this->rootFolder->getUserFolder($this->testUserId); + + // Create Forms folder + $formsFolder = $userFolder->newFolder(Constants::FILES_FOLDER); + + // Create multiple form folders (simulating form renames) + $formFolderNames = [ + $formId . ' - Test Form', + $formId . ' - Renamed Form', + ]; + + foreach ($formFolderNames as $formFolderName) { + $formFolder = $formsFolder->newFolder($formFolderName); + + foreach ($submissionIds as $submissionId) { + $submissionFolder = $formFolder->newFolder((string)$submissionId); + $questionFolderName = $questionId . ' - ' . $questionName; + $submissionFolder->newFolder($questionFolderName); + } + } + } + + private function folderExists(string $path): bool { + try { + $userFolder = $this->rootFolder->getUserFolder($this->testUserId); + $userFolder->get($path); + return true; + } catch (NotFoundException) { + return false; + } + } + + public function testRunDeletesQuestionFolders(): void { + $formId = 1; + $questionId = 42; + $submissionId = 100; + $questionName = 'Test Question'; + + // Setup test folders + $this->setupTestFolders($formId, $questionId, $submissionId, $questionName); + + // Verify folder exists before running job + $questionFolderPath = Constants::FILES_FOLDER . '/' . $formId . ' - Test Form/' . $submissionId . '/' . $questionId . ' - ' . $questionName; + $this->assertTrue($this->folderExists($questionFolderPath), 'Question folder should exist before job runs'); + + // Run the job + $this->job->run([ + 'formId' => $formId, + 'questionId' => $questionId, + 'ownerId' => $this->testUserId, + ]); + + // Verify folder is deleted + $this->assertFalse($this->folderExists($questionFolderPath), 'Question folder should be deleted after job runs'); + } + + public function testRunDeletesFromMultipleFormFolders(): void { + $formId = 1; + $questionId = 42; + $submissionIds = [100, 101]; + $questionName = 'Test Question'; + + // Setup multiple form folders with submissions + $this->setupMultipleFormFolders($formId, $questionId, $submissionIds, $questionName); + + // Verify folders exist before running job + $formFolderNames = [$formId . ' - Test Form', $formId . ' - Renamed Form']; + foreach ($formFolderNames as $formFolderName) { + foreach ($submissionIds as $submissionId) { + $questionFolderPath = Constants::FILES_FOLDER . '/' . $formFolderName . '/' . $submissionId . '/' . $questionId . ' - ' . $questionName; + $this->assertTrue($this->folderExists($questionFolderPath), "Question folder should exist before job runs: $questionFolderPath"); + } + } + + // Run the job + $this->job->run([ + 'formId' => $formId, + 'questionId' => $questionId, + 'ownerId' => $this->testUserId, + ]); + + // Verify all question folders are deleted + foreach ($formFolderNames as $formFolderName) { + foreach ($submissionIds as $submissionId) { + $questionFolderPath = Constants::FILES_FOLDER . '/' . $formFolderName . '/' . $submissionId . '/' . $questionId . ' - ' . $questionName; + $this->assertFalse($this->folderExists($questionFolderPath), "Question folder should be deleted after job runs: $questionFolderPath"); + } + } + } + + public function testRunOnlyDeletesMatchingQuestionFolders(): void { + $formId = 1; + $questionId = 42; + $otherQuestionId = 43; + $submissionId = 100; + + $userFolder = $this->rootFolder->getUserFolder($this->testUserId); + $formsFolder = $userFolder->newFolder(Constants::FILES_FOLDER); + $formFolder = $formsFolder->newFolder($formId . ' - Test Form'); + $submissionFolder = $formFolder->newFolder((string)$submissionId); + + // Create two question folders + $questionFolder1 = $submissionFolder->newFolder($questionId . ' - Question 1'); + $questionFolder2 = $submissionFolder->newFolder($otherQuestionId . ' - Question 2'); + + // Run the job to delete only questionId 42 + $this->job->run([ + 'formId' => $formId, + 'questionId' => $questionId, + 'ownerId' => $this->testUserId, + ]); + + // Verify questionId 42 folder is deleted + $questionPath1 = Constants::FILES_FOLDER . '/' . $formId . ' - Test Form/' . $submissionId . '/' . $questionId . ' - Question 1'; + $this->assertFalse($this->folderExists($questionPath1), 'Question folder 42 should be deleted'); + + // Verify other question folder still exists + $questionPath2 = Constants::FILES_FOLDER . '/' . $formId . ' - Test Form/' . $submissionId . '/' . $otherQuestionId . ' - Question 2'; + $this->assertTrue($this->folderExists($questionPath2), 'Question folder 43 should still exist'); + } + + public function testRunHandlesNonExistentFormFolder(): void { + $formId = 999; + $questionId = 42; + + // Don't create any folders - job should handle gracefully + $this->job->run([ + 'formId' => $formId, + 'questionId' => $questionId, + 'ownerId' => $this->testUserId, + ]); + + // Should not throw exception + $this->assertTrue(true, 'Job should handle non-existent form folder gracefully'); + } + + public function testRunHandlesNonExistentFormsFolder(): void { + $formId = 1; + $questionId = 42; + + // Don't create Forms folder - job should handle gracefully + $this->job->run([ + 'formId' => $formId, + 'questionId' => $questionId, + 'ownerId' => $this->testUserId, + ]); + + // Should not throw exception + $this->assertTrue(true, 'Job should handle non-existent Forms folder gracefully'); + } +} diff --git a/tests/Integration/DB/SubmissionMapperTest.php b/tests/Integration/DB/SubmissionMapperTest.php index 1ad6f1248..876b88a00 100644 --- a/tests/Integration/DB/SubmissionMapperTest.php +++ b/tests/Integration/DB/SubmissionMapperTest.php @@ -121,7 +121,9 @@ public function setUp(): void { $db = \OCP\Server::get(IDBConnection::class); $answerMapper = \OCP\Server::get(\OCA\Forms\Db\AnswerMapper::class); - $this->submissionMapper = new SubmissionMapper($db, $answerMapper); + $filePathHelper = \OCP\Server::get(\OCA\Forms\Helper\FilePathHelper::class); + $logger = \OCP\Server::get(\Psr\Log\LoggerInterface::class); + $this->submissionMapper = new SubmissionMapper($db, $answerMapper, $filePathHelper, $logger); } public function testFindByFormBasic(): void { @@ -183,4 +185,30 @@ public function testCountSubmissionsEmptyForm(): void { $this->assertEquals(0, $count); } + + public function testDeleteById(): void { + // Get the first submission from the form + $submissions = $this->submissionMapper->findByForm($this->testForms[0]['id']); + $this->assertCount(3, $submissions); + + $submissionToDelete = $submissions[0]; + $submissionId = $submissionToDelete->getId(); + + // Get the form + $form = \OCA\Forms\Db\Form::fromParams([ + 'id' => $this->testForms[0]['id'], + 'ownerId' => 'test', + ]); + + // Delete the submission + $this->submissionMapper->deleteById($form, $submissionId); + + // Verify the submission is deleted + $remainingSubmissions = $this->submissionMapper->findByForm($this->testForms[0]['id']); + $this->assertCount(2, $remainingSubmissions); + + // Verify the deleted submission is not in the remaining list + $remainingIds = array_map(fn ($s) => $s->getId(), $remainingSubmissions); + $this->assertNotContains($submissionId, $remainingIds); + } } diff --git a/tests/Unit/Controller/ApiControllerTest.php b/tests/Unit/Controller/ApiControllerTest.php index e0b01d9e2..3ae1dd6ec 100644 --- a/tests/Unit/Controller/ApiControllerTest.php +++ b/tests/Unit/Controller/ApiControllerTest.php @@ -988,7 +988,7 @@ public function testDeleteSubmission($submissionData, $formData) { $this->submissionMapper ->expects($this->once()) ->method('deleteById') - ->with(42); + ->with($form, 42); $this->assertEquals(new DataResponse(42), $this->apiController->deleteSubmission(1, 42)); } diff --git a/tests/Unit/Controller/ShareApiControllerTest.php b/tests/Unit/Controller/ShareApiControllerTest.php index bb37ee523..6ba55602b 100644 --- a/tests/Unit/Controller/ShareApiControllerTest.php +++ b/tests/Unit/Controller/ShareApiControllerTest.php @@ -16,6 +16,7 @@ use OCA\Forms\Db\Share; use OCA\Forms\Db\ShareMapper; use OCA\Forms\Exception\NoSuchFormException; +use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\FormsService; @@ -59,7 +60,8 @@ class ShareApiControllerTest extends TestCase { private IUserManager|MockObject $userManager; private ISecureRandom|MockObject $secureRandom; private CirclesService|MockObject $circlesService; - private IRootFolder|MockObject $storage; + private FilePathHelper $filePathHelper; + private IRootFolder|MockObject $rootFolder; private IManager|MockObject $shareManager; public function setUp(): void { @@ -83,7 +85,8 @@ public function setUp(): void { ->method('getUser') ->willReturn($user); - $this->storage = $this->createMock(IRootFolder::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->filePathHelper = new FilePathHelper($this->rootFolder); $this->shareManager = $this->createMock(IManager::class); $this->shareApiController = new ShareApiController( @@ -99,7 +102,8 @@ public function setUp(): void { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->rootFolder, + $this->filePathHelper, $this->shareManager, ); } @@ -541,14 +545,12 @@ public function testDeleteShare_cleansUpFileShare(): void { $userFolder->expects($this->once()) ->method('get') ->willReturn($folder); - $this->storage->expects($this->once()) + + // Set expectations on the rootFolder + $this->rootFolder->expects($this->once()) ->method('getUserFolder') ->with('currentUser') ->willReturn($userFolder); - $this->formsService->expects($this->once()) - ->method('getFormUploadedFilesFolderPath') - ->with($form) - ->willReturn('Forms/my-form'); // Mock finding and deleting the matching Files share $fileShare = $this->createMock(IShare::class); @@ -594,7 +596,6 @@ public function testDeleteShare_noResultsPermission_skipsFileShareCleanup(): voi ->willReturn($form); // No file share interactions expected - $this->storage->expects($this->never())->method('getUserFolder'); $this->shareManager->expects($this->never())->method('getSharesBy'); $this->shareManager->expects($this->never())->method('deleteShare'); @@ -848,7 +849,8 @@ public function testUpdateShare(array $share, string $formOwner, array $keyValue ->method('get') ->willReturn($folder); - $this->storage->expects($this->any()) + // Set expectations on the rootFolder + $this->rootFolder->expects($this->any()) ->method('getUserFolder') ->with('currentUser') ->willReturn($userFolder); diff --git a/tests/Unit/Helper/FilePathHelperTest.php b/tests/Unit/Helper/FilePathHelperTest.php new file mode 100644 index 000000000..8909d1870 --- /dev/null +++ b/tests/Unit/Helper/FilePathHelperTest.php @@ -0,0 +1,281 @@ +rootFolder = $this->createMock(IRootFolder::class); + $this->filePathHelper = new FilePathHelper($this->rootFolder); + } + + public function testNormalizeFileName() { + $this->assertEquals('test-file-name', $this->filePathHelper->normalizeFileName('test/file/name')); + $this->assertEquals('test-file-name', $this->filePathHelper->normalizeFileName('test\file\name')); + $this->assertEquals('test file name', $this->filePathHelper->normalizeFileName('test file name')); + $this->assertEquals('test', $this->filePathHelper->normalizeFileName(' test ')); + } + + public function testGetFormUploadedFilesFolderPath() { + $form = new Form(); + $form->setId(42); + $form->setTitle('My Form'); + + $expected = implode('/', [ + Constants::FILES_FOLDER, + '42 - My Form', + ]); + + $this->assertEquals($expected, $this->filePathHelper->getFormUploadedFilesFolderPath($form)); + } + + public function testGetUploadedFilePath() { + $form = new Form(); + $form->setId(42); + $form->setTitle('My Form'); + + $expected = implode('/', [ + implode('/', [ + Constants::FILES_FOLDER, + '42 - My Form', + ]), + 123, + '10 - Question Name', + ]); + + $this->assertEquals($expected, $this->filePathHelper->getUploadedFilePath($form, 123, 10, 'Question Name', 'Question Text')); + } + + public function testGetUploadedFilePathWithoutQuestionName() { + $form = new Form(); + $form->setId(42); + $form->setTitle('My Form'); + + $expected = implode('/', [ + implode('/', [ + Constants::FILES_FOLDER, + '42 - My Form', + ]), + 123, + '10 - Question Text', + ]); + + $this->assertEquals($expected, $this->filePathHelper->getUploadedFilePath($form, 123, 10, null, 'Question Text')); + } + + public function testGetFormsFolderReturnsFolder() { + $userFolder = $this->createMock(Folder::class); + $formsFolder = $this->createMock(Folder::class); + + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($formsFolder); + + $result = $this->filePathHelper->getFormsFolder('user1'); + $this->assertSame($formsFolder, $result); + } + + public function testGetFormsFolderReturnsNullWhenNotFound() { + $userFolder = $this->createMock(Folder::class); + + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willThrowException(new NotFoundException()); + + $result = $this->filePathHelper->getFormsFolder('user1'); + $this->assertNull($result); + } + + public function testGetFormsFolderReturnsNullWhenNotFolder() { + $userFolder = $this->createMock(Folder::class); + $notAFolder = $this->createMock(\OCP\Files\File::class); + + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($notAFolder); + + $result = $this->filePathHelper->getFormsFolder('user1'); + $this->assertNull($result); + } + + public function testGetAllFormFoldersById() { + $formsFolder = $this->createMock(Folder::class); + $matchingFolder1 = $this->createMock(Folder::class); + $matchingFolder2 = $this->createMock(Folder::class); + $nonMatchingFolder = $this->createMock(Folder::class); + + $matchingFolder1->method('getName')->willReturn('42 - Form Title'); + $matchingFolder2->method('getName')->willReturn('42 - Another Title'); + $nonMatchingFolder->method('getName')->willReturn('43 - Different Form'); + + $formsFolder->method('getDirectoryListing')->willReturn([ + $matchingFolder1, + $matchingFolder2, + $nonMatchingFolder, + ]); + + // Mock getFormsFolder to return our formsFolder + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($formsFolder); + + $result = $this->filePathHelper->getAllFormFoldersById(42, 'user1'); + $this->assertCount(2, $result); + $this->assertContains($matchingFolder1, $result); + $this->assertContains($matchingFolder2, $result); + $this->assertNotContains($nonMatchingFolder, $result); + } + + public function testGetAllFormFoldersByIdReturnsEmptyWhenFormsFolderNull() { + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willThrowException(new NotFoundException()); + + $result = $this->filePathHelper->getAllFormFoldersById(42, 'user1'); + $this->assertEmpty($result); + } + + public function testGetSubmissionFolder() { + $form = new Form(); + $form->setId(42); + $form->setOwnerId('user1'); + + $formsFolder = $this->createMock(Folder::class); + $formFolder = $this->createMock(Folder::class); + $submissionFolder = $this->createMock(Folder::class); + + $formFolder->method('getName')->willReturn('42 - Form Title'); + $formsFolder->method('getDirectoryListing')->willReturn([$formFolder]); + + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($formsFolder); + + $formFolder->expects($this->once()) + ->method('get') + ->with('123') + ->willReturn($submissionFolder); + + $result = $this->filePathHelper->getSubmissionFolder($form, 123); + $this->assertSame($submissionFolder, $result); + } + + public function testGetSubmissionFolderReturnsNullWhenNotFound() { + $form = new Form(); + $form->setId(42); + $form->setOwnerId('user1'); + + $formsFolder = $this->createMock(Folder::class); + $formFolder = $this->createMock(Folder::class); + + $formFolder->method('getName')->willReturn('42 - Form Title'); + $formsFolder->method('getDirectoryListing')->willReturn([$formFolder]); + + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($formsFolder); + + $formFolder->expects($this->once()) + ->method('get') + ->with('123') + ->willThrowException(new NotFoundException()); + + $result = $this->filePathHelper->getSubmissionFolder($form, 123); + $this->assertNull($result); + } + + public function testGetSubmissionFolderReturnsNullWhenNotFolder() { + $form = new Form(); + $form->setId(42); + $form->setOwnerId('user1'); + + $formsFolder = $this->createMock(Folder::class); + $formFolder = $this->createMock(Folder::class); + $notAFolder = $this->createMock(\OCP\Files\File::class); + + $formFolder->method('getName')->willReturn('42 - Form Title'); + $formsFolder->method('getDirectoryListing')->willReturn([$formFolder]); + + $userFolder = $this->createMock(Folder::class); + $this->rootFolder->expects($this->once()) + ->method('getUserFolder') + ->with('user1') + ->willReturn($userFolder); + + $userFolder->expects($this->once()) + ->method('get') + ->with(Constants::FILES_FOLDER) + ->willReturn($formsFolder); + + $formFolder->expects($this->once()) + ->method('get') + ->with('123') + ->willReturn($notAFolder); + + $result = $this->filePathHelper->getSubmissionFolder($form, 123); + $this->assertNull($result); + } +} diff --git a/tests/Unit/Service/FormsServiceTest.php b/tests/Unit/Service/FormsServiceTest.php index 33d6762d2..3aa911ea3 100644 --- a/tests/Unit/Service/FormsServiceTest.php +++ b/tests/Unit/Service/FormsServiceTest.php @@ -41,6 +41,7 @@ function microtime(bool|float $asFloat = false) { use OCA\Forms\Db\ShareMapper; use OCA\Forms\Db\Submission; use OCA\Forms\Db\SubmissionMapper; +use OCA\Forms\Helper\FilePathHelper; use OCA\Forms\Service\CirclesService; use OCA\Forms\Service\ConfigService; use OCA\Forms\Service\ConfirmationEmailService; @@ -77,7 +78,8 @@ class FormsServiceTest extends TestCase { private IUserManager|MockObject $userManager; private ISecureRandom|MockObject $secureRandom; private CirclesService|MockObject $circlesService; - private IRootFolder|MockObject $storage; + private FilePathHelper|MockObject $filePathHelper; + private IRootFolder|MockObject $rootFolder; private IL10N|MockObject $l10n; private LoggerInterface|MockObject $logger; @@ -109,7 +111,8 @@ public function setUp(): void { ->method('getUser') ->willReturn($user); - $this->storage = $this->createMock(IRootFolder::class); + $this->rootFolder = $this->createMock(IRootFolder::class); + $this->filePathHelper = new FilePathHelper($this->rootFolder); $this->l10n = $this->createMock(IL10N::class); $this->l10n->expects($this->any()) @@ -129,7 +132,8 @@ public function setUp(): void { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, \OCP\Server::get(IEventDispatcher::class), @@ -153,7 +157,8 @@ private function createFormsServiceWithEventDispatcher(IEventDispatcher $eventDi $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, $eventDispatcher, @@ -645,7 +650,8 @@ public function testGetPermissions_NotLoggedIn() { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, \OCP\Server::get(IEventDispatcher::class), @@ -870,7 +876,8 @@ public function testPublicCanSubmit() { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, \OCP\Server::get(IEventDispatcher::class), @@ -978,7 +985,8 @@ public function testHasUserAccess_NotLoggedIn() { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, \OCP\Server::get(IEventDispatcher::class), @@ -1238,7 +1246,8 @@ public function testNotifyNewSubmission($shares, $shareNotifications) { $this->userManager, $this->secureRandom, $this->circlesService, - $this->storage, + $this->filePathHelper, + $this->rootFolder, $this->l10n, $this->logger, $eventDispatcher, @@ -1530,7 +1539,7 @@ public function testGetFilePathThrowsAnException() { ->with(100) ->willReturn([]); - $this->storage->expects($this->once()) + $this->rootFolder->expects($this->once()) ->method('getUserFolder') ->with('user1') ->willReturn($folder);