Apply suggestions from code review

diff --git a/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts b/ts/WoltLabSuite/Core/Controller/Clipboard/Data.ts
index 5e3a26ceb4e58ad94abcb4fbd89572bc74b32e74..3964b2cabe07f116caf1a6b9e33785509eda10cc 100644 (file)
--- 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 7932840ff03e4c2014b26ac4c466ead44acc7256..f5d965e9da6c2a0f90c72a445c4eb2137078c83c 100644 (file)
--- 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 54fb91c1418ed34d48b58e75e644648871c90932..689dea7e72ec09b8225e01ab926cbca9be95334d 100644 (file)
--- 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 eb19054118fd3ce9fb8eb530d9571836b2848745..5aca3d4f7d24691e42f1cddb0e650dacc268c8a0 100644 (file)
--- 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 b7bbf2ab4be63317a2460b4ea7b42966aefc1d79..14185c1911c2fe5d2d7eb4181f1c756bd17c3f52 100644 (file)
--- 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;
         }
     }