Repository navigation
feat: allow to specify opening date #3810
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -178,6 +178,7 @@ public function newForm(?int $fromId = null): DataResponse { | |
| $form->setAllowEditSubmissions(false); | ||
| $form->setShowExpiration(false); | ||
| $form->setExpires(0); | ||
| $form->setOpens(0); | ||
| $form->setIsAnonymous(false); | ||
|
|
||
| $this->formMapper->insert($form); | ||
|
|
@@ -212,6 +213,7 @@ public function newForm(?int $fromId = null): DataResponse { | |
| $formData['allowEditSubmissions'] = false; | ||
| $formData['showExpiration'] = false; | ||
| $formData['expires'] = 0; | ||
| $formData['opens'] = 0; | ||
| $formData['isAnonymous'] = false; | ||
|
|
||
| $form = Form::fromParams($formData); | ||
|
|
@@ -282,6 +284,7 @@ public function getForm(int $formId): DataResponse { | |
| * @param int $formId FormId of form to update | ||
| * @param array<string, mixed> $keyValuePairs Array of key=>value pairs to update. | ||
| * @return DataResponse<Http::STATUS_OK, int|string, array{}> | ||
| * @throws OCSBadRequestException Invalid opening date or opening date not before expiration date | ||
| * @throws OCSBadRequestException Could not find new form owner | ||
| * @throws OCSForbiddenException Empty keyValuePairs provided | ||
| * @throws OCSForbiddenException Not allowed to update id, hash, created, fileId or lastUpdated. OwnerId only allowed if no other key provided. | ||
|
|
@@ -336,6 +339,8 @@ public function updateForm(int $formId, array $keyValuePairs): DataResponse { | |
| // Do not allow changing showToAllUsers or permitAllUsers if disabled | ||
| $this->checkAccessUpdate($keyValuePairs); | ||
|
|
||
| $this->checkOpeningDateUpdate($form, $keyValuePairs); | ||
|
|
||
| $previousAccess = $form->getAccess(); | ||
| $previousState = $form->getState(); | ||
| $previousSettings = $this->getResponseSettings($form); | ||
|
|
@@ -1950,6 +1955,28 @@ private function checkForbiddenKeys(array $keyValuePairs): void { | |
| } | ||
| } | ||
|
|
||
| /** | ||
| * Throws if the opening date is invalid or not before the expiration date | ||
| */ | ||
| private function checkOpeningDateUpdate(Form $form, array $keyValuePairs): void { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need the same check for expiration date? |
||
| if (!array_key_exists('opens', $keyValuePairs) && !array_key_exists('expires', $keyValuePairs)) { | ||
| return; | ||
| } | ||
|
|
||
| if (array_key_exists('opens', $keyValuePairs) | ||
| && (!is_int($keyValuePairs['opens']) || $keyValuePairs['opens'] < 0)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The validation is lopsided.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Partial correction after a live check (test instance built from
The point that stands is the asymmetry: |
||
| $this->logger->info('Invalid opening date'); | ||
| throw new OCSBadRequestException('Invalid opening date'); | ||
| } | ||
|
|
||
| $opens = $keyValuePairs['opens'] ?? $form->getOpens(); | ||
| $expires = $keyValuePairs['expires'] ?? $form->getExpires(); | ||
| if ($opens > 0 && $expires > 0 && $opens >= $expires) { | ||
| $this->logger->info('Opening date must be before expiration date'); | ||
| throw new OCSBadRequestException('Opening date must be before expiration date'); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Throws if fileId is present in update | ||
| */ | ||
|
|
@@ -2103,6 +2130,7 @@ private function getResponseSettings(Form $form): array { | |
| 'submitMultiple' => $form->getSubmitMultiple(), | ||
| 'allowEditSubmissions' => $form->getAllowEditSubmissions(), | ||
| 'expires' => $form->getExpires(), | ||
| 'opens' => $form->getOpens(), | ||
| 'maxSubmissions' => $form->getMaxSubmissions(), | ||
| ]; | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -203,6 +203,11 @@ protected function createPublicSubmitView(Form $form, string $hash): TemplateRes | |
| return $this->provideEmptyContent(Constants::EMPTY_EXPIRED, $form); | ||
| } | ||
|
|
||
| if (!$this->formsService->hasFormOpened($form)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This check only protects the rendered page (and Scenario: an owner shares a quiz or exam form directly with users and sets it to open tomorrow. A shared user calls If keeping questions confidential until the opening date is part of the feature's promise, |
||
| $this->initialState->provideInitialState('opens', $form->getOpens()); | ||
| return $this->provideEmptyContent(Constants::EMPTY_NOTYETOPEN, $form); | ||
| } | ||
|
|
||
| $this->insertHeaderOnIos(); | ||
|
|
||
| // Inject style on all templates | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -144,6 +144,7 @@ public function import(IUser $user, IImportSource $importSource, OutputInterface | |
| $form->setOwnerId($user->getUID()); | ||
| $form->setAccess($formData['access']); | ||
| $form->setExpires($formData['expires']); | ||
| $form->setOpens($formData['opens'] ?? 0); | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The new invariant ( |
||
| $form->setIsAnonymous($formData['isAnonymous']); | ||
| $form->setSubmitMultiple($formData['submitMultiple']); | ||
| $form->setAllowEditSubmissions($formData['allowEditSubmissions']); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,41 @@ | ||
| <?php | ||
|
|
||
| declare(strict_types=1); | ||
|
|
||
| /** | ||
| * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors | ||
| * SPDX-License-Identifier: AGPL-3.0-or-later | ||
| */ | ||
|
|
||
| namespace OCA\Forms\Migration; | ||
|
|
||
| use Closure; | ||
| use OCP\DB\ISchemaWrapper; | ||
| use OCP\DB\Types; | ||
| use OCP\Migration\Attributes\AddColumn; | ||
| use OCP\Migration\Attributes\ColumnType; | ||
| use OCP\Migration\IOutput; | ||
| use OCP\Migration\SimpleMigrationStep; | ||
| use Override; | ||
|
|
||
| #[AddColumn(table: 'forms_v2_forms', name: 'opens', type: ColumnType::INTEGER, description: 'Timestamp from which a form accepts responses')] | ||
| class Version050500Date20261030000000 extends SimpleMigrationStep { | ||
| #[Override] | ||
| public function changeSchema(IOutput $output, Closure $schemaClosure, array $options): ?ISchemaWrapper { | ||
| /** @var ISchemaWrapper $schema */ | ||
| $schema = $schemaClosure(); | ||
| $table = $schema->getTable('forms_v2_forms'); | ||
|
|
||
| if ($table->hasColumn('opens')) { | ||
| return null; | ||
| } | ||
|
|
||
| $table->addColumn('opens', Types::INTEGER, [ | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I checked this on a running instance (MariaDB 10.6). It is worse than a silent clamp: |
||
| 'notnull' => true, | ||
| 'default' => 0, | ||
| 'comment' => 'Timestamp from which the form accepts responses, 0 means immediately', | ||
| ]); | ||
|
|
||
| return $schema; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -266,6 +266,7 @@ public function getPartialFormArrays(array $forms): array { | |
| 'hash' => $form->getHash(), | ||
| 'title' => $form->getTitle(), | ||
| 'expires' => $form->getExpires(), | ||
| 'opens' => $form->getOpens(), | ||
| 'lastUpdated' => $form->getLastUpdated(), | ||
| 'permissions' => $permissions, | ||
| 'partial' => true, | ||
|
|
@@ -407,6 +408,10 @@ public function loadFormForSubmission(int $formId, string $shareHash): Form { | |
| throw new OCSForbiddenException('This form is no longer taking answers'); | ||
| } | ||
|
|
||
| if (!$this->hasFormOpened($form)) { | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Scenario: a live form with That may be intended ("paused means paused"), but it is a behaviour change for existing respondents and isn't covered by a test either way. If it's not intended, the check should apply only to new submissions (and their uploads), not to |
||
| throw new OCSForbiddenException('This form is not yet taking answers'); | ||
| } | ||
|
|
||
| return $form; | ||
| } | ||
|
|
||
|
|
@@ -671,6 +676,13 @@ public function hasFormExpired(Form $form): bool { | |
| return ($form->getExpires() !== 0 && $form->getExpires() < time()); | ||
| } | ||
|
|
||
| /** | ||
| * Check if the opening date of the form has been reached. | ||
| */ | ||
| public function hasFormOpened(Form $form): bool { | ||
| return $form->getOpens() <= time(); | ||
| } | ||
|
|
||
| /** | ||
| * Get DisplayNames to Shares | ||
| * | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.