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

"ushindi bienvenu \(@usbbush\) via TikiWiki-cvs" <[email protected]>
Newsgroups gmane.comp.cms.tiki.cvs
Message-ID <69d158f3119bf_3b19049c9591e@gitlab-sidekiq-low-urgency-cpu-bound-v2-664f654ff4-vxc7h.mail>

ushindi bienvenu pushed to branch 29.x at Tiki Wiki CMS Groupware / Tiki


Commits:
73f12667 by Sacha Pignot at 2026-04-04T18:23:32+00:00
[BP][FIX][REF] notepad: Fix note update bug and improve code quality
---
* [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

(cherry picked from commit 0d0b547c3b1eea654c4fa15d0de67beeed496958)

See merge request tikiwiki/tiki!9759

- - - - -


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
=====================================
@@ -10,12 +10,27 @@ if (strpos($_SERVER["SCRIPT_NAME"], basename(__FILE__)) !== false) {
     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
=====================================
@@ -739,16 +739,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);
@@ -757,7 +763,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,
@@ -765,8 +770,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,34 +76,30 @@ 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"];
-}
-if (! isset($_REQUEST["offset"])) {
-    $offset = 0;
-} else {
-    $offset = $_REQUEST["offset"];
-}
+$sort_mode = $_REQUEST["sort_mode"] ?? 'lastModif_desc';
+$offset = $_REQUEST["offset"] ?? 0;
 $smarty->assign_by_ref('offset', $offset);
 if (isset($_REQUEST["find"])) {
     $find = $_REQUEST["find"];
@@ -110,11 +108,7 @@ if (isset($_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/73f126672b8ded073e142b39dba0c99ac3a5f3c7

-- 
View it on GitLab: https://gitlab.com/tikiwiki/tiki/-/commit/73f126672b8ded073e142b39dba0c99ac3a5f3c7
You're receiving this email because of your account on gitlab.com. Manage all notifications: https://gitlab.com/-/profile/notifications | Help: https://gitlab.com/help

_______________________________________________
TikiWiki-cvs mailing list
[email protected]
https://lists.sourceforge.net/lists/listinfo/tikiwiki-cvs
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.