feat: implement cascading deletion for related records in delete by kulikp1 · Pull Request #488 · devforth/adminforth

kulikp1 · 2026-02-25T14:16:46Z

…oint

adminforth/modules/restApi.ts

adminforth/modules/configValidator.ts

…QLite connectors

Copilot

Pull request overview

This PR adds application-level cascading deletion support for related records in AdminForth's delete endpoint. When onDelete: 'cascade' or onDelete: 'setNull' is configured on a foreignResource column, deleting a parent record will either recursively delete child records or null-out the foreign key column in child records. The PR also adds detection of database-level ON DELETE CASCADE constraints in SQLite, PostgreSQL, and MySQL, with warnings when such constraints exist (to avoid conflicts with the new application-level cascade logic).

Changes:

New optional onDelete field ('cascade' | 'setNull') added to AdminForthForeignResource type, with validation in configValidator.ts
Core cascade deletion/null-out logic added to the /delete_record REST endpoint in restApi.ts
Database-level ON DELETE CASCADE detection and warning added to all three data connectors (SQLite, PostgreSQL, MySQL)

Reviewed changes

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

Show a summary per file

File	Description
`adminforth/types/Back.ts`	Adds `onDelete` field to `AdminForthForeignResource` interface
`adminforth/modules/restApi.ts`	Implements cascade delete / setNull logic before deleting the parent record
`adminforth/modules/configValidator.ts`	Validates the `onDelete` strategy value
`adminforth/dataConnectors/sqlite.ts`	Detects DB-level ON DELETE CASCADE via `PRAGMA foreign_key_list`
`adminforth/dataConnectors/postgres.ts`	Detects DB-level ON DELETE CASCADE via `pg_constraint` system tables
`adminforth/dataConnectors/mysql.ts`	Detects DB-level ON DELETE CASCADE via `information_schema`

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

adminforth/types/Back.ts

adminforth/modules/restApi.ts

adminforth/dataConnectors/postgres.ts

adminforth/dataConnectors/mysql.ts

adminforth/modules/configValidator.ts

Copilot

Pull request overview

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

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

adminforth/modules/restApi.ts

adminforth/modules/configValidator.ts

Copilot · 2026-03-03T11:32:32Z

adminforth/modules/restApi.ts

+                    await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);
+                  }
+                } else if (onDeleteStrategy === 'setNull') {
+                  for (const childRecord of childRecords) {
+                    await this.adminforth.resource(childRes.resourceId).update(childRecord[childPkFieldName], {[foreignResourceColumn.name]: null});


The cascade deletion at line 1496 uses this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]), which calls OperationalResource.delete() → connector.deleteRecord() directly, completely bypassing:

The resource's delete hooks (hooks.delete.beforeSave and hooks.delete.afterSave)

Permission checks (allowedActions.delete) for the child resource

This is a critical concern because these hooks are often used for cleanup of related artifacts (e.g., file storage cleanup in upload plugins, as referenced by the warning in the connector code). Additionally, the setNull path at line 1500 has the same issue — this.adminforth.resource(childRes.resourceId).update() similarly bypasses edit hooks for child records.

To trigger hooks and permissions, the implementation should use this.adminforth.deleteResourceRecord(...) for cascade deletions and this.adminforth.updateResourceRecord(...) for setNull operations, similar to how the parent record deletion is handled at line 1506.

Suggested change

await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);

}

} else if (onDeleteStrategy === 'setNull') {

for (const childRecord of childRecords) {

await this.adminforth.resource(childRes.resourceId).update(childRecord[childPkFieldName], {[foreignResourceColumn.name]: null});

const { error: childDeleteError } = await this.adminforth.deleteResourceRecord({

resource: childRes,

record: childRecord,

adminUser,

recordId: childRecord[childPkFieldName],

response,

extra: { body, query, headers, cookies, requestUrl, response }

});

if (childDeleteError) {

return { error: childDeleteError };

}

}

} else if (onDeleteStrategy === 'setNull') {

for (const childRecord of childRecords) {

const { error: childUpdateError } = await this.adminforth.updateResourceRecord({

resource: childRes,

record: childRecord,

adminUser,

recordId: childRecord[childPkFieldName],

updatedRecord: { [foreignResourceColumn.name]: null },

response,

extra: { body, query, headers, cookies, requestUrl, response }

});

if (childUpdateError) {

return { error: childUpdateError };

}

Copilot · 2026-03-03T11:32:32Z

adminforth/modules/restApi.ts

+                const foreignResourceColumn = childRes.columns.find(c => c.foreignResource?.resourceId === resource.resourceId);
+                if (!foreignResourceColumn.foreignResource.onDelete) continue;
+                const onDeleteStrategy = foreignResourceColumn.foreignResource.onDelete;    
+                const childRecords = await this.adminforth.resource(childRes.resourceId).list(Filters.EQ(foreignResourceColumn.name, body['primaryKey']))
+                const childPkColumn = childRes.columns.find(col => col.primaryKey);
+                if (!childPkColumn) continue;
+                const childPkFieldName = childPkColumn.name;
+                if (onDeleteStrategy === 'cascade') {
+                  for (const childRecord of childRecords) {
+                    await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);
+                  }
+                } else if (onDeleteStrategy === 'setNull') {
+                  for (const childRecord of childRecords) {
+                    await this.adminforth.resource(childRes.resourceId).update(childRecord[childPkFieldName], {[foreignResourceColumn.name]: null});


At line 1487, childRes.columns.find(...) only returns the FIRST column in the child resource that has a foreignResource.resourceId matching the parent. If a child resource has multiple columns referencing the same parent resource (e.g., created_by and updated_by both pointing to users), only the first matching column will have its cascade/setNull strategy applied. The second FK column will be silently ignored.

Consider using childRes.columns.filter(...) to get all FK columns pointing to the parent resource, and then iterating over each one.

Suggested change

const foreignResourceColumn = childRes.columns.find(c => c.foreignResource?.resourceId === resource.resourceId);

if (!foreignResourceColumn.foreignResource.onDelete) continue;

const onDeleteStrategy = foreignResourceColumn.foreignResource.onDelete;

const childRecords = await this.adminforth.resource(childRes.resourceId).list(Filters.EQ(foreignResourceColumn.name, body['primaryKey']))

const childPkColumn = childRes.columns.find(col => col.primaryKey);

if (!childPkColumn) continue;

const childPkFieldName = childPkColumn.name;

if (onDeleteStrategy === 'cascade') {

for (const childRecord of childRecords) {

await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);

}

} else if (onDeleteStrategy === 'setNull') {

for (const childRecord of childRecords) {

await this.adminforth.resource(childRes.resourceId).update(childRecord[childPkFieldName], {[foreignResourceColumn.name]: null});

const foreignResourceColumns = childRes.columns.filter(c => c.foreignResource?.resourceId === resource.resourceId);

if (!foreignResourceColumns.length) continue;

const childPkColumn = childRes.columns.find(col => col.primaryKey);

if (!childPkColumn) continue;

const childPkFieldName = childPkColumn.name;

for (const foreignResourceColumn of foreignResourceColumns) {

if (!foreignResourceColumn.foreignResource || !foreignResourceColumn.foreignResource.onDelete) {

continue;

}

const onDeleteStrategy = foreignResourceColumn.foreignResource.onDelete;

const childRecords = await this.adminforth.resource(childRes.resourceId).list(

Filters.EQ(foreignResourceColumn.name, body['primaryKey'])

);

if (onDeleteStrategy === 'cascade') {

for (const childRecord of childRecords) {

await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);

}

} else if (onDeleteStrategy === 'setNull') {

for (const childRecord of childRecords) {

await this.adminforth.resource(childRes.resourceId).update(

childRecord[childPkFieldName],

{ [foreignResourceColumn.name]: null }

);

}

Copilot · 2026-03-03T11:32:32Z

adminforth/dataConnectors/mysql.ts

+    const [fkResults] = await this.client.execute(`
+      SELECT
+        kcu.TABLE_NAME AS child_table,
+        kcu.COLUMN_NAME AS column_name,
+        rc.DELETE_RULE AS delete_rule
+      FROM information_schema.KEY_COLUMN_USAGE kcu
+      JOIN information_schema.REFERENTIAL_CONSTRAINTS rc
+        ON kcu.CONSTRAINT_NAME = rc.CONSTRAINT_NAME
+        AND kcu.CONSTRAINT_SCHEMA = rc.CONSTRAINT_SCHEMA
+      WHERE kcu.REFERENCED_TABLE_NAME = ?
+        AND kcu.TABLE_SCHEMA = DATABASE()
+    `, [resource.table]);
+
+    const fkMap: Record<string, { cascade: boolean; childTable: string }> = {};
+    for (const fk of fkResults as any[]) {
+      fkMap[String(fk.column_name)] = {
+        cascade: String(fk.delete_rule).toUpperCase() === 'CASCADE',
+        childTable: fk.child_table
+      };
+      if (fkMap[fk.column_name].cascade) {
+        afLogger.warn(`The database has ON DELETE CASCADE, which may conflict with adminForth cascade deletion and upload logic. Please remove it.`);      
+      }


The MySQL query uses WHERE kcu.REFERENCED_TABLE_NAME = ? and selects kcu.COLUMN_NAME, which returns column names from CHILD tables that reference the current table (i.e., columns in other tables that point to this table as a parent). This is the opposite perspective from the SQLite implementation, which uses PRAGMA foreign_key_list(tableName) to return FK columns defined ON the current table (the child side).

The practical effect is that in MySQL, fkMap keys are column names from child tables rather than columns of the current table being discovered. For the warning logic this is functionally harmless since the warning fires as long as any cascade is found. However, it is semantically inconsistent with the SQLite approach and with the intent of discoverFields (which is supposed to characterize the fields of the current table). If fkMap is later used to set field properties (similar to SQLite's field.cascade = fkMap[row.name] || false), this inverted mapping would cause incorrect results.

adminforth/modules/restApi.ts

Copilot

Pull request overview

Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (2)

adminforth/modules/restApi.ts:1526

The deleteWithCascade method (line 190) already calls this.adminforth.deleteResourceRecord(...) for the top-level resource at the end, which performs the actual delete and fires before/after hooks. Then at lines 1521-1526, deleteWithCascade is called first, and immediately after, deleteResourceRecord is called again for the same record. This results in the record being deleted twice (which will likely cause a "record not found" error or silent no-op on the second call) and before/after delete hooks being fired twice for the top-level record.

The second call to deleteResourceRecord at lines 1523-1526 should be removed; deleteWithCascade already handles deleting the record and firing its hooks.

            await this.deleteWithCascade(resource, body.primaryKey, {body, adminUser, query, headers, cookies, requestUrl, response});

            const { error: deleteError } = await this.adminforth.deleteResourceRecord({ 
              resource, record, adminUser, recordId: body['primaryKey'], response, 
              extra: { body, query, headers, cookies, requestUrl, response } 
            });

adminforth/modules/configValidator.ts:301

The bulk delete action manually invokes beforeSave hooks (lines 263-279) and afterSave hooks (lines 288-301) around the call to deleteWithCascade. However, deleteWithCascade internally calls this.adminforth.deleteResourceRecord(...) (restApi.ts line 190), which itself runs all beforeSave and afterSave hooks for the resource (index.ts lines 758 and 781). This causes all delete hooks to fire twice for each record deleted via the bulk action: once explicitly in the bulk action and once inside deleteWithCascade/deleteResourceRecord.

The manual hook invocations in lines 263-279 and 288-301 should be removed from the bulk delete action, as deleteWithCascade (via deleteResourceRecord) already handles them.

            await Promise.all(
              (res.hooks.delete.beforeSave).map(
                async (hook) => {
                  const resp = await hook({ 
                    recordId: recordId,
                    resource: res as AdminForthResource, 
                    record, 
                    adminUser,
                    response,
                    adminforth: this.adminforth
                  }); 
                  if (!error && resp.error) {
                    error = resp.error;
                  }
                }
              )
            )

            if (error) {
              return;
            }
            
            const restApi = new AdminForthRestAPI (this.adminforth) 
            restApi.deleteWithCascade(res as AdminForthResource, recordId, { adminUser, response, body: undefined, query: undefined, headers: undefined, cookies: undefined, requestUrl: undefined});

            await Promise.all(
              (res.hooks.delete.afterSave).map(
                async (hook) => {
                  await hook({ 
                    resource: res as AdminForthResource, 
                    record, 
                    adminUser,
                    recordId: recordId,
                    response,
                    adminforth: this.adminforth,
                  }); 
                }
              )
            )

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

adminforth/modules/restApi.ts

adminforth/modules/configValidator.ts

feat: implement cascading deletion for related records in delete endp…

ca65f67

…oint

SerVitasik requested changes Feb 25, 2026

View reviewed changes

adminforth/modules/restApi.ts Show resolved Hide resolved

Pavlo Kulyk added 2 commits February 26, 2026 11:46

add alowwedAction check

bda166a

add missing spaces

12d2ba6

SerVitasik requested changes Feb 26, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

feat: implement cascading deletion logic in delete endpoint

02e216b

SerVitasik requested changes Feb 26, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

Pavlo Kulyk added 3 commits February 26, 2026 13:25

fix: update check strategy

23d178b

feat: refine cascading deletion logic in delete endpoint

8b5b7b5

fix: update condition

01dfcfd

SerVitasik requested changes Feb 26, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

Pavlo Kulyk added 3 commits February 26, 2026 16:37

fix: change variable name foreignKeyColumn to foreignResourceColumn

ee04911

fix: add check for foreign resource onDelete strategy

96c2c8f

feat: add onDelete type

a300f83

SerVitasik requested changes Feb 27, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

Pavlo Kulyk added 2 commits February 27, 2026 10:27

fix: delete strategy check

8ef2973

fix: add check for cascade strategy

552ecdc

SerVitasik requested changes Feb 27, 2026

View reviewed changes

adminforth/modules/configValidator.ts Outdated Show resolved Hide resolved

adminforth/modules/configValidator.ts Outdated Show resolved Hide resolved

Pavlo Kulyk added 4 commits March 2, 2026 13:44

fix: delete mistake in error message

423d6a0

fix: streamline foreign resource onDelete strategy validation

ff63b6c

add missing space

9520f80

fix: implement cascading deletion checks for MySQL, PostgreSQL, and S…

d6502d3

…QLite connectors

SerVitasik requested a review from Copilot March 3, 2026 09:17

Copilot started reviewing on behalf of SerVitasik March 3, 2026 09:18 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

fix: resolve copilot comment

1843641

SerVitasik requested a review from Copilot March 3, 2026 11:24

Copilot started reviewing on behalf of SerVitasik March 3, 2026 11:25 View session

Copilot AI reviewed Mar 3, 2026

View reviewed changes

fix: add required check for setNull deletion

21cc9a4

Pavlo Kulyk added 3 commits March 3, 2026 15:31

fix: change resource.options.allowedActions.delete check

81fce83

feat: implement cascading deletion logic in AdminForthRestAPI

ec17bd5

fix: delete unused console.log

a48d0a7

SerVitasik requested changes Mar 4, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

SerVitasik requested a review from Copilot March 4, 2026 13:06

Copilot started reviewing on behalf of SerVitasik March 4, 2026 13:06 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

adminforth/modules/restApi.ts Outdated Show resolved Hide resolved

adminforth/modules/configValidator.ts Outdated Show resolved Hide resolved

Pavlo Kulyk added 3 commits March 4, 2026 15:29

fix: delete unused arguments from function

c3a7a73

fix: resolve copilot comment

b4d8aa1

docs: add documentation for cascade deletion

604c0b3

-                    await this.adminforth.resource(childRes.resourceId).delete(childRecord[childPkFieldName]);
-                  }
-                } else if (onDeleteStrategy === 'setNull') {
-                  for (const childRecord of childRecords) {
-                    await this.adminforth.resource(childRes.resourceId).update(childRecord[childPkFieldName], {[foreignResourceColumn.name]: null});
+                    const { error: childDeleteError } = await this.adminforth.deleteResourceRecord({
+                      resource: childRes,
+                      record: childRecord,
+                      adminUser,
+                      recordId: childRecord[childPkFieldName],
+                      response,
+                      extra: { body, query, headers, cookies, requestUrl, response }
+                    });
+                    if (childDeleteError) {
+                      return { error: childDeleteError };
+                    }
+                  }
+                } else if (onDeleteStrategy === 'setNull') {
+                  for (const childRecord of childRecords) {
+                    const { error: childUpdateError } = await this.adminforth.updateResourceRecord({
+                      resource: childRes,
+                      record: childRecord,
+                      adminUser,
+                      recordId: childRecord[childPkFieldName],
+                      updatedRecord: { [foreignResourceColumn.name]: null },
+                      response,
+                      extra: { body, query, headers, cookies, requestUrl, response }
+                    });
+                    if (childUpdateError) {
+                      return { error: childUpdateError };
+                    }

Conversation

kulikp1 commented Feb 25, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 3, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants