Fix error displaying when editing groups/clients/domains/lists (#2808)

groups.lp

@@ -81,6 +81,7 @@ mg.include('scripts/pi-hole/lua/header_authenticated.lp','r')
 
 <script src="<?=pihole.fileversion('scripts/vendor/bootstrap-select.min.js')?>"></script>
 <script src="<?=pihole.fileversion('scripts/vendor/bootstrap-toggle.min.js')?>"></script>
+<script src="<?=pihole.fileversion('scripts/pi-hole/js/groups-common.js')?>"></script>
 <script src="<?=pihole.fileversion('scripts/pi-hole/js/groups.js')?>"></script>
 
 <? mg.include('scripts/pi-hole/lua/footer.lp','r')?>

scripts/pi-hole/js/groups-clients.js

@@ -5,7 +5,7 @@
  *  This file is copyright under the latest version of the EUPL.
  *  Please see LICENSE file for your rights under this license. */
 
-/* global utils:false, groups:false,, apiFailure:false, updateFtlInfo:false, getGroups:false */
+/* global utils:false, groups:false,, apiFailure:false, updateFtlInfo:false, getGroups:false, processGroupResult:false */
 
 var table;
 
@@ -489,6 +489,16 @@ function editClient() {
   var done = "edited";
   var notDone = "editing";
   switch (elem) {
+    case "enabled_" + client:
+      if (!enabled) {
+        done = "disabled";
+        notDone = "disabling";
+      } else {
+        done = "enabled";
+        notDone = "enabling";
+      }
+
+      break;
     case "multiselect_" + client:
       done = "edited groups of";
       notDone = "editing groups of";
@@ -516,14 +526,9 @@ function editClient() {
       comment: comment,
       enabled: enabled,
     }),
-    success: function () {
+    success: function (data) {
       utils.enableAll();
-      utils.showAlert(
+      processGroupResult(data, "client", done, notDone);
-        "success",
-        "fas fa-pencil-alt",
-        "Successfully " + done + " client",
-        clientDecoded
-      );
       table.ajax.reload(null, false);
     },
     error: function (data, exception) {

scripts/pi-hole/js/groups-common.js

@@ -5,7 +5,7 @@
  *  This file is copyright under the latest version of the EUPL.
  *  Please see LICENSE file for your rights under this license. */
 
-/* global apiFailure:false */
+/* global apiFailure:false, utils:false */
 
 // eslint-disable-next-line no-unused-vars
 var groups = [];
@@ -24,3 +24,21 @@ function getGroups() {
     },
   });
 }
+
+// eslint-disable-next-line no-unused-vars
+function processGroupResult(data, type, done, notDone) {
+  // Loop over data.processed.success and show toasts
+  data.processed.success.forEach(function (item) {
+    utils.showAlert("success", "fas fa-pencil-alt", `Successfully ${done} ${type}`, item);
+  });
+  // Loop over errors and display them
+  data.processed.errors.forEach(function (error) {
+    console.log(error); // eslint-disable-line no-console
+    utils.showAlert(
+      "error",
+      "",
+      `Error while ${notDone} ${type} ${utils.escapeHtml(error.item)}`,
+      error.error
+    );
+  });
+}

scripts/pi-hole/js/groups-domains.js

@@ -5,7 +5,7 @@
  *  This file is copyright under the latest version of the EUPL.
  *  Please see LICENSE file for your rights under this license. */
 
-/* global utils:false, groups:false,, getGroups:false, updateFtlInfo:false, apiFailure:false */
+/* global utils:false, groups:false,, getGroups:false, updateFtlInfo:false, apiFailure:false, processGroupResult:false */
 
 var table;
 var GETDict = {};
@@ -13,6 +13,7 @@ var GETDict = {};
 $(function () {
   GETDict = utils.parseQueryString();
 
+  // Tabs: Domain/Regex handling
   // sync description fields, reset inactive inputs on tab change
   $('a[data-toggle="tab"]').on("shown.bs.tab", function () {
     var tabHref = $(this).attr("href");
@@ -34,6 +35,7 @@ $(function () {
 
   $("#add_deny, #add_allow").on("click", addDomain);
 
+  // Domain suggestion handling
   var suggestTimeout;
   $("#new_domain").on("input", function (e) {
     hideSuggestDomains();
@@ -45,6 +47,7 @@ $(function () {
   initTable();
 });
 
+// Show a list of suggested domains based on the user's input
 function showSuggestDomains(value) {
   function createButton(hostname) {
     // Purposefully omit 'btn' class to save space on padding
@@ -134,8 +137,9 @@ function initTable() {
         $("body > .bootstrap-select.dropdown").remove();
       },
       rowCallback: function (row, data) {
-        var dataId = utils.hexEncode(data.domain);
+        var dataId = utils.hexEncode(data.domain) + "_" + data.type + "_" + data.kind;
         $(row).attr("data-id", dataId);
+        // Tooltip for domain
         var tooltip =
           "Added: " +
           utils.datetime(data.date_added, false) +
@@ -182,6 +186,7 @@ function initTable() {
         var typeEl = $("#type_" + dataId, row);
         typeEl.on("change", editDomain);
 
+        // Initialize bootstrap-toggle for status field (enabled/disabled)
         $("td:eq(3)", row).html(
           '<input type="checkbox" id="enabled_' +
             dataId +
@@ -199,12 +204,13 @@ function initTable() {
         });
         statusEl.on("change", editDomain);
 
+        // Comment field
         $("td:eq(4)", row).html('<input id="comment_' + dataId + '" class="form-control">');
         var commentEl = $("#comment_" + dataId, row);
         commentEl.val(utils.unescapeHtml(data.comment));
         commentEl.on("change", editDomain);
 
-        // Group assignment field
+        // Group assignment field (multi-select)
         $("td:eq(5)", row).empty();
         $("td:eq(5)", row).append(
           '<select class="selectpicker" id="multiselect_' + dataId + '" multiple></select>'
@@ -227,6 +233,7 @@ function initTable() {
         // Select assigned groups
         selectEl.val(data.groups);
         // Initialize bootstrap-select
+        const applyBtn = "#btn_apply_" + dataId;
         selectEl
           // fix dropdown if it would stick out right of the viewport
           .on("show.bs.select", function () {
@@ -242,7 +249,8 @@ function initTable() {
             }
           })
           .on("changed.bs.select", function () {
-            // enable Apply button
+            // enable Apply button if changes were made to the drop-down menu
+            // and have it call editDomain() on click
             if ($(applyBtn).prop("disabled")) {
               $(applyBtn)
                 .addClass("btn-success")
@@ -253,7 +261,9 @@ function initTable() {
             }
           })
           .on("hide.bs.select", function () {
-            // Restore values if drop-down menu is closed without clicking the Apply button
+            // Restore values if drop-down menu is closed without clicking the
+            // Apply button (e.g. by clicking outside) and re-disable the Apply
+            // button
             if (!$(applyBtn).prop("disabled")) {
               $(this).val(data.groups).selectpicker("refresh");
               $(applyBtn).removeClass("btn-success").prop("disabled", true).off("click");
@@ -268,8 +278,6 @@ function initTable() {
               ' class="btn btn-block btn-sm" disabled>Apply</button>'
           );
 
-        var applyBtn = "#btn_apply_" + dataId;
-
         // Highlight row (if url parameter "domainid=" is used)
         if ("domainid" in GETDict && data.id === parseInt(GETDict.domainid, 10)) {
           $(row).find("td").addClass("highlight");
@@ -288,7 +296,7 @@ function initTable() {
       },
       select: {
         style: "multi",
-        selector: "td:not(:last-child)",
+        selector: "td:first-child",
         info: false,
       },
       buttons: [
@@ -433,7 +441,7 @@ function delItems(ids) {
 
   // Get first element from array
   const domainRaw = ids[0];
-  const domain = utils.hexDecode(domainRaw);
+  const domain = utils.hexDecode(domainRaw.split("_")[0]);
   const typestr = $("#old_type_" + domainRaw).val();
 
   // Remove first element from array
@@ -449,7 +457,7 @@ function delItems(ids) {
   })
     .done(function () {
       utils.enableAll();
-      utils.showAlert("success", "far fa-trash-alt", "Successfully deleted list: ", domain);
+      utils.showAlert("success", "far fa-trash-alt", "Successfully deleted domain: ", domain);
       table.row(domainRaw).remove().draw(false);
       if (ids.length > 0) {
         // Recursively delete all remaining items
@@ -581,7 +589,7 @@ function editDomain() {
   var notDone = "editing";
   switch (elem) {
     case "enabled_" + domain:
-      if (enabled) {
+      if (!enabled) {
         done = "disabled";
         notDone = "disabling";
       } else {
@@ -612,7 +620,7 @@ function editDomain() {
   }
 
   utils.disableAll();
-  const domainDecoded = utils.hexDecode(domain);
+  const domainDecoded = utils.hexDecode(domain.split("_")[0]);
   utils.showAlert("info", "", "Editing domain...", domain);
   $.ajax({
     url: "/api/domains/" + newTypestr + "/" + encodeURIComponent(domainDecoded),
@@ -626,14 +634,9 @@ function editDomain() {
       type: oldType,
       kind: oldKind,
     }),
-    success: function () {
+    success: function (data) {
       utils.enableAll();
-      utils.showAlert(
+      processGroupResult(data, "domain", done, notDone);
-        "success",
-        "fas fa-pencil-alt",
-        "Successfully " + done + " domain",
-        domainDecoded
-      );
       table.ajax.reload(null, false);
     },
     error: function (data, exception) {

scripts/pi-hole/js/groups-lists.js

@@ -5,7 +5,7 @@
  *  This file is copyright under the latest version of the EUPL.
  *  Please see LICENSE file for your rights under this license. */
 
-/* global utils:false, groups:false, apiFailure:false, updateFtlInfo:false, getGroups:false */
+/* global utils:false, groups:false, apiFailure:false, updateFtlInfo:false, getGroups:false, processGroupResult:false */
 
 var table;
 var GETDict = {};
@@ -549,20 +549,19 @@ function editList() {
   const tr = $(this).closest("tr");
   const type = tr.attr("data-type");
   const address = tr.attr("data-id");
-  const status = tr.find("#enabled_" + address).is(":checked");
+  const enabled = tr.find("#enabled_" + address).is(":checked");
   const comment = utils.escapeHtml(tr.find("#comment_" + address).val());
   // Convert list of string integers to list of integers using map(Number)
   const groups = tr
     .find("#multiselect_" + address)
     .val()
     .map(Number);
-  const enabled = tr.find("#enabled_" + address).is(":checked");
 
   var done = "edited";
   var notDone = "editing";
   switch (elem) {
     case "enabled_" + address:
-      if (status) {
+      if (!enabled) {
         done = "disabled";
         notDone = "disabling";
       } else {
@@ -598,14 +597,9 @@ function editList() {
       enabled: enabled,
       type: type,
     }),
-    success: function () {
+    success: function (data) {
       utils.enableAll();
-      utils.showAlert(
+      processGroupResult(data, "list", done, notDone);
-        "success",
-        "fas fa-pencil-alt",
-        "Successfully " + done + " list",
-        addressDecoded
-      );
       table.ajax.reload(null, false);
     },
     error: function (data, exception) {

scripts/pi-hole/js/groups.js

@@ -5,7 +5,7 @@
  *  This file is copyright under the latest version of the EUPL.
  *  Please see LICENSE file for your rights under this license. */
 
-/* global utils:false, apiFailure:false, updateFtlInfo:false */
+/* global utils:false, apiFailure:false, updateFtlInfo:false, processGroupResult:false */
 
 var table,
   idNames = {};
@@ -344,10 +344,10 @@ function editGroup() {
   var notDone = "editing";
   switch (elem) {
     case "enabled_" + id:
-      if (enabled === false) {
+      if (!enabled) {
         done = "disabled";
         notDone = "disabling";
-      } else if (enabled === true) {
+      } else {
         done = "enabled";
         notDone = "enabling";
       }
@@ -378,9 +378,10 @@ function editGroup() {
       comment: comment,
       enabled: enabled,
     }),
-    success: function () {
+    success: function (data) {
       utils.enableAll();
-      utils.showAlert("success", "fas fa-pencil-alt", "Successfully " + done + " group", oldName);
+      processGroupResult(data, "group", done, notDone);
+      table.ajax.reload(null, false);
     },
     error: function (data, exception) {
       apiFailure(data);

scripts/pi-hole/js/utils.js

@@ -113,7 +113,8 @@ function showAlert(type, icon, title, message) {
       break;
     case "error":
       options.icon = "fas fa-times";
-      options.title = "&nbsp;<strong>Error, something went wrong!</strong><br>";
+      if (title.length === 0)
+        options.title = "&nbsp;<strong>Error, something went wrong!</strong><br>";
       settings.delay *= 2;
 
       // If the message is an API object, nicely format the error message