Adds Delete Trigger action by ramsaysewell · Pull Request #529 · rajnandan1/kener

ramsaysewell · 2026-01-04T19:30:28Z

Adds a "Delete Trigger" button next to the "Test Trigger" and "Settings" buttons that removes the Trigger from the database.

There is currently no "confirm" implemented for this destructive action.

Copilot

Pull request overview

This PR adds the ability to delete triggers through a new "Delete Trigger" button in the UI. The implementation includes backend API endpoints, database operations, and frontend UI changes, but notably lacks a confirmation dialog for this destructive action.

Key Changes

Added DeleteTrigger action handler in the API server with proper role-based authorization
Implemented database method deleteTrigger to remove triggers by ID
Added "Delete Trigger" button with loading/success/error states in the trigger management UI

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 8 comments.

File	Description
src/routes/(manage)/manage/(app)/app/api/+server.js	Added API route handler for "deleteTrigger" action with AdminEditor authorization check; also reorganized imports alphabetically
src/lib/server/db/dbimpl.js	Added `deleteTrigger` database method to delete triggers by ID; minor whitespace cleanup
src/lib/server/controllers/controller.js	Added `DeleteTrigger` controller function with ID validation; reorganized imports alphabetically
src/lib/components/manage/triggerInfo.svelte	Added delete button UI, `deleteTrigger` async function with loader states, and updated trigger list rendering; removed unused import and simplified `testTrigger` function signature

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-01-04T19:35:40Z

src/lib/components/manage/triggerInfo.svelte

+
+      if (resp.error) {
+        triggers[i].deleteLoaders = "error";
+        alert(resp.error);


Using alert() for error messages is not consistent with modern UI/UX practices and can be jarring for users. Consider using a toast notification or inline error message that matches the existing design system, similar to how other parts of the application handle errors.

Copilot · 2026-01-04T19:35:41Z

src/lib/components/manage/triggerInfo.svelte

+        }, 1000);
+      }
+    } catch (error) {
+      alert("Error: " + error);


Using alert() for error messages is not consistent with modern UI/UX practices. Consider using a toast notification or inline error message that matches the existing design system.

Copilot · 2026-01-04T19:35:41Z

src/lib/server/db/dbimpl.js

    });
  }

+  //delete alert


The comment "//delete alert" is inconsistent with the function name. The function is called deleteTrigger and works with triggers, not alerts. The comment should say "//delete trigger" to match the function's purpose.

Suggested change

//delete alert

//delete trigger

Copilot · 2026-01-04T19:35:41Z

src/lib/server/db/dbimpl.js


+  //delete alert
+  async deleteTrigger(data) {
+    return await this.knex("triggers").where({ id: data.id }).delete();


Deleting a trigger without handling references to it in the monitors table could result in orphaned references. The monitors table has down_trigger and degraded_trigger fields that may reference trigger IDs. Consider either: 1) Setting up a foreign key with CASCADE on the database level, 2) Checking if the trigger is referenced by any monitors before deletion and preventing the delete or clearing those references, or 3) Automatically clearing monitor references when the trigger is deleted.

Suggested change

return await this.knex("triggers").where({ id: data.id }).delete();

const triggerId = data.id;

return await this.knex.transaction(async (trx) => {

// Clear references from monitors that point to this trigger

await trx("monitors")

.where("down_trigger", triggerId)

.update({ down_trigger: null });

await trx("monitors")

.where("degraded_trigger", triggerId)

.update({ degraded_trigger: null });

// Delete the trigger itself

return await trx("triggers").where({ id: triggerId }).delete();

});

Copilot · 2026-01-04T19:35:41Z

src/lib/components/manage/triggerInfo.svelte

+      setTimeout(() => {
+        triggers[i].deleteLoaders = "idle";
+      }, 3000);
+    }


There's a potential timing issue with the loader state management. The finally block sets a 3-second timeout to reset the loader state, but on successful deletion, loadData() is called after 1 second, which will replace the triggers array. This means the loader state reset on line 291 will try to update a stale reference that's no longer in the triggers array. Consider clearing the timeout when data is reloaded, or manage the loader state differently.

Copilot · 2026-01-04T19:35:42Z

src/lib/components/manage/triggerInfo.svelte

+          "Content-Type": "application/json"
+        },
+        body: JSON.stringify({ action: "deleteTrigger", data: { id: trigger_id } })
+      });


The delete function doesn't check the HTTP response status code. Even if the server returns an error status code (e.g., 500, 403), the code attempts to parse JSON and only checks for resp.error. If the server returns a non-JSON error response, this will fail. Consider checking data.ok or data.status before parsing the JSON response.

Suggested change

});

});

if (!data.ok) {

triggers[i].deleteLoaders = "error";

let message = "Request failed with status " + data.status;

try {

const errorBody = await data.text();

if (errorBody) {

message += ": " + errorBody;

}

} catch (e) {

// ignore body parsing errors, we already have status

}

alert(message);

return;

}

Copilot · 2026-01-04T19:35:42Z

src/lib/server/controllers/controller.js

+  if (alertData.id) {
+    return await db.deleteTrigger(alertData);
+  }


The condition on line 327 is redundant. The check on line 323 already ensures that alertData.id exists, so the second check on line 327 will always be true. Either remove the second condition and unconditionally call db.deleteTrigger(alertData), or combine both error cases into a single validation block.

Suggested change

if (alertData.id) {

return await db.deleteTrigger(alertData);

}

return await db.deleteTrigger(alertData);

Copilot · 2026-01-04T19:35:42Z

src/lib/components/manage/triggerInfo.svelte

+  async function deleteTrigger(i, trigger_id) {
+    triggers[i].deleteLoaders = "loading";
+    try {
+      let data = await fetch(base + "/manage/app/api/", {
+        method: "POST",
+        headers: {
+          "Content-Type": "application/json"
+        },
+        body: JSON.stringify({ action: "deleteTrigger", data: { id: trigger_id } })
+      });
+      let resp = await data.json();
+
+      if (resp.error) {
+        triggers[i].deleteLoaders = "error";
+        alert(resp.error);
+      } else {
+        triggers[i].deleteLoaders = "success";
+        setTimeout(async () => {
+          await loadData();
+        }, 1000);
+      }
+    } catch (error) {
+      alert("Error: " + error);
+    } finally {
+      setTimeout(() => {
+        triggers[i].deleteLoaders = "idle";
+      }, 3000);
+    }
+  }


This destructive action lacks user confirmation before deleting the trigger. As noted in the PR description, there is no confirmation dialog implemented. Consider adding a confirmation prompt (e.g., using a modal or browser confirm) before proceeding with the deletion to prevent accidental data loss.

ramsaysewell · 2026-01-05T22:07:18Z

This pr will resolve #522

rajnandan1 · 2026-02-24T14:37:35Z

This request/issue/feature has been added to v4.x
The release of this new version was a lot of effort. If you find this project then please support me by either of the below options

Ramsay Sewell added 4 commits January 4, 2026 19:26

refactor: clean up and reorder imports

e67306a

fix: improve trigger list rendering and minor formatting

d280897

feat: add trigger deletion functionality

009783d

refactor: simplify test trigger function

6374146

Copilot AI review requested due to automatic review settings January 4, 2026 19:30

Copilot started reviewing on behalf of ramsaysewell January 4, 2026 19:30 View session

Copilot AI reviewed Jan 4, 2026

View reviewed changes

rajnandan1 added this to Kener v4 Launch Jan 23, 2026

github-project-automation bot moved this to Backlog in Kener v4 Launch Jan 23, 2026

rajnandan1 moved this from Backlog to In progress in Kener v4 Launch Feb 2, 2026

rajnandan1 moved this from In progress to Done in Kener v4 Launch Feb 24, 2026

rajnandan1 closed this Feb 24, 2026

github-project-automation bot moved this from Done to Dev Done in Kener v4 Launch Feb 24, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Adds Delete Trigger action#529

Adds Delete Trigger action#529
ramsaysewell wants to merge 4 commits intorajnandan1:mainfrom
ramsaysewell:feature/delete-trigger-action

ramsaysewell commented Jan 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

Copilot AI Jan 4, 2026

Uh oh!

ramsaysewell commented Jan 5, 2026

Uh oh!

rajnandan1 commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

-    return await this.knex("triggers").where({ id: data.id }).delete();
+    const triggerId = data.id;
+    return await this.knex.transaction(async (trx) => {
+      // Clear references from monitors that point to this trigger
+      await trx("monitors")
+        .where("down_trigger", triggerId)
+        .update({ down_trigger: null });
+      await trx("monitors")
+        .where("degraded_trigger", triggerId)
+        .update({ degraded_trigger: null });
+      // Delete the trigger itself
+      return await trx("triggers").where({ id: triggerId }).delete();
+    });

-      });
+      });
+      if (!data.ok) {
+        triggers[i].deleteLoaders = "error";
+        let message = "Request failed with status " + data.status;
+        try {
+          const errorBody = await data.text();
+          if (errorBody) {
+            message += ": " + errorBody;
+          }
+        } catch (e) {
+          // ignore body parsing errors, we already have status
+        }
+        alert(message);
+        return;
+      }

Uh oh!

Conversation

ramsaysewell commented Jan 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Key Changes

Reviewed changes

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Jan 4, 2026

Choose a reason for hiding this comment

Uh oh!

ramsaysewell commented Jan 5, 2026

Uh oh!

rajnandan1 commented Feb 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants