[TikiWiki-commits] [Git][tikiwiki/tiki][master] [FIX][REF] notepad: Fix note update bug and improve code quality

Benoit Grégoire pushed to branch master at Tiki Wiki CMS Groupware / Tiki


Commits:
0d0b547c by Sacha Pignot at 2026-01-30T20:33:55+00:00
[FIX][REF] notepad: Fix note update bug and improve code quality
---
* [FIX][REF] notepad: Fix get_note return type and simplify wikify conditionals
* Fix get_note() return type declaration to array|false matching actual behavior
* Refactor wikify/overwrite logic from three separate if blocks into if/elseif/else chain

* [REF] notepad: Clean up PHPDoc tags and simplify file upload handling
* Remove redundant @package/@subpackage tags from NotepadLib class
* Remove @see cross-reference from replace_note() in TikiLib
* Extract $_FILES variable before isset() check in tiki-notepad_list.php

* [FIX][REF] notepad: Fix created timestamp and add type declarations
* Fix replace_note() to only set 'created' timestamp on INSERT, not UPDATE
* Add PHP type declarations to NotepadLib methods
* Rename $note_id to $noteId for consistent camelCase naming in tiki-notepad_write.php

* [REF] notepad: Improve code quality and add PHPDoc documentation
* Add comprehensive PHPDoc comments to NotepadLib class methods
* Add type declarations to replace_note() method
* Fix replace_note() condition to explicitly check $noteId > 0
* Simplify conditionals with ternary operators and null coalescing
* Replace manual file reading with file_get_contents()
* Refactor nested if/else blocks to switch statements
* Flatten complex wiki page creation/update logic for readability
* Improve code organization with consistent whitespace

See merge request tikiwiki/tiki!9452

- - - - -


6 changed files:

- lib/notepad/notepadlib.php
- lib/tikilib.php
- tiki-notepad_get.php
- tiki-notepad_list.php
- tiki-notepad_read.php
- tiki-notepad_write.php


Changes:

=====================================
lib/notepad/notepadlib.php
=====================================
@@ -4,18 +4,33 @@
 //
 // All Rights Reserved. See copyright.txt for details and a complete list of authors.
 // Licensed under the GNU LESSER GENERAL PUBLIC LICENSE. See license.txt for details.
-//this script may only be included - so its better to die if called directly.
+// this script may only be included - so its better to die if called directly.
 if (str_contains($_SERVER["SCRIPT_NAME"], basename(__FILE__))) {
     header("location: index.php");
     exit;
 }
 
