From: Matthias Schmidt <gravatronics@live.com>
Date: Mon, 15 Mar 2021 09:13:46 +0000 (+0100)
Subject: Apply suggestions from code review
X-Git-Tag: 5.4.0_Alpha_1~132^2~24
X-Git-Url: https://git.stricted.de/?a=commitdiff_plain;h=f05cd4e8fb442dcd279e5b63599845febc55b7e5;p=GitHub%2FWoltLab%2FWCF.git

Apply suggestions from code review
---

diff --git a/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts b/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts
index 5e3a26ceb4..3964b2cabe 100644
--- a/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts
+++ b/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts
@@ -40,7 +40,7 @@ export interface AjaxResponse extends DatabaseObjectActionResponse {
   returnValues: {
     action: string;
     items?: {
-      // They key is the `typeName`
+      // The key is the `typeName`
       [key: string]: ClipboardItemData;
     };
     markedItems?: AjaxResponseMarkedItems;
diff --git a/ts/WoltLabSuite/Core/Ui/Object/Action.ts b/ts/WoltLabSuite/Core/Ui/Object/Action.ts
index 7932840ff0..f5d965e9da 100644
--- a/ts/WoltLabSuite/Core/Ui/Object/Action.ts
+++ b/ts/WoltLabSuite/Core/Ui/Object/Action.ts
@@ -36,16 +36,26 @@ function executeAction(event: Event): void {
   }
 
   // Collect additional request parameters.
-  // TODO: Is still untested.
   const parameters = {};
   Object.entries(actionElement.dataset).forEach(([key, value]) => {
-    if (/^objectActionParameterData.+/.exec(key)) {
-      if (!("data" in parameters)) {
+    let matches = /^objectActionParameterData(.+)/.exec(key);
+    if (matches) {
+      if (!Object.prototype.hasOwnProperty.call(parameters, "data")) {
         parameters["data"] = {};
       }
-      parameters[StringUtil.lcfirst(key.replace(/^objectActionParameterData/, ""))] = value;
-    } else if (/^objectActionParameter.+/.exec(key)) {
-      parameters[StringUtil.lcfirst(key.replace(/^objectActionParameter/, ""))] = value;
+      parameters["data"][StringUtil.lcfirst(matches[1])] = value;
+    } else {
+      matches = /^objectActionParameter(.+)/.exec(key);
+
+      if (matches) {
+        const key = StringUtil.lcfirst(matches[1]);
+
+        if (key === "data") {
+          throw new Error("Additional object action parameters may not use 'data' as key.");
+        }
+
+        parameters[key] = value;
+      }
     }
   });
 
diff --git a/ts/WoltLabSuite/Core/Ui/Object/Action/Toogle.ts b/ts/WoltLabSuite/Core/Ui/Object/Action/Toogle.ts
index 54fb91c141..689dea7e72 100644
--- a/ts/WoltLabSuite/Core/Ui/Object/Action/Toogle.ts
+++ b/ts/WoltLabSuite/Core/Ui/Object/Action/Toogle.ts
@@ -17,16 +17,12 @@ function toggleObject(data: DatabaseObjectActionResponse, objectElement: HTMLEle
   if (toggleButton.classList.contains("fa-square-o")) {
     toggleButton.classList.replace("fa-square-o", "fa-check-square-o");
 
-    const newTitle = toggleButton.dataset.disableTitle
-      ? toggleButton.dataset.disableTitle
-      : Language.get("wcf.global.button.disable");
+    const newTitle = toggleButton.dataset.disableTitle || Language.get("wcf.global.button.disable");
     toggleButton.title = newTitle;
   } else {
     toggleButton.classList.replace("fa-check-square-o", "fa-square-o");
 
-    const newTitle = toggleButton.dataset.enableTitle
-      ? toggleButton.dataset.enableTitle
-      : Language.get("wcf.global.button.enable");
+    const newTitle = toggleButton.dataset.enableTitle || Language.get("wcf.global.button.enable");
     toggleButton.title = newTitle;
   }
 }
diff --git a/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action.js b/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action.js
index eb19054118..5aca3d4f7d 100644
--- a/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action.js
+++ b/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action.js
@@ -32,17 +32,24 @@ define(["require", "exports", "tslib", "../../Ajax", "../../Event/Handler", "../
             objectId = actionElement.dataset.objectId;
         }
         // Collect additional request parameters.
-        // TODO: Is still untested.
         const parameters = {};
         Object.entries(actionElement.dataset).forEach(([key, value]) => {
-            if (/^objectActionParameterData.+/.exec(key)) {
-                if (!("data" in parameters)) {
+            let matches = /^objectActionParameterData(.+)/.exec(key);
+            if (matches) {
+                if (!Object.prototype.hasOwnProperty.call(parameters, "data")) {
                     parameters["data"] = {};
                 }
-                parameters[StringUtil.lcfirst(key.replace(/^objectActionParameterData/, ""))] = value;
+                parameters["data"][StringUtil.lcfirst(matches[1])] = value;
             }
-            else if (/^objectActionParameter.+/.exec(key)) {
-                parameters[StringUtil.lcfirst(key.replace(/^objectActionParameter/, ""))] = value;
+            else {
+                matches = /^objectActionParameter(.+)/.exec(key);
+                if (matches) {
+                    const key = StringUtil.lcfirst(matches[1]);
+                    if (key === "data") {
+                        throw new Error("Additional object action parameters may not use 'data' as key.");
+                    }
+                    parameters[key] = value;
+                }
             }
         });
         function sendRequest() {
diff --git a/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action/Toogle.js b/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action/Toogle.js
index b7bbf2ab4b..14185c1911 100644
--- a/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action/Toogle.js
+++ b/wcfsetup/install/files/js/WoltLabSuite/Core/Ui/Object/Action/Toogle.js
@@ -16,16 +16,12 @@ define(["require", "exports", "tslib", "../../../Language", "./Handler"], functi
         const toggleButton = objectElement.querySelector('.jsObjectAction[data-object-action="toggle"]');
         if (toggleButton.classList.contains("fa-square-o")) {
             toggleButton.classList.replace("fa-square-o", "fa-check-square-o");
-            const newTitle = toggleButton.dataset.disableTitle
-                ? toggleButton.dataset.disableTitle
-                : Language.get("wcf.global.button.disable");
+            const newTitle = toggleButton.dataset.disableTitle || Language.get("wcf.global.button.disable");
             toggleButton.title = newTitle;
         }
         else {
             toggleButton.classList.replace("fa-check-square-o", "fa-square-o");
-            const newTitle = toggleButton.dataset.enableTitle
-                ? toggleButton.dataset.enableTitle
-                : Language.get("wcf.global.button.enable");
+            const newTitle = toggleButton.dataset.enableTitle || Language.get("wcf.global.button.enable");
             toggleButton.title = newTitle;
         }
     }