From 2fd85da119a8ef5bf98c7b3db09d781fb7ccd7ff Mon Sep 17 00:00:00 2001 From: tombrain Date: Fri, 30 Oct 2026 23:10:46 +0100 Subject: [PATCH 1/7] feat: {id} in the filename of uploadField() is replaced by the id of the record --- CHANGELOG.md | 14 + README.md | 41 +++ src/class.FormHandler.php | 26 +- src/class.dbFormHandler.php | 154 ++++++++++- src/fields/class.UploadField.php | 54 +++- tests/uts/dbFormhandler_UploadIdTest.php | 325 +++++++++++++++++++++++ 6 files changed, 609 insertions(+), 5 deletions(-) create mode 100644 tests/uts/dbFormhandler_UploadIdTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 0912221..d5991e9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,20 @@ All notable changes to this project are documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/) and the project follows [Semantic Versioning](https://semver.org/). +## [4.8.0] - unreleased + +### Added + +- `{id}` in the `name` of the upload config of `uploadField()` is replaced by the id of the + record, e.g. `'name' => 'spieler_{id}'`. An edit form uses the id from the URL. A new + record uploads the file after the INSERT with the id from `getInsertId()` and then saves + the filename in the column of the field (an UPDATE); when the INSERT fails, no file is + moved. `onCorrect` gets an empty filename for a new record, `onSaved` the real one. It + needs a `dbFormHandler` with a table and a single primary key; otherwise it throws a + `FormHandlerException`, as well as for an id with other characters than letters, digits, + `_` and `-`, and for a file posted on an earlier page of a form with more pages. +- `dbFormHandler::getEditId()` returns the id of the edited record (`null` for a new one). + ## [4.7.0] - 2026-10-30 ### Added diff --git a/README.md b/README.md index 5a8e1a7..4f62713 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,47 @@ this page". with `fh_conf('FH_CSRF_PROTECTION', false)` before the first form is created (the `fh_conf()` values become constants then). +### Uploads with the id of the record in the filename + +`{id}` in the `name` of the upload config is replaced by the id of the record (4.8). A new +record does not have an id while the form is checked, so FormHandler uploads the file after +the INSERT; there is no need to guess the next id with `SHOW TABLE STATUS`. + +```php +$form = new dbFormHandler(); +$form->setConnectionResource($conn, 'spieler'); +$form->textField('Name', 'name', FH_NOT_EMPTY); +$form->uploadField('Picture', 'bild', [ + 'path' => __DIR__ . '/images/spieler/', + 'type' => 'jpg jpeg png', + 'name' => 'spieler_{id}', // spieler_42.jpg + 'exists' => 'overwrite', // a new picture replaces the old one +]); +$form->onSaved('spielerSaved'); +$form->flush(); + +function spielerSaved($id, $data, $form) +{ + // $data['bild'] is "spieler_42.jpg" +} +``` + +- **Edit form** (`?id=42`): the file is uploaded as before, with the id from the URL. +- **New record**: the file is uploaded after the INSERT, then `imageResize()` and + `mergeImage()` of the field run. If the table has a column with the name of the field, + FormHandler saves the filename there with an UPDATE. When the INSERT fails, no file is + moved. +- `onCorrect` runs before the INSERT: for a new record the filename is still empty there, + `onSaved` gets it. `isUploaded()` and `getFileInfo()` work before the INSERT too. +- `'exists'` is used for the filename with the id. `'alert'` (the default) can not warn + on a new record, the filename is not known while the form is checked; use `'overwrite'` + or `'rename'`. +- It needs a `dbFormHandler` with a table and a single primary key. `FormHandler` throws a + `FormHandlerException`, as well as an id with other characters than letters, digits, + `_` and `-`. +- On a form with more pages the field has to be on the last page: the uploaded file of an + earlier page is gone when the record is saved. + ### Images and files in the editor The bundled file manager is removed (4.7): it had no login. Uploading or choosing files diff --git a/src/class.FormHandler.php b/src/class.FormHandler.php index 59057fb..2c3e1b8 100644 --- a/src/class.FormHandler.php +++ b/src/class.FormHandler.php @@ -3681,14 +3681,30 @@ public function _registerFileName($sFilename, $sField) * * Private: method to handle the uploads and image convertions * + * @param array|null $names: only upload and convert these uploadfields + * (null: all, except the ones waiting for {id}) * @return void * @access public * @author Teye Heimans */ - protected function _handleUploads() + protected function _handleUploads($names = null) { + $all = is_null($names); + if ($all) + { + // a filename with {id} waits for the id of the new record (see dbFormHandler) + $names = array(); + foreach ($this->_upload as $name) + { + if (!$this->_fields[$name][1]->hasIdPlaceholder()) + { + $names[] = $name; + } + } + } + // upload the uploaded files - foreach ($this->_upload as $name) + foreach ($names as $name) { $this->_fields[$name][1]->doUpload(); } @@ -3696,6 +3712,12 @@ protected function _handleUploads() // walk all convert actions for the upload fields foreach ($this->_convert as $field => $convertions) { + // only the convertions of the handled uploadfields (and of files, when all are handled) + if (in_array($field, $this->_upload) ? !in_array($field, $names) : !$all) + { + continue; + } + // walk all convertions for this field foreach ($convertions as $action => $data) { diff --git a/src/class.dbFormHandler.php b/src/class.dbFormHandler.php index feaa87a..870de4d 100644 --- a/src/class.dbFormHandler.php +++ b/src/class.dbFormHandler.php @@ -113,6 +113,18 @@ public function setEditName($sEditName) } } + /** + * dbFormHandler::getEditId() + * + * Return the id of the record which is edited (the first one when the URL has more) + * + * @return mixed the id, or null on a new record + */ + public function getEditId(): mixed + { + return $this->edit && sizeof($this->_id) > 0 ? reset($this->_id) : null; + } + /********************************************************/ /************* FIELDS ***********************************/ /********************************************************/ @@ -843,6 +855,16 @@ public function flush($return = false) $oAjaxValidator->CreateObservers($this, $this->_fields, $this->_customMsg); } + // {id} in the filename of an edit form: the id is known (not before the flush, a copy + // of a record changes to an insert form here). A new record gets it after the INSERT. + if ($this->edit) + { + foreach ($this->_getIdUploads() as $name) + { + $this->_fields[$name][1]->resolveIdPlaceholder($this->getEditId()); + } + } + // when the form is not posted or the form is not valid if (!$this->isPosted() || !$this->isCorrect()) { @@ -862,6 +884,17 @@ public function flush($return = false) // when the form is not totaly completed yet (multiple pages) else if ($this->_curPage < $this->_pageCounter) { + // a file waiting for {id} can not be kept until the last page + foreach ($this->_getIdUploads() as $name) + { + if ($this->_fields[$name][1]->isUploaded()) + { + throw new FormHandlerException( + 'The uploadfield "' . $name . '" uses {id} in its filename and has to be on the last page of the form!' + ); + } + } + // upload and convert uploads $this->_handleUploads(); @@ -871,7 +904,22 @@ public function flush($return = false) // when the form is valid else { - // upload and convert uploads + // a file waiting for {id} needs the INSERT + $idUploads = $this->_getIdUploads(); + if (is_null($this->_db) || empty($this->_table)) + { + foreach ($idUploads as $name) + { + if ($this->_fields[$name][1]->isUploaded()) + { + throw new FormHandlerException( + 'The uploadfield "' . $name . '" uses {id} in its filename, which needs a database table!' + ); + } + } + } + + // upload and convert uploads (the ones waiting for {id} after the INSERT) $this->_handleUploads(); // generate the data array @@ -923,6 +971,12 @@ public function flush($return = false) $id = $id[0]; } + // the filenames with {id} are known now + foreach ($idUploads as $name) + { + $data[$name] = $this->_fields[$name][1]->getValue(); + } + // call the onsaved function if (!is_null($this->_onSaved)) { @@ -1538,6 +1592,9 @@ protected function _saveDbData($data) $size = sizeof($result); $return = ($size > 1) ? $result : ($size == 1 ? $result[0] : null); } + + // the id of the new record is known: upload the files with {id} in their filename + $this->_handleIdUploads($return); } } @@ -1561,6 +1618,101 @@ protected function _saveDbData($data) return $return; } + /** + * dbFormHandler::_getIdUploads() + * + * Get the uploadfields whose filename waits for the id of the new record ({id}) + * + * @return array the names of the fields + */ + private function _getIdUploads(): array + { + $names = array(); + foreach ($this->_upload as $name) + { + if ($this->_fields[$name][1]->hasIdPlaceholder()) + { + $names[] = $name; + } + } + + return $names; + } + + /** + * dbFormHandler::_handleIdUploads() + * + * Replace {id} by the id of the new record, upload and convert these files and save + * their filenames in the record + * + * @param mixed $id: the id of the new record + * @return void + */ + private function _handleIdUploads(mixed $id): void + { + $names = $this->_getIdUploads(); + if (sizeof($names) == 0) + { + return; + } + + // one primary key is needed + if (is_array($id)) + { + $id = sizeof($id) == 1 ? reset($id) : null; + } + if (is_null($id) || $id === '' || $id === 0 || $id === '0') + { + throw new FormHandlerException( + 'The id of the new record is unknown, {id} can not be used in the filename of the uploadfield "' . + implode('", "', $names) . '"!' + ); + } + + foreach ($names as $name) + { + $this->_fields[$name][1]->resolveIdPlaceholder($id); + } + $this->_handleUploads($names); + + // the INSERT did not know the filenames: save them in their columns + $fieldNames = $this->_db->getFieldNames($this->_table); + $set = array(); + foreach ($names as $name) + { + $fld = $this->_fields[$name][1]; + if ($fld->isUploaded() && in_array($name, $fieldNames) && !in_array($name, $this->_dontSave) && !$fld->getViewMode()) + { + $set[] = $this->_db->quote($name) . " = '" . $this->_db->escapeString($fld->getValue()) . "'"; + } + } + if (sizeof($set) == 0) + { + return; + } + + $keys = $this->_db->getPrKeys($this->_table); + if (sizeof($keys) == 0) + { + trigger_error('Could not save the filename: the table ' . $this->_table . ' has no primary key', E_USER_WARNING); + return; + } + + $query = + 'UPDATE ' . $this->_db->quote($this->_table) . ' SET ' . implode(', ', $set) . + ' WHERE ' . $this->_db->quote($keys[0]) . " = '" . $this->_db->escapeString((string) $id) . "'"; + + if (!$this->_db->query($query)) + { + trigger_error( + "Error, query failed!
\n" . + "Error message: " . $this->_db->getError() . "
\n" . + "Query: " . $query, + E_USER_WARNING + ); + } + } + /** * dbFormHandler::_getQuery() * diff --git a/src/fields/class.UploadField.php b/src/fields/class.UploadField.php index 951a9ce..452f02f 100644 --- a/src/fields/class.UploadField.php +++ b/src/fields/class.UploadField.php @@ -101,6 +101,14 @@ public function __construct(&$oForm, $sName, $aConfig) $this->_oForm = $oForm; $this->_sName = $sName; + // {id} in the filename: the id of the record, only known by a dbFormHandler (which replaces it) + if ($this->hasIdPlaceholder() && !($oForm instanceof dbFormHandler)) + { + throw new FormHandlerException( + 'The uploadfield "' . $sName . '" uses {id} in its filename, which needs a dbFormHandler!' + ); + } + // get the value of the field if ($oForm->isPosted()) { @@ -127,8 +135,8 @@ public function __construct(&$oForm, $sName, $aConfig) // save the uploaded file data $this->_mValue = $_FILES[$sName]; - // check if there is a file uploaded... - if ($this->isUploaded()) + // check if there is a file uploaded... (a filename with {id} is known later) + if ($this->isUploaded() && !$this->hasIdPlaceholder()) { $this->setValue($this->_getFilename()); } @@ -241,6 +249,48 @@ public function getSavePath() return $this->_aConfig['path']; } + /** + * UploadField::hasIdPlaceholder() + * + * Does the filename (config "name") still contain {id}? Then the file waits for the id + * of the new record. + * + * @return bool + * @access public + */ + public function hasIdPlaceholder(): bool + { + return is_string($this->_aConfig['name']) && strpos($this->_aConfig['name'], '{id}') !== false; + } + + /** + * UploadField::resolveIdPlaceholder() + * + * Replace {id} in the filename with the id of the record (used by dbFormHandler) + * + * @param mixed $id: the id of the record + * @return void + * @access public + */ + public function resolveIdPlaceholder(mixed $id): void + { + // the id can come from the URL: it must not change the path + if (!is_scalar($id) || !preg_match('/^[A-Za-z0-9_-]+$/', (string) $id)) + { + throw new FormHandlerException( + 'The id "' . (is_scalar($id) ? $id : gettype($id)) . '" can not be used in the filename of the uploadfield "' . $this->_sName . '"!' + ); + } + + $this->_aConfig['name'] = str_replace('{id}', (string) $id, $this->_aConfig['name']); + + // the filename is known now + if ($this->isUploaded()) + { + $this->setValue($this->_getFilename()); + } + } + /** * UploadField::getValue() * diff --git a/tests/uts/dbFormhandler_UploadIdTest.php b/tests/uts/dbFormhandler_UploadIdTest.php new file mode 100644 index 0000000..52fb576 --- /dev/null +++ b/tests/uts/dbFormhandler_UploadIdTest.php @@ -0,0 +1,325 @@ +_isCopy = isset($_GET['funktion']) && $_GET['funktion'] == "copy"; + } + + public function uploadField($title, $name, $config = array(), $validator = null, $extra = null, $alertOverwrite = null) + { + $fld = new uploadIdTest_UploadField($this, $name, $config); + + if (!empty($validator)) $fld->setValidator($validator); + if (!empty($extra)) $fld->setExtra($extra); + if (!is_null($alertOverwrite)) $fld->setAlertOverwrite($alertOverwrite); + + $this->_registerField($name, $fld, $title); + $this->_upload[] = $name; + } + + public function flush($return = false) + { + if ($this->_isCopy) + { + $this->_id = array(); + $this->insert = true; + $this->edit = false; + } + + return parent::flush($return); + } +} + +final class dbFormhandler_UploadIdTest extends dbFormhandlerTestCase +{ + private string $_tempPath = ''; + + /** @var array the values given to onCorrect */ + private array $_onCorrectData = array(); + + protected function setUp(): void + { + parent::setUp(); + + $this->getConnection()->executeStatement( + "CREATE TABLE spieler ( + id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, + name VARCHAR(50) NOT NULL, + bild VARCHAR(255) NULL + )" + ); + + $this->_tempPath = sys_get_temp_dir() . "/uploadidtest"; + if (is_dir($this->_tempPath)) + { + array_map('unlink', glob($this->_tempPath . '/*')); + } + else + { + mkdir($this->_tempPath); + } + + $_POST['FormHandler_submit'] = "1"; + $_FILES = [ + 'bild' => [ + 'error' => UPLOAD_ERR_OK, + 'name' => 'test.jpg', + 'size' => 8009, + 'tmp_name' => dirname(__FILE__) . "/test.jpg", + 'type' => 'image/jpeg' + ] + ]; + } + + public function callback_onCorrect(array $data, FormHandler $form): bool + { + $this->_onCorrectData = $data; + return true; + } + + /** + * a form for the table "spieler" with the uploadfield "bild" (posted with test.jpg) + */ + private function createForm(array $config = array()): uploadIdTest_dbFormHandler + { + $form = new uploadIdTest_dbFormHandler(); + $form->setConnectionResource($this->getConnection(), 'spieler'); + + $form->textField("Name", "name"); + $form->uploadField("Bild", "bild", array_merge(array( + "type" => "jpg", + "name" => "spieler_{id}", + "path" => $this->_tempPath, + "exists" => "overwrite" + ), $config)); + + return $form; + } + + private function assertFiles(array $expected): void + { + $files = array_map('basename', glob($this->_tempPath . '/*')); + sort($files); + + static::assertSame($expected, $files); + } + + private function getBild(int $id): mixed + { + return $this->getConnection()->fetchOne('SELECT bild FROM spieler WHERE id = ?', [$id]); + } + + public function test_insert(): void + { + $this->getConnection()->insert('spieler', ['id' => 7, 'name' => 'old']); + $_POST['name'] = "new"; + + $form = $this->createForm(); + $form->onCorrect(array($this, "callback_onCorrect")); + $this->setCallbackOnSaved($form); + + // before the INSERT: the file is uploaded, its name is not known yet + static::assertTrue($form->isUploaded("bild")); + static::assertEquals($_FILES['bild'], $form->getFileInfo("bild")); + static::assertSame('', $form->getValue("bild")); + + static::assertFlush($form); + + static::assertSame('', $this->_onCorrectData['bild']); + $this->assertSavedId(8); + $this->assertSavedValue('spieler_8.jpg', 'bild'); + static::assertSame('spieler_8.jpg', $this->getBild(8)); + static::assertSame('spieler_8.jpg', $form->getValue("bild")); + $this->assertFiles(['spieler_8.jpg']); + } + + public function test_insert_noFile(): void + { + $_POST['name'] = "new"; + $_FILES['bild'] = ['error' => UPLOAD_ERR_NO_FILE, 'name' => '', 'size' => 0, 'tmp_name' => '', 'type' => '']; + + $form = $this->createForm(); + $this->setCallbackOnSaved($form); + + static::assertFlush($form); + + $this->assertSavedId(1); + $this->assertSavedValueEmtpy('bild'); + static::assertNull($this->getBild(1)); + $this->assertFiles([]); + } + + public function test_insert_rename(): void + { + static::assertTrue(copy(dirname(__FILE__) . "/test.jpg", "{$this->_tempPath}/spieler_1.jpg")); + $_POST['name'] = "new"; + + $form = $this->createForm(['exists' => 'rename']); + + static::assertFlush($form); + + static::assertSame('spieler_1(1).jpg', $this->getBild(1)); + $this->assertFiles(['spieler_1(1).jpg', 'spieler_1.jpg']); + } + + public function test_insert_fieldNotInTable(): void + { + $_POST['name'] = "new"; + $_FILES['foto'] = $_FILES['bild']; + + $form = $this->createForm(); + $form->uploadField("Foto", "foto", [ + "type" => "jpg", + "name" => "foto_{id}", + "path" => $this->_tempPath + ]); + $this->setCallbackOnSaved($form); + + static::assertFlush($form); + + $this->assertSavedValue('foto_1.jpg', 'foto'); + $this->assertFiles(['foto_1.jpg', 'spieler_1.jpg']); + } + + public function test_insert_failed(): void + { + // no name: the NOT NULL column makes the INSERT fail + + $errors = array(); + set_error_handler(function (int $level, string $message) use (&$errors) + { + $errors[] = $message; + return true; + }); + + $form = new uploadIdTest_dbFormHandler(); + $form->setConnectionResource($this->getConnection(), 'spieler'); + $form->uploadField("Bild", "bild", [ + "type" => "jpg", + "name" => "spieler_{id}", + "path" => $this->_tempPath + ]); + + $r = $form->flush(true); + restore_error_handler(); + + static::assertCount(1, $errors); + static::assertStringStartsWith('Error, query failed!', $errors[0]); + static::assertNotEmpty($r); + static::assertEquals(0, $this->getConnection()->fetchOne('SELECT COUNT(*) FROM spieler')); + $this->assertFiles([]); + } + + public function test_edit(): void + { + $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old', 'bild' => 'old.jpg']); + $_GET['id'] = "100"; + $_POST['name'] = "new"; + + $form = $this->createForm(); + $this->setCallbackOnSaved($form); + + static::assertFlush($form); + + $this->assertSavedId(100); + $this->assertSavedValue('spieler_100.jpg', 'bild'); + static::assertSame('spieler_100.jpg', $this->getBild(100)); + $this->assertFiles(['spieler_100.jpg']); + } + + public function test_copy(): void + { + $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old', 'bild' => 'spieler_100.jpg']); + static::assertTrue(file_put_contents("{$this->_tempPath}/spieler_100.jpg", 'original') > 0); + $_GET['id'] = "100"; + $_GET['funktion'] = "copy"; + $_POST['name'] = "copy"; + + $form = $this->createForm(); + + static::assertFlush($form); + + // the copy gets its own file, the one of the original is kept + static::assertSame('spieler_101.jpg', $this->getBild(101)); + static::assertSame('spieler_100.jpg', $this->getBild(100)); + static::assertSame('original', file_get_contents("{$this->_tempPath}/spieler_100.jpg")); + $this->assertFiles(['spieler_100.jpg', 'spieler_101.jpg']); + } + + public function test_invalidId(): void + { + $form = $this->createForm(); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('The id "../x" can not be used in the filename of the uploadfield "bild"!'); + + $form->getField('bild')->resolveIdPlaceholder('../x'); + } + + public function test_multiplePages(): void + { + $_POST['name'] = "new"; + + $form = $this->createForm(); + $form->newPage(); + $form->textField("Page 2", "page2"); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename and has to be on the last page of the form!'); + + $form->flush(true); + } + + public function test_noTable(): void + { + $form = new uploadIdTest_dbFormHandler(); + $form->uploadField("Bild", "bild", [ + "type" => "jpg", + "name" => "spieler_{id}", + "path" => $this->_tempPath + ]); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename, which needs a database table!'); + + $form->flush(true); + } + + public function test_formHandler(): void + { + $form = new FormHandler(); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename, which needs a dbFormHandler!'); + + $form->uploadField("Bild", "bild", ["name" => "spieler_{id}"]); + } +} From a9902dbb76f494db57c6eb82be955dfa9bd0df33 Mon Sep 17 00:00:00 2001 From: tombrain Date: Fri, 30 Oct 2026 23:10:46 +0100 Subject: [PATCH 2/7] feat: dbFormHandler::getNewId() and getRecordId() --- CHANGELOG.md | 5 ++ README.md | 2 + src/class.dbFormHandler.php | 31 ++++++++ tests/uts/dbFormhandler_CopyTest.php | 20 +++++ tests/uts/dbFormhandler_RecordIdTest.php | 93 ++++++++++++++++++++++++ 5 files changed, 151 insertions(+) create mode 100644 tests/uts/dbFormhandler_RecordIdTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index d5991e9..74129e3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,11 @@ and the project follows [Semantic Versioning](https://semver.org/). `FormHandlerException`, as well as for an id with other characters than letters, digits, `_` and `-`, and for a file posted on an earlier page of a form with more pages. - `dbFormHandler::getEditId()` returns the id of the edited record (`null` for a new one). +- `dbFormHandler::getNewId()` returns the id the INSERT gave a new record (an array with + more primary keys; `null` before the INSERT, on an edit form and when the INSERT failed). + It is set right after the INSERT, so `onSaved` can use it already. `getRecordId()` + returns the id of the record the form works on: `getNewId() ?? getEditId()`. Projects + no longer have to override `_saveDbData()` to remember the id. ## [4.7.0] - 2026-10-30 diff --git a/README.md b/README.md index 4f62713..a2a61f0 100644 --- a/README.md +++ b/README.md @@ -182,6 +182,8 @@ function spielerSaved($id, $data, $form) moved. - `onCorrect` runs before the INSERT: for a new record the filename is still empty there, `onSaved` gets it. `isUploaded()` and `getFileInfo()` work before the INSERT too. +- After saving, `$form->getRecordId()` returns the id of the record, of a new one as well + as of an edited one (`getNewId()` only the id of the INSERT). - `'exists'` is used for the filename with the id. `'alert'` (the default) can not warn on a new record, the filename is not known while the form is checked; use `'overwrite'` or `'rename'`. diff --git a/src/class.dbFormHandler.php b/src/class.dbFormHandler.php index 870de4d..c075554 100644 --- a/src/class.dbFormHandler.php +++ b/src/class.dbFormHandler.php @@ -33,6 +33,8 @@ class dbFormHandler extends FormHandler protected $_table; /** name of the primary key to edit (set by setEditName() in the constructor) */ private string $_editName = ''; + /** the id the INSERT gave the new record (an array with more primary keys) */ + private mixed $_newId = null; // public /** @var bool if the form is an insert-form */ @@ -125,6 +127,32 @@ public function getEditId(): mixed return $this->edit && sizeof($this->_id) > 0 ? reset($this->_id) : null; } + /** + * dbFormHandler::getNewId() + * + * Return the id the INSERT gave the new record + * + * @return mixed the id (an array with more primary keys), or null before the INSERT, + * on an edit form and when the INSERT failed + */ + public function getNewId(): mixed + { + return $this->_newId; + } + + /** + * dbFormHandler::getRecordId() + * + * Return the id of the record the form works on: the new one after the INSERT, + * otherwise the edited one + * + * @return mixed the id, or null for a new record which is not saved yet + */ + public function getRecordId(): mixed + { + return $this->getNewId() ?? $this->getEditId(); + } + /********************************************************/ /************* FIELDS ***********************************/ /********************************************************/ @@ -1593,6 +1621,9 @@ protected function _saveDbData($data) $return = ($size > 1) ? $result : ($size == 1 ? $result[0] : null); } + // remember the id for getNewId(), already for what is called from here on + $this->_newId = $return; + // the id of the new record is known: upload the files with {id} in their filename $this->_handleIdUploads($return); } diff --git a/tests/uts/dbFormhandler_CopyTest.php b/tests/uts/dbFormhandler_CopyTest.php index 590e70e..f07237b 100644 --- a/tests/uts/dbFormhandler_CopyTest.php +++ b/tests/uts/dbFormhandler_CopyTest.php @@ -78,4 +78,24 @@ public function test_copy_savesANewRecord(): void $this->assertRecord(100, ['textNullable' => 'text1', 'textNotNullable' => 'text2']); $this->assertRecord(101, ['textNullable' => 'copy1', 'textNotNullable' => 'copy2']); } + + public function test_copy_newId(): void + { + $this->insertRecord(['id' => 100, 'textNullable' => 'text1', 'textNotNullable' => 'text2']); + + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "copy1"; + $_GET['id'] = "100"; + $_GET['funktion'] = "copy"; + + $form = new copyTest_dbFormHandler(); + $this->setConnectedTable($form, "test"); + + $form->textField("TextNullable", "textNullable"); + + static::assertFlush($form); + + static::assertEquals(101, $form->getNewId()); + static::assertEquals(101, $form->getRecordId()); + } } diff --git a/tests/uts/dbFormhandler_RecordIdTest.php b/tests/uts/dbFormhandler_RecordIdTest.php new file mode 100644 index 0000000..0a99e26 --- /dev/null +++ b/tests/uts/dbFormhandler_RecordIdTest.php @@ -0,0 +1,93 @@ + what onSaved got and getNewId() returned there */ + private array $_onSaved = array(); + + public function callback_onSaved_newId(mixed $id, array $data, dbFormHandler $form): bool + { + $this->_onSaved = ['id' => $id, 'newId' => $form->getNewId()]; + return true; + } + + public function test_insert(): void + { + $this->insertRecord(['id' => 7, 'textNullable' => 'old']); + + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "new"; + + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + $form->textField("TextNullable", "textNullable"); + $form->onSaved(array($this, "callback_onSaved_newId")); + + static::assertNull($form->getNewId()); + static::assertNull($form->getRecordId()); + + static::assertFlush($form); + + static::assertEquals(8, $form->getNewId()); + static::assertEquals(8, $form->getRecordId()); + + // onSaved gets the same id, and getNewId() knows it already there + static::assertEquals(8, $this->_onSaved['id']); + static::assertSame($this->_onSaved['id'], $this->_onSaved['newId']); + } + + public function test_edit(): void + { + $this->insertRecord(['id' => 5, 'textNullable' => 'old']); + + $_GET['id'] = "5"; + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "new"; + + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + $form->textField("TextNullable", "textNullable"); + + static::assertNull($form->getNewId()); + static::assertEquals(5, $form->getRecordId()); + + static::assertFlush($form); + + static::assertNull($form->getNewId()); + static::assertEquals(5, $form->getRecordId()); + $this->assertRecord(5, ['textNullable' => 'new']); + } + + public function test_insert_failed(): void + { + // no value for the NOT NULL column "required": the INSERT fails + $this->getConnection()->executeStatement( + "CREATE TABLE strict (id INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT, required VARCHAR(10) NOT NULL, optional VARCHAR(10) NULL)" + ); + + $_POST['FormHandler_submit'] = "1"; + $_POST['optional'] = "text"; + + $errors = array(); + set_error_handler(function (int $level, string $message) use (&$errors) + { + $errors[] = $message; + return true; + }); + + $form = new dbFormHandler(); + $this->setConnectedTable($form, "strict"); + $form->textField("Optional", "optional"); + + $r = $form->flush(true); + restore_error_handler(); + + static::assertCount(1, $errors); + static::assertStringStartsWith('Error, query failed!', $errors[0]); + static::assertNotEmpty($r); + static::assertNull($form->getNewId()); + static::assertNull($form->getRecordId()); + } +} From 131ed26226b084fa4e12f1a56d010cc1b0e315ab Mon Sep 17 00:00:00 2001 From: tombrain Date: Fri, 30 Oct 2026 23:10:46 +0100 Subject: [PATCH 3/7] feat: {id} in the filename of resizeImage() --- CHANGELOG.md | 4 + README.md | 3 + src/class.FormHandler.php | 37 +++++++- src/class.dbFormHandler.php | 37 ++++++-- tests/uts/dbFormhandler_UploadIdTest.php | 106 ++++++++++++++++++++++- 5 files changed, 178 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74129e3..5c73589 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ and the project follows [Semantic Versioning](https://semver.org/). needs a `dbFormHandler` with a table and a single primary key; otherwise it throws a `FormHandlerException`, as well as for an id with other characters than letters, digits, `_` and `-`, and for a file posted on an earlier page of a form with more pages. +- `{id}` in the filename of `resizeImage()` (`$saveAs`) is replaced by the same id, e.g. + `resizeImage('bild', $dir . 'spieler_{id}.jpg', 200)`. On a new record the upload of the + field waits for the INSERT then, also when its own filename has no `{id}`. `FormHandler` + throws a `FormHandlerException` for it. - `dbFormHandler::getEditId()` returns the id of the edited record (`null` for a new one). - `dbFormHandler::getNewId()` returns the id the INSERT gave a new record (an array with more primary keys; `null` before the INSERT, on an edit form and when the INSERT failed). diff --git a/README.md b/README.md index a2a61f0..538e0ee 100644 --- a/README.md +++ b/README.md @@ -180,6 +180,9 @@ function spielerSaved($id, $data, $form) `mergeImage()` of the field run. If the table has a column with the name of the field, FormHandler saves the filename there with an UPDATE. When the INSERT fails, no file is moved. +- `{id}` also works in the filename of `resizeImage()`, e.g. + `$form->resizeImage('bild', __DIR__ . '/images/spieler/klein_{id}.jpg', 200)`. On a new + record the field then waits for the INSERT, even when its own filename has no `{id}`. - `onCorrect` runs before the INSERT: for a new record the filename is still empty there, `onSaved` gets it. `isUploaded()` and `getFileInfo()` work before the INSERT too. - After saving, `$form->getRecordId()` returns the id of the record, of a new one as well diff --git a/src/class.FormHandler.php b/src/class.FormHandler.php index 2c3e1b8..c74d9d6 100644 --- a/src/class.FormHandler.php +++ b/src/class.FormHandler.php @@ -3146,6 +3146,14 @@ public function resizeImage($field, $saveAs = null, $maxWidth = null, $maxHeight if (is_null($maxWidth)) $maxWidth = FH_DEFAULT_RESIZE_WIDTH; if (is_null($maxHeight)) $maxHeight = $maxWidth; + // {id}: the id of the record, only known by a dbFormHandler (which replaces it) + if (is_string($saveAs) && strpos($saveAs, '{id}') !== false && !($this instanceof dbFormHandler)) + { + throw new FormHandlerException( + 'resizeImage() of the field "' . $field . '" uses {id} in its filename, which needs a dbFormHandler!' + ); + } + // save the settings $this->_convert[$field]['resize'][] = array($saveAs, $maxWidth, $maxHeight, $quality, $constrainProportions); } @@ -3676,6 +3684,33 @@ public function _registerFileName($sFilename, $sField) return true; } + /** + * FormHandler::_waitsForId() + * + * Does the uploadfield wait for the id of the record? ({id} in its filename or in the + * filename of resizeImage()) + * + * @param string $name: the name of the uploadfield + * @return bool + */ + protected function _waitsForId(string $name): bool + { + if ($this->_fields[$name][1]->hasIdPlaceholder()) + { + return true; + } + + foreach ($this->_convert[$name]['resize'] ?? array() as $info) + { + if (is_string($info[0]) && strpos($info[0], '{id}') !== false) + { + return true; + } + } + + return false; + } + /** * FormHandler::_handleUploads() * @@ -3696,7 +3731,7 @@ protected function _handleUploads($names = null) $names = array(); foreach ($this->_upload as $name) { - if (!$this->_fields[$name][1]->hasIdPlaceholder()) + if (!$this->_waitsForId($name)) { $names[] = $name; } diff --git a/src/class.dbFormHandler.php b/src/class.dbFormHandler.php index c075554..7985566 100644 --- a/src/class.dbFormHandler.php +++ b/src/class.dbFormHandler.php @@ -889,7 +889,7 @@ public function flush($return = false) { foreach ($this->_getIdUploads() as $name) { - $this->_fields[$name][1]->resolveIdPlaceholder($this->getEditId()); + $this->_resolveIdUpload($name, $this->getEditId()); } } @@ -918,7 +918,7 @@ public function flush($return = false) if ($this->_fields[$name][1]->isUploaded()) { throw new FormHandlerException( - 'The uploadfield "' . $name . '" uses {id} in its filename and has to be on the last page of the form!' + 'The uploadfield "' . $name . '" uses {id} in its filename or in resizeImage() and has to be on the last page of the form!' ); } } @@ -941,7 +941,7 @@ public function flush($return = false) if ($this->_fields[$name][1]->isUploaded()) { throw new FormHandlerException( - 'The uploadfield "' . $name . '" uses {id} in its filename, which needs a database table!' + 'The uploadfield "' . $name . '" uses {id} in its filename or in resizeImage(), which needs a database table!' ); } } @@ -1652,7 +1652,8 @@ protected function _saveDbData($data) /** * dbFormHandler::_getIdUploads() * - * Get the uploadfields whose filename waits for the id of the new record ({id}) + * Get the uploadfields which wait for the id of the new record ({id} in the filename + * or in resizeImage()) * * @return array the names of the fields */ @@ -1661,7 +1662,7 @@ private function _getIdUploads(): array $names = array(); foreach ($this->_upload as $name) { - if ($this->_fields[$name][1]->hasIdPlaceholder()) + if ($this->_waitsForId($name)) { $names[] = $name; } @@ -1670,6 +1671,30 @@ private function _getIdUploads(): array return $names; } + /** + * dbFormHandler::_resolveIdUpload() + * + * Replace {id} by the id of the record in the filename of the uploadfield and in the + * filenames of its resizeImage() + * + * @param string $name: the name of the uploadfield + * @param mixed $id: the id of the record + * @return void + */ + private function _resolveIdUpload(string $name, mixed $id): void + { + // checks the id too + $this->_fields[$name][1]->resolveIdPlaceholder($id); + + foreach ($this->_convert[$name]['resize'] ?? array() as $i => $info) + { + if (is_string($info[0])) + { + $this->_convert[$name]['resize'][$i][0] = str_replace('{id}', (string) $id, $info[0]); + } + } + } + /** * dbFormHandler::_handleIdUploads() * @@ -1702,7 +1727,7 @@ private function _handleIdUploads(mixed $id): void foreach ($names as $name) { - $this->_fields[$name][1]->resolveIdPlaceholder($id); + $this->_resolveIdUpload($name, $id); } $this->_handleUploads($names); diff --git a/tests/uts/dbFormhandler_UploadIdTest.php b/tests/uts/dbFormhandler_UploadIdTest.php index 52fb576..b3e031e 100644 --- a/tests/uts/dbFormhandler_UploadIdTest.php +++ b/tests/uts/dbFormhandler_UploadIdTest.php @@ -293,7 +293,7 @@ public function test_multiplePages(): void $form->textField("Page 2", "page2"); $this->expectException(FormHandlerException::class); - $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename and has to be on the last page of the form!'); + $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename or in resizeImage() and has to be on the last page of the form!'); $form->flush(true); } @@ -308,11 +308,113 @@ public function test_noTable(): void ]); $this->expectException(FormHandlerException::class); - $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename, which needs a database table!'); + $this->expectExceptionMessage('The uploadfield "bild" uses {id} in its filename or in resizeImage(), which needs a database table!'); $form->flush(true); } + /** + * a form whose upload keeps its name ("upload.jpg") and whose resizeImage() uses {id} + */ + private function createResizeForm(): uploadIdTest_dbFormHandler + { + $form = $this->createForm(['name' => 'upload']); + $form->resizeImage('bild', $this->_tempPath . '/spieler_{id}.jpg', 30, 30); + + return $form; + } + + private function assertResized(string $file): void + { + $info = getimagesize($this->_tempPath . '/' . $file); + static::assertIsArray($info, "$file is no image"); + static::assertEquals(30, $info[0]); + } + + public function test_resize_insert(): void + { + $this->getConnection()->insert('spieler', ['id' => 7, 'name' => 'old']); + $_POST['name'] = "new"; + + $form = $this->createResizeForm(); + + static::assertFlush($form); + + // the upload without {id} waited for the INSERT too + static::assertSame('upload.jpg', $this->getBild(8)); + $this->assertFiles(['spieler_8.jpg', 'upload.jpg']); + $this->assertResized('spieler_8.jpg'); + } + + public function test_resize_edit(): void + { + $this->getConnection()->insert('spieler', ['id' => 5, 'name' => 'old']); + $_GET['id'] = "5"; + $_POST['name'] = "new"; + + $form = $this->createResizeForm(); + + static::assertFlush($form); + + static::assertSame('upload.jpg', $this->getBild(5)); + $this->assertFiles(['spieler_5.jpg', 'upload.jpg']); + $this->assertResized('spieler_5.jpg'); + } + + public function test_resize_copy(): void + { + $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old']); + $_GET['id'] = "100"; + $_GET['funktion'] = "copy"; + $_POST['name'] = "copy"; + + $form = $this->createResizeForm(); + + static::assertFlush($form); + + $this->assertFiles(['spieler_101.jpg', 'upload.jpg']); + $this->assertResized('spieler_101.jpg'); + } + + public function test_resize_insert_failed(): void + { + // no name: the NOT NULL column makes the INSERT fail + + $errors = array(); + set_error_handler(function (int $level, string $message) use (&$errors) + { + $errors[] = $message; + return true; + }); + + $form = new uploadIdTest_dbFormHandler(); + $form->setConnectionResource($this->getConnection(), 'spieler'); + $form->uploadField("Bild", "bild", [ + "type" => "jpg", + "name" => "upload", + "path" => $this->_tempPath + ]); + $form->resizeImage('bild', $this->_tempPath . '/spieler_{id}.jpg', 30, 30); + + $r = $form->flush(true); + restore_error_handler(); + + static::assertCount(1, $errors); + static::assertStringStartsWith('Error, query failed!', $errors[0]); + static::assertNotEmpty($r); + $this->assertFiles([]); + } + + public function test_resize_formHandler(): void + { + $form = new FormHandler(); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('resizeImage() of the field "bild" uses {id} in its filename, which needs a dbFormHandler!'); + + $form->resizeImage('bild', $this->_tempPath . '/spieler_{id}.jpg', 30); + } + public function test_formHandler(): void { $form = new FormHandler(); From d8305122b75c72a0771085bfe8fbcc0c6499b68e Mon Sep 17 00:00:00 2001 From: tombrain Date: Sat, 31 Oct 2026 19:32:15 +0100 Subject: [PATCH 4/7] fix: cancelButton() escapes its URL --- CHANGELOG.md | 9 ++++++ src/class.FormHandler.php | 45 +++++++++++++++++++++++---- tests/uts/formhandler_ButtonsTest.php | 39 ++++++++++++++++++++++- 3 files changed, 86 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c73589..f820889 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,15 @@ and the project follows [Semantic Versioning](https://semver.org/). returns the id of the record the form works on: `getNewId() ?? getEditId()`. Projects no longer have to override `_saveDbData()` to remember the id. +### Security + +- `cancelButton()` escapes its URL: it is put into the javascript as a JSON string and the + `onclick` attribute is HTML escaped, so a URL from the request with `'` or `"` can not + break out any more. `&` (from a HTML link) is decoded first. A URL with another + scheme than `http`/`https` (e.g. `javascript:`, `data:`) goes back in the history, like a + button without a URL. Only a history call such as `history.back(-1)` or `history.go(-2)` + is used as javascript; before, every URL containing "history" was (e.g. `/history/x.php`). + ## [4.7.0] - 2026-10-30 ### Added diff --git a/src/class.FormHandler.php b/src/class.FormHandler.php index c74d9d6..b8b5c9f 100644 --- a/src/class.FormHandler.php +++ b/src/class.FormHandler.php @@ -1317,13 +1317,8 @@ public function cancelButton($caption = null, $url = null, $name = null, $extra $name = $this->_getNewButtonName(); } - if (!$url) - { - $url = 'history.back(-1)'; - } - // where to go when the button is clicked... - $extra .= preg_match('/history/', $url) ? ' onclick="' . $url . '"' : ' onclick="document.location.href=\'' . $url . '\'"'; + $extra .= ' onclick="' . htmlspecialchars($this->_getCancelScript($url), ENT_QUOTES) . '"'; // if no caption is given, get our own caption if (is_null($caption)) @@ -1340,6 +1335,44 @@ public function cancelButton($caption = null, $url = null, $name = null, $extra $this->_registerField($name, $btn); } + /** + * FormHandler::_getCancelScript() + * + * Get the javascript of a cancelButton: go to the URL, or one page back + * + * @param string|null $url: the URL, or a history call like "history.back(-1)" + * @return string the javascript (not escaped for the HTML attribute yet) + */ + private function _getCancelScript(?string $url): string + { + $back = 'history.back(-1)'; + if (empty($url)) + { + return $back; + } + + // a history call is javascript + if (preg_match('/^history\.(back|go)\(\s*-?\d*\s*\);?$/', trim($url))) + { + return trim($url); + } + + // the URL from a HTML link: & is & + $url = html_entity_decode($url, ENT_QUOTES | ENT_HTML5, 'UTF-8'); + + // only http and https (javascript:, data:, ... go back); the browser ignores + // whitespace and control characters, e.g. in "java\tscript:" + if (preg_match('/^([a-z][a-z0-9+.\-]*):/i', preg_replace('/[\x00-\x20]+/', '', $url), $match) && + !in_array(strtolower($match[1]), array('http', 'https')) + ) + { + return $back; + } + + return 'document.location.href=' . + json_encode($url, JSON_HEX_TAG | JSON_HEX_APOS | JSON_HEX_QUOT | JSON_HEX_AMP | JSON_UNESCAPED_SLASHES | JSON_UNESCAPED_UNICODE); + } + /** * FormHandler::backButton() * diff --git a/tests/uts/formhandler_ButtonsTest.php b/tests/uts/formhandler_ButtonsTest.php index 118513c..e179e2b 100644 --- a/tests/uts/formhandler_ButtonsTest.php +++ b/tests/uts/formhandler_ButtonsTest.php @@ -2,6 +2,8 @@ declare(strict_types=1); +use PHPUnit\Framework\Attributes\DataProvider; + final class formhandler_ButtonsTest extends FormhandlerTestCase { public function test_button(): void @@ -109,12 +111,47 @@ public function test_cancelButton(): void static::assertFormFlushContains($form, [ 'error_button1', 'error_button2', - 'error_button3', + 'error_button3', 'error_cancelbutton', 'error_button4' ]); } + /** + * @return array + */ + public static function cancelUrlProvider(): array + { + return [ + 'no url' => [null, 'history.back(-1)'], + 'relative url' => ['list.php', 'document.location.href="list.php"'], + 'absolute url' => ['https://example.com/a/b', 'document.location.href="https://example.com/a/b"'], + 'apostrophe' => ["list.php?q=it's", 'document.location.href="list.php?q=it\u0027s"'], + 'quote' => ['list.php?q="x"', 'document.location.href="list.php?q=\u0022x\u0022"'], + 'html link' => ['list.php?id=1&page=2', 'document.location.href="list.php?id=1\u0026page=2"'], + 'tag' => ['list.php?q=', 'document.location.href="list.php?q=\u003C/script\u003E"'], + 'javascript' => ['javascript:alert(1)', 'history.back(-1)'], + 'javascript upper' => [' JavaScript:alert(1)', 'history.back(-1)'], + 'javascript tab' => ["java\tscript:alert(1)", 'history.back(-1)'], + 'data' => ['data:text/html,', 'history.back(-1)'], + 'history call' => ['history.go(-2)', 'history.go(-2)'], + 'history in path' => ['/history/list.php', 'document.location.href="/history/list.php"'], + ]; + } + + #[DataProvider('cancelUrlProvider')] + public function test_cancelButton_url(?string $url, string $onclick): void + { + $form = new FormHandler(); + + $form->cancelButton(null, $url); + + static::assertFormFlushContains( + $form, + 'error_button1' + ); + } + public function test_backButton(): void { $form = new FormHandler(); From fe5d8d9d17b60f116e8b946609142524cfa3ea81 Mon Sep 17 00:00:00 2001 From: tombrain Date: Sat, 31 Oct 2026 21:55:12 +0100 Subject: [PATCH 5/7] feat: dbFormHandler keeps its database adapter after saving, getDatabaseAdapter() --- CHANGELOG.md | 7 ++ README.md | 4 ++ src/class.dbFormHandler.php | 16 ++++- .../uts/dbFormhandler_DatabaseAdapterTest.php | 66 +++++++++++++++++++ 4 files changed, 90 insertions(+), 3 deletions(-) create mode 100644 tests/uts/dbFormhandler_DatabaseAdapterTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index f820889..a373fe5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,13 @@ and the project follows [Semantic Versioning](https://semver.org/). It is set right after the INSERT, so `onSaved` can use it already. `getRecordId()` returns the id of the record the form works on: `getNewId() ?? getEditId()`. Projects no longer have to override `_saveDbData()` to remember the id. +- `dbFormHandler::getDatabaseAdapter()` returns the database adapter of the form. + +### Changed + +- `dbFormHandler` keeps its database adapter after saving the record (it was unset), so + `onSaved` and the code after `flush()` can still query the database, e.g. to compare + the old and the new values. Projects no longer have to remember the connection. ### Security diff --git a/README.md b/README.md index 538e0ee..f4263a8 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,10 @@ this page". with `fh_conf('FH_CSRF_PROTECTION', false)` before the first form is created (the `fh_conf()` values become constants then). +**The database after saving** (4.8). `$form->getDatabaseAdapter()` returns the +`DatabaseAdapter` of a `dbFormHandler`, also after `flush()` saved the record; before 4.8 +it was unset after saving. + ### Uploads with the id of the record in the filename `{id}` in the `name` of the upload config is replaced by the id of the record (4.8). A new diff --git a/src/class.dbFormHandler.php b/src/class.dbFormHandler.php index 7985566..6846551 100644 --- a/src/class.dbFormHandler.php +++ b/src/class.dbFormHandler.php @@ -667,6 +667,18 @@ public function setDatabaseAdapter($db) } } + /** + * dbFormHandler::getDatabaseAdapter() + * + * Return the database adapter of the form, also after the record is saved + * + * @return DatabaseAdapter|null the adapter, or null when no database is set + */ + public function getDatabaseAdapter(): ?DatabaseAdapter + { + return $this->_db; + } + /** * dbFormHandler::dbInfo() * @@ -1643,9 +1655,7 @@ protected function _saveDbData($data) } } - // unset the database object (we dont need it anymore) - unset($this->_db); - + // the database object is kept: onSaved and the code after flush() may still need it return $return; } diff --git a/tests/uts/dbFormhandler_DatabaseAdapterTest.php b/tests/uts/dbFormhandler_DatabaseAdapterTest.php new file mode 100644 index 0000000..5ad3100 --- /dev/null +++ b/tests/uts/dbFormhandler_DatabaseAdapterTest.php @@ -0,0 +1,66 @@ +getDatabaseAdapter()); + } + + public function test_beforeSaving(): void + { + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + + static::assertInstanceOf(DbalAdapter::class, $form->getDatabaseAdapter()); + } + + public function test_setDatabaseAdapter(): void + { + $adapter = new DbalAdapter($this->getConnection()); + + $form = new dbFormHandler(); + $form->setDatabaseAdapter($adapter); + + static::assertSame($adapter, $form->getDatabaseAdapter()); + } + + public function test_afterSaving(): void + { + $this->insertRecord(['id' => 5, 'textNullable' => 'old']); + + $_GET['id'] = "5"; + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "new"; + + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + $form->textField("TextNullable", "textNullable"); + + static::assertFlush($form); + + // e.g. to compare the old and the new values after saving + $db = $form->getDatabaseAdapter(); + static::assertInstanceOf(DbalAdapter::class, $db); + $sql = $db->query("SELECT textNullable FROM test WHERE id = 5"); + static::assertEquals(['textNullable' => 'new'], $db->getRecord($sql)); + } + + public function test_afterInsert(): void + { + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "new"; + + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + $form->textField("TextNullable", "textNullable"); + + static::assertFlush($form); + + static::assertInstanceOf(DbalAdapter::class, $form->getDatabaseAdapter()); + } +} From eca9bf1b9176ee7583569fbdf1c99ad63f599ae7 Mon Sep 17 00:00:00 2001 From: tombrain Date: Sat, 31 Oct 2026 22:01:12 +0100 Subject: [PATCH 6/7] feat: dbFormHandler::setCopyOf() copies a record, isCopy() and getCopyOfId() --- CHANGELOG.md | 10 ++ README.md | 16 ++ src/class.dbFormHandler.php | 102 ++++++++++- src/fields/class.UploadField.php | 22 ++- tests/uts/dbFormhandler_CopyTest.php | 159 +++++++++++++----- tests/uts/dbFormhandler_JunctionTableTest.php | 31 +++- tests/uts/dbFormhandler_UploadIdTest.php | 73 ++++---- 7 files changed, 326 insertions(+), 87 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a373fe5..134fa7e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,16 @@ and the project follows [Semantic Versioning](https://semver.org/). returns the id of the record the form works on: `getNewId() ?? getEditId()`. Projects no longer have to override `_saveDbData()` to remember the id. - `dbFormHandler::getDatabaseAdapter()` returns the database adapter of the form. +- `dbFormHandler::setCopyOf($id)` copies a record: the form shows the record with this id + and saves it as a new record (an INSERT). Call it before `dbInfo()`, + `setConnectionResource()` and `setDatabaseAdapter()`, on every request of the form. + The primary keys and the files of the uploadfields are not copied (both records would + use the same file), the values of the junction tables are. `isCopy()` and + `getCopyOfId()` tell if and which record is copied; `getEditId()` is `null` for a copy, + `getNewId()` and `getRecordId()` return the id of the new record after saving. Unique + fields are checked like on a new record. A record which does not exist throws a + `FormHandlerException`. Projects no longer have to change `$insert`, `$edit` and `$_id` + themselves. ### Changed diff --git a/README.md b/README.md index f4263a8..6c242cd 100644 --- a/README.md +++ b/README.md @@ -150,6 +150,22 @@ this page". with `fh_conf('FH_CSRF_PROTECTION', false)` before the first form is created (the `fh_conf()` values become constants then). +**Copy a record** (4.8). `setCopyOf()` shows a record and saves it as a new one: + +```php +$form = new dbFormHandler(); +if (($_GET['funktion'] ?? '') == 'copy') +{ + $form->setCopyOf($_GET['id']); // before dbInfo() / setConnectionResource() +} +$form->setConnectionResource($conn, 'news'); +``` + +Call it on every request of the form, also when it is posted. The primary key and the +files of the uploadfields are not copied, the values of the junction tables are. +`isCopy()` and `getCopyOfId()` tell if and which record is copied, `getEditId()` is +`null`, and after saving `getRecordId()` returns the id of the new record. + **The database after saving** (4.8). `$form->getDatabaseAdapter()` returns the `DatabaseAdapter` of a `dbFormHandler`, also after `flush()` saved the record; before 4.8 it was unset after saving. diff --git a/src/class.dbFormHandler.php b/src/class.dbFormHandler.php index 6846551..133a973 100644 --- a/src/class.dbFormHandler.php +++ b/src/class.dbFormHandler.php @@ -35,6 +35,8 @@ class dbFormHandler extends FormHandler private string $_editName = ''; /** the id the INSERT gave the new record (an array with more primary keys) */ private mixed $_newId = null; + /** @var array|null the id(s) of the record which is copied (setCopyOf()) */ + private ?array $_copyOf = null; // public /** @var bool if the form is an insert-form */ @@ -78,6 +80,12 @@ public function __construct($name = null, $action = null, $extra = null) */ private function initFHEditName(): void { + // a copy does not use the id from the URL + if ($this->isCopy()) + { + return; + } + // initialisation $this->insert = !isset($_GET[$this->_editName]); $this->edit = !$this->insert; @@ -124,7 +132,66 @@ public function setEditName($sEditName) */ public function getEditId(): mixed { - return $this->edit && sizeof($this->_id) > 0 ? reset($this->_id) : null; + return $this->edit && !$this->isCopy() && sizeof($this->_id) > 0 ? reset($this->_id) : null; + } + + /** + * dbFormHandler::setCopyOf() + * + * Copy a record: the form shows the record with the given id and saves it as a new + * record. The primary keys and the files of the uploadfields are not copied, the + * values of the junction tables are. Until flush() the form loads the values like an + * edit form ($edit is true), flush() saves them with an INSERT. + * + * Call it before dbInfo(), setConnectionResource() and setDatabaseAdapter(), on every + * request of the form (also when it is posted). + * + * @param mixed $id the id of the record (an array with more primary keys) + * @return void + */ + public function setCopyOf(mixed $id): void + { + if (isset($this->_table) || isset($this->_db)) + { + throw new FormHandlerException( + 'setCopyOf() has to be called before dbInfo(), setConnectionResource() and setDatabaseAdapter()!' + ); + } + + $ids = is_array($id) ? array_values($id) : array($id); + if (sizeof($ids) == 0 || in_array(null, $ids, true) || in_array('', $ids, true)) + { + throw new FormHandlerException('setCopyOf() needs the id of the record to copy!'); + } + + $this->_copyOf = $ids; + $this->_id = $ids; + $this->insert = true; + $this->edit = true; + } + + /** + * dbFormHandler::isCopy() + * + * Return if the form copies a record (setCopyOf()) + * + * @return bool + */ + public function isCopy(): bool + { + return !is_null($this->_copyOf); + } + + /** + * dbFormHandler::getCopyOfId() + * + * Return the id of the record which is copied (the first one when it has more) + * + * @return mixed the id, or null when the form does not copy a record + */ + public function getCopyOfId(): mixed + { + return $this->isCopy() ? reset($this->_copyOf) : null; } /** @@ -895,8 +962,15 @@ public function flush($return = false) $oAjaxValidator->CreateObservers($this, $this->_fields, $this->_customMsg); } - // {id} in the filename of an edit form: the id is known (not before the flush, a copy - // of a record changes to an insert form here). A new record gets it after the INSERT. + // a copy has loaded the record like an edit form, it is saved as a new record + if ($this->isCopy()) + { + $this->_id = array(); + $this->edit = false; + } + + // {id} in the filename of an edit form: the id is known. A new record (and a copy) + // gets it after the INSERT. if ($this->edit) { foreach ($this->_getIdUploads() as $name) @@ -1243,7 +1317,8 @@ private function _checkUniqueFields(): bool continue; } - $where = $this->_getWhereClause('<>', $extra); + // a copy (before flush()) is a new record: no record is excluded + $where = $this->_getWhereClause('<>', $extra, '=', $this->isCopy() ? array() : null); // is there a selection filter ? if (sizeof($extra) > 0) @@ -1393,6 +1468,15 @@ protected function _loadDbData() if (count($this->_dbData) == 0) { $this->_dbData = $this->_db->getRecord($sql); + + // a copy gets its own primary key + if ($this->isCopy()) + { + foreach ($this->_db->getPrKeys($this->_table) as $key) + { + unset($this->_dbData[$key]); + } + } } if ($this->edit) @@ -1414,6 +1498,11 @@ protected function _loadDbData() } } } + // the record to copy has to exist + else if ($this->isCopy()) + { + throw new FormHandlerException('Try to copy a none existing record!'); + } // no records found, auto insert ? else if (FH_AUTO_INSERT) { @@ -1905,17 +1994,18 @@ private function _getQuery(?string $table, array $data, array $sqlFields, bool $ * @param string $operator The operator used for the primary key's and their values * @param array $extra: extra where clause arguments * @param string $extraOperator: operator used for the extra arguments + * @param array|null $record: the primary key values (default the ones of the record) * @return string * @access public * @author Teye Heimans */ - private function _getWhereClause(string $operator = '=', array $extra = array(), string $extraOperator = '='): string + private function _getWhereClause(string $operator = '=', array $extra = array(), string $extraOperator = '=', ?array $record = null): string { // get the primary key fields from the table $keys = $this->_db->getPrKeys($this->_table); // get the values for the key fields - $record = $this->_id; + $record = $record ?? $this->_id; // walk untill the field's or values are all handled $size1 = sizeof($record); diff --git a/src/fields/class.UploadField.php b/src/fields/class.UploadField.php index 452f02f..6891ae6 100644 --- a/src/fields/class.UploadField.php +++ b/src/fields/class.UploadField.php @@ -144,7 +144,7 @@ public function __construct(&$oForm, $sName, $aConfig) { // if this is an edit form and no value is given to the field, // keep the existing value thats in the database - if (isset($oForm->edit) && $oForm->edit && isset($oForm->_dbData[$sName])) + if ($this->_loadsRecord() && isset($oForm->_dbData[$sName])) { $this->setValue($oForm->_dbData[$sName]); } @@ -156,7 +156,7 @@ public function __construct(&$oForm, $sName, $aConfig) $this->setValue($_POST[$sName]); } // edit form? - elseif ((isset($oForm->edit) && $oForm->edit)) + elseif ($this->_loadsRecord()) { // value known from the database? if (isset($oForm->_dbData[$sName])) @@ -166,7 +166,7 @@ public function __construct(&$oForm, $sName, $aConfig) } } // load database value if exists - elseif ((isset($oForm->edit) && $oForm->edit)) + elseif ($this->_loadsRecord()) { if (isset($oForm->_dbData[$sName])) { @@ -175,7 +175,7 @@ public function __construct(&$oForm, $sName, $aConfig) } // if no file is uploaded and it is a edit form, dont overwrite the current value - if (!$this->isUploaded() && (isset($oForm->edit) && $oForm->edit) && $oForm->isPosted()) + if (!$this->isUploaded() && $this->_loadsRecord() && $oForm->isPosted()) { if (property_exists($this->_oForm, '_dontSave')) { @@ -235,6 +235,20 @@ public function setValue($sFilename) $this->_sFilename = $sFilename; } + /** + * UploadField::_loadsRecord() + * + * Return if the field keeps the filename of the record: on an edit form, but not on a + * copy of a record (both records would use the same file) + * + * @return bool + */ + private function _loadsRecord(): bool + { + return isset($this->_oForm->edit) && $this->_oForm->edit && + !($this->_oForm instanceof dbFormHandler && $this->_oForm->isCopy()); + } + /** * UploadField::getSavePath() * diff --git a/tests/uts/dbFormhandler_CopyTest.php b/tests/uts/dbFormhandler_CopyTest.php index f07237b..ed4d85f 100644 --- a/tests/uts/dbFormhandler_CopyTest.php +++ b/tests/uts/dbFormhandler_CopyTest.php @@ -3,51 +3,33 @@ declare(strict_types=1); /** - * Copies a record like the projects using FormHandler do (?id=...&funktion=copy): - * the form loads the record like an edit form and saves it as a new record. + * Copies a record with setCopyOf(): the form loads the record like an edit form and + * saves it as a new record. */ -class copyTest_dbFormHandler extends dbFormHandler +final class dbFormhandler_CopyTest extends dbFormhandlerTestCase { - private bool $_isCopy = false; - - public function __construct($name = null, $action = null, $extra = null) + private function createForm(mixed $copyOf = "100"): dbFormHandler { - parent::__construct($name, $action, $extra); - - if (isset($_GET['funktion']) && $_GET['funktion'] == "copy") - { - $this->_isCopy = true; - $this->insert = true; - } - } - - public function flush($return = false) - { - if ($this->_isCopy) - { - $this->_id = array(); - $this->edit = false; - } + $form = new dbFormHandler(); + $form->setCopyOf($copyOf); + $this->setConnectedTable($form, "test"); - return parent::flush($return); + return $form; } -} -final class dbFormhandler_CopyTest extends dbFormhandlerTestCase -{ public function test_copy_showsTheRecord(): void { $this->insertRecord(['id' => 100, 'textNullable' => 'text1', 'textNotNullable' => 'text2']); - $_GET['id'] = "100"; - $_GET['funktion'] = "copy"; - - $form = new copyTest_dbFormHandler(); - $this->setConnectedTable($form, "test"); - + $form = $this->createForm(); $form->textField("TextNullable", "textNullable"); $form->textField("TextNotNullable", "textNotNullable"); + static::assertTrue($form->isCopy()); + static::assertEquals(100, $form->getCopyOfId()); + static::assertNull($form->getEditId()); + static::assertNull($form->getRecordId()); + static::assertFormFlushContains($form, [ 'input type="text" name="textNullable" id="textNullable" value="text1"', 'input type="text" name="textNotNullable" id="textNotNullable" value="text2"' @@ -61,12 +43,10 @@ public function test_copy_savesANewRecord(): void $_POST['FormHandler_submit'] = "1"; $_POST['textNullable'] = "copy1"; $_POST['textNotNullable'] = "copy2"; + // the URL of a copy usually has the id too (?id=100&funktion=copy) $_GET['id'] = "100"; - $_GET['funktion'] = "copy"; - - $form = new copyTest_dbFormHandler(); - $this->setConnectedTable($form, "test"); + $form = $this->createForm(); $form->textField("TextNullable", "textNullable"); $form->textField("TextNotNullable", "textNotNullable"); @@ -77,25 +57,116 @@ public function test_copy_savesANewRecord(): void $this->assertRecordCount(2); $this->assertRecord(100, ['textNullable' => 'text1', 'textNotNullable' => 'text2']); $this->assertRecord(101, ['textNullable' => 'copy1', 'textNotNullable' => 'copy2']); + + static::assertTrue($form->isCopy()); + static::assertEquals(100, $form->getCopyOfId()); + static::assertNull($form->getEditId()); + static::assertEquals(101, $form->getNewId()); + static::assertEquals(101, $form->getRecordId()); + } + + public function test_copy_primaryKeyIsNotCopied(): void + { + $this->insertRecord(['id' => 100, 'textNullable' => 'text1']); + + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "copy1"; + + $form = $this->createForm(); + $form->textField("TextNullable", "textNullable"); + + // the values of the record without the primary key + static::assertArrayNotHasKey('id', $form->_dbData); + static::assertEquals('text1', $form->getDBValue('textNullable')); + + static::assertFlush($form); + + $this->assertRecordCount(2); + $this->assertRecord(101, ['textNullable' => 'copy1']); } - public function test_copy_newId(): void + public function test_copy_postedWithAnError(): void { $this->insertRecord(['id' => 100, 'textNullable' => 'text1', 'textNotNullable' => 'text2']); $_POST['FormHandler_submit'] = "1"; $_POST['textNullable'] = "copy1"; - $_GET['id'] = "100"; - $_GET['funktion'] = "copy"; + $_POST['textNotNullable'] = ""; - $form = new copyTest_dbFormHandler(); - $this->setConnectedTable($form, "test"); + $form = $this->createForm(); + $form->textField("TextNullable", "textNullable"); + $form->textField("TextNotNullable", "textNotNullable", FH_NOT_EMPTY); + static::assertFormFlushContains($form, [ + 'input type="text" name="textNullable" id="textNullable" value="copy1"', + 'error_textNotNullable' + ]); + + // still a copy, nothing saved + static::assertTrue($form->isCopy()); + static::assertNull($form->getEditId()); + static::assertNull($form->getRecordId()); + $this->assertRecordCount(1); + $this->assertRecord(100, ['textNullable' => 'text1', 'textNotNullable' => 'text2']); + } + + public function test_copy_uniqueField(): void + { + $this->getConnection()->executeStatement('CREATE UNIQUE INDEX test_text ON test (textNullable)'); + $this->insertRecord(['id' => 100, 'textNullable' => 'text1']); + + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "text1"; + + $form = $this->createForm(); $form->textField("TextNullable", "textNullable"); - static::assertFlush($form); + // the value of the original is used already: before and in flush() + static::assertFalse($form->isCorrect()); + $form->flush(true); + $this->assertRecordCount(1); + } - static::assertEquals(101, $form->getNewId()); - static::assertEquals(101, $form->getRecordId()); + public function test_copy_noneExistingRecord(): void + { + $form = new dbFormHandler(); + $form->setCopyOf(100); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('Try to copy a none existing record!'); + + $this->setConnectedTable($form, "test"); + } + + public function test_copy_afterTheConnection(): void + { + $form = new dbFormHandler(); + $this->setConnectedTable($form, "test"); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('setCopyOf() has to be called before dbInfo(), setConnectionResource() and setDatabaseAdapter()!'); + + $form->setCopyOf(100); + } + + public function test_copy_noId(): void + { + $form = new dbFormHandler(); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('setCopyOf() needs the id of the record to copy!'); + + $form->setCopyOf(''); + } + + public function test_noCopy(): void + { + $_GET['id'] = "100"; + + $form = new dbFormHandler(); + + static::assertFalse($form->isCopy()); + static::assertNull($form->getCopyOfId()); + static::assertEquals(100, $form->getEditId()); } } diff --git a/tests/uts/dbFormhandler_JunctionTableTest.php b/tests/uts/dbFormhandler_JunctionTableTest.php index 0603615..e1504d4 100644 --- a/tests/uts/dbFormhandler_JunctionTableTest.php +++ b/tests/uts/dbFormhandler_JunctionTableTest.php @@ -17,9 +17,13 @@ protected function setUp(): void ); } - private function createForm(): dbFormHandler + private function createForm(mixed $copyOf = null): dbFormHandler { $form = new dbFormHandler(); + if (!is_null($copyOf)) + { + $form->setCopyOf($copyOf); + } $this->setConnectedTable($form, "test"); $form->textField("TextNullable", "textNullable"); @@ -77,6 +81,31 @@ public function test_update(): void static::assertEquals([[5, '1'], [6, '1']], $this->getJunction()); } + public function test_copy(): void + { + $this->insertRecord(['id' => 5, 'textNullable' => 'text']); + $this->getConnection()->executeStatement("INSERT INTO test_options VALUES (5, '2'), (5, '3')"); + + // the copy shows the values of the original + static::assertEquals(['2', '3'], $this->createForm(5)->getValue("option_id")); + } + + public function test_copy_save(): void + { + $this->insertRecord(['id' => 5, 'textNullable' => 'text']); + $this->getConnection()->executeStatement("INSERT INTO test_options VALUES (5, '2'), (5, '3')"); + + $_POST['FormHandler_submit'] = "1"; + $_POST['textNullable'] = "copy"; + $_POST['option_id'] = ['2', '3']; + + static::assertEmpty($this->createForm(5)->flush(true)); + + // the copy gets its own rows, the ones of the original are kept + $this->assertSavedId(6); + static::assertEquals([[5, '2'], [5, '3'], [6, '2'], [6, '3']], $this->getJunction()); + } + public function test_insert_quotesInValue(): void { $_POST['FormHandler_submit'] = "1"; diff --git a/tests/uts/dbFormhandler_UploadIdTest.php b/tests/uts/dbFormhandler_UploadIdTest.php index b3e031e..a1b8d49 100644 --- a/tests/uts/dbFormhandler_UploadIdTest.php +++ b/tests/uts/dbFormhandler_UploadIdTest.php @@ -19,20 +19,10 @@ protected function _moveUploadedFile($sFrom, $sTo) } /** - * A form whose uploadField() creates an uploadIdTest_UploadField, and which copies a - * record like the projects do (?id=...&funktion=copy, see dbFormhandler_CopyTest) + * A form whose uploadField() creates an uploadIdTest_UploadField */ class uploadIdTest_dbFormHandler extends dbFormHandler { - private bool $_isCopy = false; - - public function __construct($name = null, $action = null, $extra = null) - { - parent::__construct($name, $action, $extra); - - $this->_isCopy = isset($_GET['funktion']) && $_GET['funktion'] == "copy"; - } - public function uploadField($title, $name, $config = array(), $validator = null, $extra = null, $alertOverwrite = null) { $fld = new uploadIdTest_UploadField($this, $name, $config); @@ -44,18 +34,6 @@ public function uploadField($title, $name, $config = array(), $validator = null, $this->_registerField($name, $fld, $title); $this->_upload[] = $name; } - - public function flush($return = false) - { - if ($this->_isCopy) - { - $this->_id = array(); - $this->insert = true; - $this->edit = false; - } - - return parent::flush($return); - } } final class dbFormhandler_UploadIdTest extends dbFormhandlerTestCase @@ -108,9 +86,13 @@ public function callback_onCorrect(array $data, FormHandler $form): bool /** * a form for the table "spieler" with the uploadfield "bild" (posted with test.jpg) */ - private function createForm(array $config = array()): uploadIdTest_dbFormHandler + private function createForm(array $config = array(), mixed $copyOf = null): uploadIdTest_dbFormHandler { $form = new uploadIdTest_dbFormHandler(); + if (!is_null($copyOf)) + { + $form->setCopyOf($copyOf); + } $form->setConnectionResource($this->getConnection(), 'spieler'); $form->textField("Name", "name"); @@ -259,11 +241,9 @@ public function test_copy(): void { $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old', 'bild' => 'spieler_100.jpg']); static::assertTrue(file_put_contents("{$this->_tempPath}/spieler_100.jpg", 'original') > 0); - $_GET['id'] = "100"; - $_GET['funktion'] = "copy"; $_POST['name'] = "copy"; - $form = $this->createForm(); + $form = $this->createForm(array(), 100); static::assertFlush($form); @@ -274,6 +254,37 @@ public function test_copy(): void $this->assertFiles(['spieler_100.jpg', 'spieler_101.jpg']); } + public function test_copy_withoutFile(): void + { + $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old', 'bild' => 'spieler_100.jpg']); + static::assertTrue(file_put_contents("{$this->_tempPath}/spieler_100.jpg", 'original') > 0); + $_POST['name'] = "copy"; + $_FILES['bild'] = ['error' => UPLOAD_ERR_NO_FILE, 'name' => '', 'size' => 0, 'tmp_name' => '', 'type' => '']; + + $form = $this->createForm(array('name' => 'spieler'), 100); + + // the copy does not take the file of the original (both records would use it) + static::assertSame('', $form->getValue('bild')); + + static::assertFlush($form); + + static::assertNull($this->getBild(101)); + static::assertSame('spieler_100.jpg', $this->getBild(100)); + $this->assertFiles(['spieler_100.jpg']); + } + + public function test_copy_showsNoFile(): void + { + $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old', 'bild' => 'spieler_100.jpg']); + unset($_POST['FormHandler_submit']); + $_FILES = array(); + + $form = $this->createForm(array(), 100); + + static::assertSame('', $form->getValue('bild')); + static::assertStringNotContainsString('spieler_100.jpg', (string) $form->flush(true)); + } + public function test_invalidId(): void { $form = $this->createForm(); @@ -316,9 +327,9 @@ public function test_noTable(): void /** * a form whose upload keeps its name ("upload.jpg") and whose resizeImage() uses {id} */ - private function createResizeForm(): uploadIdTest_dbFormHandler + private function createResizeForm(mixed $copyOf = null): uploadIdTest_dbFormHandler { - $form = $this->createForm(['name' => 'upload']); + $form = $this->createForm(['name' => 'upload'], $copyOf); $form->resizeImage('bild', $this->_tempPath . '/spieler_{id}.jpg', 30, 30); return $form; @@ -364,11 +375,9 @@ public function test_resize_edit(): void public function test_resize_copy(): void { $this->getConnection()->insert('spieler', ['id' => 100, 'name' => 'old']); - $_GET['id'] = "100"; - $_GET['funktion'] = "copy"; $_POST['name'] = "copy"; - $form = $this->createResizeForm(); + $form = $this->createResizeForm(100); static::assertFlush($form); From 63dcf2be54b5cb9067f88c66eb4d02125960842a Mon Sep 17 00:00:00 2001 From: tombrain Date: Sat, 31 Oct 2026 22:05:13 +0100 Subject: [PATCH 7/7] feat: FormHandler::joinNextFields() shows the next fields in one row --- CHANGELOG.md | 7 + README.md | 9 + src/class.FormHandler.php | 145 ++++++++++-- tests/uts/formhandler_JoinNextFieldsTest.php | 222 +++++++++++++++++++ 4 files changed, 359 insertions(+), 24 deletions(-) create mode 100644 tests/uts/formhandler_JoinNextFieldsTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 134fa7e..e684291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,13 @@ and the project follows [Semantic Versioning](https://semver.org/). fields are checked like on a new record. A record which does not exist throws a `FormHandlerException`. Projects no longer have to change `$insert`, `$edit` and `$_id` themselves. +- `FormHandler::joinNextFields($count, $separator = ' ')` shows the next fields in one row + of the mask, e.g. a select field and a text field. The title, help and value of the + first field fill the mask, the fields are put together with the separator (HTML), and + the error messages of all of them are shown. Hidden fields are not counted; HTML, a + line, a fieldset, a new mask or page ends the row earlier. The row uses the mask once, + so a mask set with `setMask()` applies to the fields after it as before. Projects no + longer have to change `%field%` in the mask themselves. ### Changed diff --git a/README.md b/README.md index 6c242cd..64cb511 100644 --- a/README.md +++ b/README.md @@ -166,6 +166,15 @@ files of the uploadfields are not copied, the values of the junction tables are. `isCopy()` and `getCopyOfId()` tell if and which record is copied, `getEditId()` is `null`, and after saving `getRecordId()` returns the id of the new record. +**More fields in one row** (4.8). `joinNextFields()` puts the next fields into one row of +the mask, with the title and help of the first one and the error messages of all: + +```php +$form->joinNextFields(2, ' / '); +$form->selectField('Opponent', 'gegner', $gegner); +$form->textField('Team', 'team'); +``` + **The database after saving** (4.8). `$form->getDatabaseAdapter()` returns the `DatabaseAdapter` of a `dbFormHandler`, also after `flush()` saved the record; before 4.8 it was unset after saving. diff --git a/src/class.FormHandler.php b/src/class.FormHandler.php index b8b5c9f..283c82c 100644 --- a/src/class.FormHandler.php +++ b/src/class.FormHandler.php @@ -1752,6 +1752,29 @@ public function setMask($mask = null, $repeat = true) $this->_fields[] = array('__MASK__', array($mask, $repeat)); } + /** + * FormHandler::joinNextFields() + * + * Show the next fields in one row of the mask: the title, help and value of the + * first field are used, the fields are put together with the separator and the error + * messages of all of them are shown. Hidden fields are not counted. HTML, a line, a + * fieldset, a new mask or page ends the row earlier. The mask is used once for the + * row, so the mask of the fields after it is the same as without joined fields. + * + * @param int $count: the number of fields (at least 2) + * @param string $separator: the HTML between the fields + * @return void + */ + public function joinNextFields(int $count, string $separator = ' '): void + { + if ($count < 2) + { + throw new FormHandlerException('joinNextFields() needs at least 2 fields!'); + } + + $this->_fields[] = array('__JOIN__', array($count, $separator)); + } + /** * Get the file contents by including it, to enable parsing of php files * @@ -3883,6 +3906,53 @@ protected function _handleUploads($names = null) } } + /** + * FormHandler::_fillRow() + * + * Fill the mask with the values of a field (or of joined fields) and return the HTML + * when the mask is full + * + * @param MaskLoader $mask: the mask loader + * @param array $replace: the values for the search strings of the mask + * @param bool $hasError: if the field has an error message + * @param bool|int $repeat: if the mask is repeated (an integer is counted down) + * @return string the HTML, empty when the mask is not full yet + */ + private function _fillRow(MaskLoader $mask, array $replace, bool $hasError, bool|int &$repeat): string + { + // fill the mask + $html = $mask->fill($replace); + + // added 07-01-2009 in order to specify which element should get the error class + if ($this->isPosted() == true and $hasError) + { + $html = $this->parse_error_style($html); + } + else + { + $html = str_replace('%error_style%', '', $html); + } + + // is the mask filled ? + if (!$html) + { + return ''; + } + + // if we don't have to repeat the current mask, use the original + if (!$repeat) + { + $this->_mask = FH_DEFAULT_ROW_MASK; + } + // if we have to repeat the mask, repeat it and countdown + else if (is_numeric($repeat)) + { + $repeat--; + } + + return str_replace('____FH-percent____', '%', $html); + } + /** * FormHandler::_getForm() * @@ -3957,6 +4027,7 @@ protected function _getForm($iDisplayPage = null) $buffer = array(); $repeat = true; $page = 1; + $join = null; // start a new mask loader $mask = new MaskLoader(); @@ -3978,8 +4049,31 @@ protected function _getForm($iDisplayPage = null) // walk trought the fields array foreach ($this->_fields as $id => $field) { + // HTML, a fieldset, a new mask or page ends a row of joined fields + if (!is_null($join) && in_array($field[0], array('__PAGE__', '__MASK__', '__JOIN__', '__HTML__', '__LINE__', '__FIELDSET__', '__FIELDSET-END__'), true)) + { + if (!is_null($join['replace'])) + { + $form .= $this->_fillRow($mask, $join['replace'], $join['error'], $repeat); + } + $join = null; + } + switch ($field[0]) { + // the next fields share a row + case '__JOIN__': + if ($page == $iDisplayPage) + { + $join = array( + 'count' => $field[1][0], + 'separator' => $field[1][1], + 'replace' => null, + 'error' => false + ); + } + break; + // multiple pages in this form case '__PAGE__': # why did we stop at the current page ? @@ -4191,41 +4285,44 @@ protected function _getForm($iDisplayPage = null) /* %help% */ $help ); - // fill the mask - $html = $mask->fill($replace); - - // added 07-01-2009 in order to specify which element should get the error class - if ($this->isPosted() == true and $error != '') - { - $html = $this->parse_error_style($html); - } - else - { - $html = str_replace('%error_style%', '', $html); - } - - // is the mask filled ? - if ($html) + // joined fields (joinNextFields()): the first one fills the mask, + // the others only add their field and error message + if (!is_null($join)) { - // add it the the form HTML - $form .= str_replace('____FH-percent____', '%', $html); - - // if we don't have to repeat the current mask, use the original - if (!$repeat) + if (is_null($join['replace'])) + { + $join['replace'] = $replace; + } + else { - $this->_mask = FH_DEFAULT_ROW_MASK; + $join['replace'][0] .= str_replace('%', '____FH-percent____', $join['separator']) . $fld; + $join['replace'][1] .= ($join['replace'][1] != '' && $error != '' ? ' ' : '') . $error; } - // if we have to repeat the mask, repeat it and countdown - else if (is_numeric($repeat)) + $join['error'] = $join['error'] || $error != ''; + + // wait for the next field of the row + if (--$join['count'] > 0) { - $repeat--; + break; } + + $form .= $this->_fillRow($mask, $join['replace'], $join['error'], $repeat); + $join = null; + break; } + + $form .= $this->_fillRow($mask, $replace, $error != '', $repeat); } break; } } + // a row of joined fields with fewer fields than said + if (!is_null($join) && !is_null($join['replace'])) + { + $form .= $this->_fillRow($mask, $join['replace'], $join['error'], $repeat); + } + // add the page number to the forms HTML if ($this->_pageCounter > 1) { diff --git a/tests/uts/formhandler_JoinNextFieldsTest.php b/tests/uts/formhandler_JoinNextFieldsTest.php new file mode 100644 index 0000000..8785784 --- /dev/null +++ b/tests/uts/formhandler_JoinNextFieldsTest.php @@ -0,0 +1,222 @@ +'; + private const FIELD_B = ''; + private const FIELD_C = ''; + private const FIELD_D = ''; + + public function test_twoFields(): void + { + $form = new FormHandler(); + + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->textField("C", "c"); + + static::assertFormFlushContains($form, [ + 'A:' . self::FIELD_A . ' ' . self::FIELD_B . 'error_a', + 'C:' . self::FIELD_C . 'error_c' + ]); + } + + public function test_threeFields_separator(): void + { + $form = new FormHandler(); + + $form->joinNextFields(3, ' / '); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->textField("C", "c"); + $form->textField("D", "d"); + + $t = static::assertFormFlushContains($form, [ + 'A:' . self::FIELD_A . ' / ' . self::FIELD_B . ' / ' . self::FIELD_C . 'error_a', + 'D:' . self::FIELD_D . 'error_d' + ]); + static::assertStringNotContainsString('B:', $t); + static::assertStringNotContainsString('C:', $t); + } + + public function test_percentInSeparator(): void + { + $form = new FormHandler(); + + $form->joinNextFields(2, ' %title% '); + $form->textField("A", "a"); + $form->textField("B", "b"); + + static::assertFormFlushContains($form, 'A:' . self::FIELD_A . ' %title% ' . self::FIELD_B . 'error_a'); + } + + public function test_errorOfTheSecondField(): void + { + $_POST['FormHandler_submit'] = "1"; + $_POST['a'] = ""; + $_POST['b'] = ""; + + $form = new FormHandler(); + + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->textField("B", "b", FH_NOT_EMPTY); + + $t = static::assertFormFlushContains($form, 'error_a'); + static::assertStringContainsString('', $t); + } + + public function test_errorsOfBothFields(): void + { + $_POST['FormHandler_submit'] = "1"; + $_POST['a'] = ""; + $_POST['b'] = ""; + + $form = new FormHandler(); + + $form->joinNextFields(2); + $form->textField("A", "a", FH_NOT_EMPTY); + $form->textField("B", "b", FH_NOT_EMPTY); + + static::assertFormFlushContains($form, [ + 'error_a', + ' ' + ]); + } + + public function test_viewMode(): void + { + $form = new FormHandler(); + + $form->joinNextFields(2, ' - '); + $form->textField("A", "a"); + $form->setValue("a", "valueA"); + $form->textField("B", "b"); + $form->setValue("b", "valueB"); + $form->enableViewMode(); + + static::assertFormFlushContains($form, 'A:valueA - valueBerror_a'); + } + + public function test_hiddenFieldIsNotCounted(): void + { + $form = new FormHandler(); + + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->hiddenField("h", "hidden"); + $form->textField("B", "b"); + + static::assertFormFlushContains($form, 'A:' . self::FIELD_A . ' ' . self::FIELD_B . 'error_a'); + } + + public function test_htmlEndsTheRow(): void + { + $form = new FormHandler(); + + $form->joinNextFields(3); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->addHTML("HTML"); + $form->textField("C", "c"); + + static::assertFormFlushContains($form, [ + 'A:' . self::FIELD_A . ' ' . self::FIELD_B . 'error_aHTML', + 'C:' . self::FIELD_C . 'error_c' + ]); + } + + public function test_lastRowWithFewerFields(): void + { + $form = new FormHandler(); + + $form->joinNextFields(3); + $form->textField("A", "a"); + $form->textField("B", "b"); + + static::assertFormFlushContains($form, 'A:' . self::FIELD_A . ' ' . self::FIELD_B . 'error_a'); + } + + public function test_maskRepeated(): void + { + $form = new FormHandler(); + + $form->setMask("[%title%|%field%]", true); + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->textField("C", "c"); + $form->joinNextFields(2); + $form->textField("D", "d"); + $form->textField("E", "e"); + + static::assertFormFlushContains($form, [ + '[A|' . self::FIELD_A . ' ' . self::FIELD_B . ']', + '[C|' . self::FIELD_C . ']', + '[D|' . self::FIELD_D . ' ]' + ]); + } + + public function test_maskOnce(): void + { + $form = new FormHandler(); + + // the row uses the mask once, the next field the default mask + $form->setMask("[%title%|%field%]", false); + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->textField("C", "c"); + + static::assertFormFlushContains($form, [ + '[A|' . self::FIELD_A . ' ' . self::FIELD_B . ']', + 'C:' . self::FIELD_C . 'error_c' + ]); + } + + public function test_maskAfterTheRow(): void + { + $form = new FormHandler(); + + $form->joinNextFields(2); + $form->textField("A", "a"); + $form->textField("B", "b"); + $form->setMask("[%title%|%field%]", false); + $form->textField("C", "c"); + $form->textField("D", "d"); + + static::assertFormFlushContains($form, [ + 'A:' . self::FIELD_A . ' ' . self::FIELD_B . 'error_a', + '[C|' . self::FIELD_C . ']', + 'D:' . self::FIELD_D . 'error_d' + ]); + } + + public function test_newPage(): void + { + $form = new FormHandler(); + + $form->textField("A", "a"); + $form->newPage(); + $form->joinNextFields(2); + $form->textField("B", "b"); + $form->textField("C", "c"); + + // page 1: the row of page 2 does not join a field of page 1 + $t = static::assertFormFlushContains($form, 'A:' . self::FIELD_A . 'error_a'); + static::assertStringNotContainsString(self::FIELD_A . ' ', $t); + } + + public function test_tooFewFields(): void + { + $form = new FormHandler(); + + $this->expectException(FormHandlerException::class); + $this->expectExceptionMessage('joinNextFields() needs at least 2 fields!'); + + $form->joinNextFields(1); + } +}