-/* Task properties:
-   user, taskId, title, description, date, status, priority, completed, percentage
-*/
+/**
+ * Library for managing user personal notes.
+ *
+ * Provides functionality for creating, reading, updating, and deleting
+ * personal text notes stored in the tiki_user_notes table.
+ * Notes are user-scoped (each user can only access their own notes).
+ *
+ * @see TikiLib::replace_note() For creating and updating notes
+ */
 class NotepadLib extends TikiLib
 {
-    public function get_note($user, $noteId)
+    /**
+     * Retrieve a single note by ID for a specific user.
+     *
+     * @param string $user    The username of the note owner
+     * @param int    $noteId  The unique identifier of the note
+     *
+     * @return array|false Note data array with keys (noteId, user, name, data, created,
+     *                     lastModif, size, parse_mode) or false if not found
+     */
+    public function get_note(string $user, int $noteId): array|false
     {
         $query = "select * from `tiki_user_notes` where `user`=? and `noteId`=?";
         $result = $this->query($query, [$user,(int)$noteId]);
@@ -23,22 +38,49 @@ class NotepadLib extends TikiLib
         return $res;
     }
 
-    public function set_note_parsing($user, $noteId, $mode)
+    /**
+     * Update the parsing mode for a note.
+     *
+     * @param string $user    The username of the note owner
+     * @param int    $noteId  The unique identifier of the note
+     * @param string $mode    The parse mode ('raw' for plain text, 'wiki' for wiki syntax)
+     *
+     * @return bool Always returns true
+     */
+    public function set_note_parsing(string $user, int $noteId, string $mode): bool
     {
         $query = "update `tiki_user_notes` set `parse_mode`=? where `user`=? and `noteId`=?";
         $this->query($query, [$mode,$user,(int)$noteId]);
         return true;
     }
 
-    public function remove_note($user, $noteId)
+    /**
+     * Delete a note for a specific user.
+     *
+     * @param string $user    The username of the note owner
+     * @param int    $noteId  The unique identifier of the note to delete
+     *
+     * @return void
+     */
+    public function remove_note(string $user, int $noteId): void
     {
         $query = "delete from `tiki_user_notes` where `user`=? and `noteId`=?";
         $this->query($query, [$user,(int)$noteId]);
     }
 
-    public function list_notes($user, $offset, $maxRecords, $sort_mode, $find)
+    /**
+     * List notes for a user with pagination, sorting, and optional search.
+     *
+     * @param string $user        The username of the note owner
+     * @param int    $offset      Number of records to skip (for pagination)
+     * @param int    $maxRecords  Maximum number of records to return
+     * @param string $sort_mode   Sort order (e.g., 'lastModif_desc', 'name_asc', 'created_desc')
+     * @param string $find        Optional search string to filter notes by name or content
+     *
+     * @return array{data: array, count: int} Array with 'data' (list of notes with calculated size) and 'count' (total number of matching notes)
+     */
+    public function list_notes(string $user, int $offset, int $maxRecords, string $sort_mode, string $find): array
     {
-
         $bindvars = [$user];
         if ($find) {
             $findesc = '%' . $find . '%';
@@ -67,4 +109,5 @@ class NotepadLib extends TikiLib
         return $retval;
     }
 }
+
 $notepadlib = new NotepadLib();


=====================================
lib/tikilib.php
=====================================
@@ -701,16 +701,22 @@ class TikiLib extends TikiDb_Bridge
         return false;
     }
 
-    // $noteId 0 means create a new note
     /**
-     * @param $user
-     * @param $noteId
-     * @param $name
-     * @param $data
-     * @param null $parse_mode
-     * @return mixed
+     * Create or update a user note in the notepad.
+     *
+     * Inserts a new note when $noteId is null/0, or updates an existing note when $noteId is provided.
+     * Automatically converts absolute links to relative links before storing.
+     * Sets both created and lastModif timestamps to current time.
+     *
+     * @param string      $user       The username of the note owner
+     * @param int|null    $noteId     The note ID to update, or null/0 to create a new note
+     * @param string      $name       The title/name of the note
+     * @param string      $data       The note content
+     * @param string|null $parse_mode The parsing mode ('raw' for plain text, 'wiki' for wiki syntax), or null to use default
+     *
+     * @return int The note ID (newly created ID for inserts, or the provided ID for updates)
      */
-    public function replace_note($user, $noteId, $name, $data, $parse_mode = null)
+    public function replace_note(string $user, ?int $noteId, string $name, string $data, ?string $parse_mode = null): int
     {
         $data = $this->convertAbsoluteLinksToRelative($data);
         $size = strlen($data);
@@ -719,7 +725,6 @@ class TikiLib extends TikiDb_Bridge
             'user' => $user,
             'name' => $name,
             'data' => $data,
-            'created' => $this->now,
             'lastModif' => $this->now,
             'size' => (int) $size,
             'parse_mode' => $parse_mode,
@@ -727,8 +732,9 @@ class TikiLib extends TikiDb_Bridge
 
         $userNotes = $this->table('tiki_user_notes');
         if ($noteId) {
-            $userNotes->update($queryData, ['noteId' => (int) $noteId,]);
+            $userNotes->update($queryData, ['noteId' => (int) $noteId]);
         } else {
+            $queryData['created'] = $this->now;
             $noteId = $userNotes->insert($queryData);
         }
 


=====================================
tiki-notepad_get.php
=====================================
@@ -17,16 +17,15 @@ include_once('lib/notepad/notepadlib.php');
 $access->check_feature('feature_notepad');
 $access->check_user($user);
 $access->check_permission('tiki_p_notepad');
+
 if (! isset($_REQUEST["noteId"])) {
     Feedback::errorAndDie(tra("No note indicated"), \Laminas\Http\Response::STATUS_CODE_409);
 }
 
-if (isset($_REQUEST["save"])) {
-    $disposition = "attachment";
-} else {
-    $disposition = "inline";
-}
+$disposition = isset($_REQUEST["save"]) ? "attachment" : "inline";
+
 $info = $notepadlib->get_note($user, $_REQUEST["noteId"]);
+
 header("Content-type: text/plain");
 header("Content-Disposition: $disposition; filename=note_" . urlencode($user) . '_' . $_REQUEST["noteId"] . ".txt;");
 echo $info['data'];


=====================================
tiki-notepad_list.php
=====================================
@@ -20,39 +20,41 @@ $inputConfiguration = [
         ],
     ],
 ];
+
 require_once('tiki-setup.php');
 include_once('lib/notepad/notepadlib.php');
 include_once('lib/userfiles/userfileslib.php');
 $access->check_feature('feature_notepad');
 $access->check_user($user);
 $access->check_permission('tiki_p_notepad');
-// Process upload here
+
+// Process file upload
 if (isset($_FILES['userfile1'])) {
-    if (is_uploaded_file($_FILES['userfile1']['tmp_name'])) {
+    $uploadedFile = $_FILES['userfile1'];
+
+    if (! is_uploaded_file($uploadedFile['tmp_name'])) {
+        Feedback::error($tikilib->uploaded_file_error($uploadedFile['error']));
+        // Continue to display the page with the error message
+    } else {
         $access->checkCsrf();
+
         $filegallib = TikiLib::lib('filegal');
         try {
-            $filegallib->assertUploadedFileIsSafe($_FILES['userfile1']['tmp_name'], $_FILES['userfile1']['name']);
+            $filegallib->assertUploadedFileIsSafe($uploadedFile['tmp_name'], $uploadedFile['name']);
         } catch (Exception $e) {
             Feedback::errorAndDie($e->getMessage(), \Laminas\Http\Response::STATUS_CODE_403);
         }
-        $fp = fopen($_FILES['userfile1']['tmp_name'], "rb");
-        $data = '';
-        while (! feof($fp)) {
-            $data .= fread($fp, 8192 * 16);
-        }
-        fclose($fp);
-        if (strlen($data) > 1000000) {
+
+        $maxNoteSize = 1000000; // 1 MB
+        $data = file_get_contents($uploadedFile['tmp_name']);
+        if (strlen($data) > $maxNoteSize) {
             Feedback::errorAndDie(tra("The file is too large"), \Laminas\Http\Response::STATUS_CODE_409);
         }
-        $size = $_FILES['userfile1']['size'];
-        $name = $_FILES['userfile1']['name'];
-        $type = $_FILES['userfile1']['type'];
-        $notepadlib->replace_note($user, 0, $name, $data);
-    } else {
-        Feedback::error($tikilib->uploaded_file_error($_FILES['userfile1']['error']));
+
+        $notepadlib->replace_note($user, 0, $uploadedFile['name'], $data);
     }
 }
+
 if (isset($_REQUEST["merge"])) {
     $access->checkCsrf();
     $merge = '';
@@ -74,39 +76,35 @@ if (isset($_REQUEST["merge"])) {
     // Now create the merged note
     $tikilib->replace_note($user, 0, $_REQUEST['merge_name'], $merge);
 }
+
 if (isset($_REQUEST["delete"]) && isset($_REQUEST["note"]) && $access->checkCsrf()) {
     foreach (array_keys($_REQUEST["note"]) as $note) {
         $notepadlib->remove_note($user, $note);
     }
 }
+
 $quota = $userfileslib->userfiles_quota($user);
 $limit = $prefs['userfiles_quota'] * 1024 * 1000;
 if ($limit == 0) {
     $limit = 999999999;
 }
+
 $percentage = ($quota / $limit) * 100;
 $cellsize = round($percentage / 100 * 200);
 if ($cellsize == 0) {
     $cellsize = 1;
 }
+
 $percentage = round($percentage);
 $smarty->assign('cellsize', $cellsize);
 $smarty->assign('percentage', $percentage);
-if (! isset($_REQUEST["sort_mode"])) {
-    $sort_mode = 'lastModif_desc';
-} else {
-    $sort_mode = $_REQUEST["sort_mode"];
-}
+$sort_mode = $_REQUEST["sort_mode"] ?? 'lastModif_desc';
 $offset = $_REQUEST["offset"] ?? 0;
 $smarty->assign_by_ref('offset', $offset);
 $find = $_REQUEST["find"] ?? '';
 $smarty->assign('find', $find);
 $smarty->assign_by_ref('sort_mode', $sort_mode);
-if (isset($_SESSION['thedate'])) {
-    $pdate = $_SESSION['thedate'];
-} else {
-    $pdate = $tikilib->now;
-}
+$pdate = $_SESSION['thedate'] ?? $tikilib->now;
 $channels = $notepadlib->list_notes($user, $offset, $maxRecords, $sort_mode, $find);
 $smarty->assign_by_ref('pages_count', $channels["count"]);
 $smarty->assign_by_ref('channels', $channels["data"]);


=====================================
tiki-notepad_read.php
=====================================
@@ -24,11 +24,14 @@ $inputConfiguration = [
         ],
     ],
 ];
+
 require_once('tiki-setup.php');
 include_once('lib/notepad/notepadlib.php');
+
 $access->check_feature('feature_notepad');
 $access->check_user($user);
 $access->check_permission('tiki_p_notepad');
+
 if (! isset($_REQUEST["noteId"])) {
     Feedback::errorAndDie(tra("No note indicated"), \Laminas\Http\Response::STATUS_CODE_400);
 }
@@ -47,23 +50,24 @@ if (isset($_REQUEST['wikify']) || isset($_REQUEST['over'])) {
     if (empty($_REQUEST['wiki_name'])) {
         Feedback::errorAndDie(tra("No name indicated for wiki page"), \Laminas\Http\Response::STATUS_CODE_400);
     }
-    if ($tikilib->page_exists($_REQUEST['wiki_name'])) {
-        if (isset($_REQUEST['over'])) {
-            $pageperms = $tikilib->get_perm_object($_REQUEST['wiki_name'], 'wiki page', '', false);
-            if ($pageperms["tiki_p_edit"] == 'y') {
-                $tikilib->update_page($_REQUEST['wiki_name'], $info['data'], tra('created from notepad'), $user, '127.0.1.1', $info['name']);
-            } else {
-                Feedback::errorAndDie(tra("You do not have permission to edit this page."), \Laminas\Http\Response::STATUS_CODE_401);
-            }
-        } else {
-            Feedback::errorAndDie(tra("Page already exists"), \Laminas\Http\Response::STATUS_CODE_409);
+    $pageExists = $tikilib->page_exists($_REQUEST['wiki_name']);
+
+    if ($pageExists && ! isset($_REQUEST['over'])) {
+        // Page exists but user didn't request overwrite
+        Feedback::errorAndDie(tra("Page already exists"), \Laminas\Http\Response::STATUS_CODE_409);
+    } elseif ($pageExists) {
+        // Page exists and user wants to overwrite
+        $pageperms = $tikilib->get_perm_object($_REQUEST['wiki_name'], 'wiki page', '', false);
+        if ($pageperms["tiki_p_edit"] != 'y') {
+            Feedback::errorAndDie(tra("You do not have permission to edit this page."), \Laminas\Http\Response::STATUS_CODE_401);
         }
+        $tikilib->update_page($_REQUEST['wiki_name'], $info['data'], tra('created from notepad'), $user, '127.0.1.1', $info['name']);
     } else {
-        if ($tiki_p_edit == 'y') {
-            $tikilib->create_page($_REQUEST['wiki_name'], 0, $info['data'], $tikilib->now, tra('created from notepad'), $user, $ip = '0.0.0.0', $info['name']);
-        } else {
+        // Page doesn't exist, create new
+        if ($tiki_p_edit != 'y') {
             Feedback::errorAndDie(tra("You do not have permission to edit this page."), \Laminas\Http\Response::STATUS_CODE_401);
         }
+        $tikilib->create_page($_REQUEST['wiki_name'], 0, $info['data'], $tikilib->now, tra('created from notepad'), $user, '0.0.0.0', $info['name']);
     }
 }
 
@@ -72,10 +76,12 @@ if ($tikilib->page_exists($info['name'])) {
 } else {
     $smarty->assign("wiki_exists", "n");
 }
+
 if (isset($_REQUEST['parse_mode']) and $_REQUEST['parse_mode'] != $info['parse_mode']) {
     $notepadlib->set_note_parsing($user, $_REQUEST['noteId'], $_REQUEST['parse_mode']);
     $info['parse_mode'] = $_REQUEST['parse_mode'];
 }
+
 if ($info['parse_mode'] == 'raw') {
     $info['parsed'] = nl2br(htmlspecialchars($info['data']));
     $smarty->assign('wysiwyg', 'n');
@@ -83,6 +89,7 @@ if ($info['parse_mode'] == 'raw') {
     include 'lib/setup/editmode.php';
     $info['parsed'] = TikiLib::lib('parser')->parse_data($info['data'], ['is_html' => $is_html]);
 }
+
 $smarty->assign('noteId', $_REQUEST["noteId"]);
 $smarty->assign('info', $info);
 include_once('tiki-section_options.php');


=====================================
tiki-notepad_write.php
=====================================
@@ -26,15 +26,18 @@ include_once('lib/notepad/notepadlib.php');
 $access->check_feature('feature_notepad');
 $access->check_user($user);
 $access->check_permission('tiki_p_notepad');
+
 if (isset($_REQUEST["remove"])) {
     $access->checkCsrf();
     $notepadlib->remove_note($user, $_REQUEST['remove']);
 }
 include 'lib/setup/editmode.php';
+
 if (isset($_REQUEST["noteId"])) {
-    $note_id = $_REQUEST["noteId"];
-    $smarty->assign('noteId', $note_id);
-    $info = $notepadlib->get_note($user, $note_id);
+    $noteId = $_REQUEST["noteId"];
+    $smarty->assign('noteId', $noteId);
+    $info = $notepadlib->get_note($user, $noteId);
+
     if ($info['parse_mode'] == 'raw') {
         $info['parsed'] = nl2br(htmlspecialchars($info['data']));
         $smarty->assign('wysiwyg', 'n');
@@ -47,10 +50,11 @@ if (isset($_REQUEST["noteId"])) {
     $info['data'] = '';
     $info['parse_mode'] = 'wiki';
 }
+
 if (isset($_REQUEST['save'])) {
     $access->checkCsrf();
     $noteId = $notepadlib->replace_note($user, $noteId ?? null, $_REQUEST["name"], $_REQUEST["data"], $_REQUEST["parse_mode"]);
-    header('location: tiki-notepad_read.php?noteId=' . $noteId);
+    header("location: tiki-notepad_read.php?noteId=$noteId");
     die;
 }
 



View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/0d0b547c3b1eea654c4fa15d0de67beeed496958

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/0d0b547c3b1eea654c4fa15d0de67beeed496958
You're receiving this email because of your account on gitlab.com.

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs