feat: implement cascading deletion for related records in delete #488
feat: implement cascading deletion for related records in delete #488
Conversation
adminforth/modules/restApi.ts
Outdated
| return { error }; | ||
| } | ||
|
|
||
| await handleCascadeOnDelete.call(this, resource, body['primaryKey']); |
There was a problem hiding this comment.
if (resource.foreignResource.onDelete) {
if (resource.foreignResource.onDelete === "cascade") {
await adminforth.resource(resource.resourceId).delete(childId);
} else if (resource.foreignResource.onDelete === "setNull") {
await adminforth.resource(resource.resourceId).update(childId, {[foreignKeyColumn.name]: null,});
}
}
adminforth/modules/restApi.ts
Outdated
| return { error: `Resource '${resource.resourceId}' does not allow delete action` }; | ||
| } | ||
|
|
||
| for (const childRes of this.adminforth.config.resources) { |
There was a problem hiding this comment.
No, where is the "if" which checks foreignResourcu.onDelete?
Also you have O(n)^3 complexity
This is bad practice to have loop inside loop inside loop
adminforth/modules/restApi.ts
Outdated
| const childResources = this.adminforth.config.resources.filter(r => r.columns.some(c => c.foreignResource?.resourceId === resource.resourceId)); | ||
| if (childResources.length){ | ||
| for (const childRes of childResources) { | ||
| const foreignKeyColumn = childRes.columns.find(c => c.foreignResource?.resourceId === resource.resourceId); |
There was a problem hiding this comment.
Why foreignKey? And also add check if we have onDelete property or not
Maybe better i think in the end move it to separate function
adminforth/modules/restApi.ts
Outdated
| await this.adminforth.resource(childRes.resourceId).update(childRecord.id, {[foreignResourceColumn.name]: null}); | ||
| } | ||
| } else { | ||
| return { error: `Wrong onDelete strategy: ${onDeleteStrategy}` }; |
There was a problem hiding this comment.
Remove it, we have to throw error earlier in configValidator.
If user enters for example "cascdrvfv" we should throw error on the start of app and suggest him use cascade or setNull
| } | ||
| } | ||
|
|
||
| if (col.foreignResource){ |
There was a problem hiding this comment.
You already have this check below
you can add your validation below
Also you can merge your two checks in one
like
if i have onDelete and onDelete is not cascade or setNull
throw error
| if (col.foreignResource){ | ||
| if (col.foreignResource.onDelete){ | ||
| if (col.foreignResource.onDelete !== 'cascade' && col.foreignResource.onDelete !== 'setNull'){ | ||
| errors.push (`Wrong delete strategy you can use 'onDelete' or 'cascade'`); |
There was a problem hiding this comment.
Mistake in message you have setNull and cascade strategies but your error message tells that you have onDelete or cascade
2 participants
…oint