Entity deletion: Remove child entities #2486
Conversation
* v2-master: (77 commits) Only show org/space name in users pills when needed Update v2 info on readme, fix typo in roadmap doc Fix merge issue - missing comma Don't hide users tabs in production - they are now implemented Fix for panic when res is not set in error logging Fix issue where space count is wrong after deleting a space Update the connected user roles section of store on roles change - includes renaming of role change actions from 'permissions' to 'roles' Fix list state (deleting/etc) - This fixes all the following list states - Endpoints warning state - Space routes undbind state - app github tabs commit table highlighting - By.. - Both `rowsState` and `getRowState` are plumbed in to data source config - If we have a `rowsState` use it before falling back to `getRowState` Fix exception when a space is added to an empty org Fix exception thrown when only assign an org user role Fix pagination of space level routes and service instance tables - Actions were set to flattenPagination - Ensure consistent page size between the two tables as well as other si tables (5 for table only, otherwise 9) Fix missing end of line of sed Fix tests Fix old (maybe angular 6?) issue where deployment info commit was truncated for giturl type Update expression to support Mac V2 Beta 1 Change Log Fix imports Minor tweaks Disable meta-card actions on delete, fix app service binding delete entity config Fixed bug with not storing deploy type. Remove subtype. ...
|
Hey KlapTrap! Thanks for submitting this pull request! I'm here to inform the recipients of the pull request that you and the commit authors have already signed the CLA. |
Codecov Report
@@ Coverage Diff @@
## v2-master #2486 +/- ##
============================================
Coverage ? 70.44%
============================================
Files ? 594
Lines ? 25111
Branches ? 5668
============================================
Hits ? 17690
Misses ? 7421
Partials ? 0 |
* v2-master: Fix backend error logging Fix error shown when cancelling out of a freshly loaded create space stepper Fix location of Dockerfile Fixed c+p orgRoles.hasOwnProperty, tweaked return behaviour Fix unit tests Address comments adding back change Fix unit tests Fix newline in helm chart minor tweaks Put back in cookie domain and allow use of - to ignore it Disable removal of `org user` role if user has others Add warning if the configured domain does not match the URL Add new line at end of file Add support for sharing tokens for metrics endpoints
There was a problem hiding this comment.
Think we need to limit to entities we delete, particularly for one's we know aren't 'owned' by the entity we're deleting. For instance when an app is deleted we also delete it's space, all things associated in that space including routes, domains, quotas etc. For space it's not too bad, however we do delete services and service plans (if the space contains service instances). This causes us to refetch a lot of data. I think we should have something similar to the entity relations work, delete actions should specify which parent-child relations should be traversed for deletion. This would also avoids any issue with someone adding extra fields to the entity schemas.
We're also quite brutal in that if we delete an entity, all pagination lists associated with that type are cleared. For instance deleting an app means we have to refetch the entire app wall and orgs for the filter. There's a lot more requests because of it.
Picked up a couple of bugs relating to the two issues above
- After deleting an app go to the organisation that it was in and the space list will be empty
- After deleting an app go to create app stepper and the domains drop down is empty
|
|
||
| private deleteSuccessApiActionGenerators = { | ||
| application: (guid: string, endpointGuid: string) => { | ||
| return new APISuccessOrFailedAction(DELETE_SUCCESS, new DeleteApplication(guid, endpointGuid) as ICFAction); |
There was a problem hiding this comment.
Update - Action is duplicated when deleting an app, but needed when deleting apps via other entities.
Not sure if this is needed. APISuccessOrFailedAction is called post validation and this code path isn't hit.
| @@ -146,6 +150,14 @@ export class APIEffect { | |||
| return []; | |||
| } | |||
|
|
|||
| if (requestType === 'delete') { | |||
There was a problem hiding this comment.
Need to handle the case where delete fails, all 'deleting' entities should be reset
| if (this.deleteSuccessApiActionGenerators[key]) { | ||
| keyActions.push(this.deleteSuccessApiActionGenerators[key](action.guid, action.endpointGuid)); | ||
| } | ||
| keyActions.push(new ClearPaginationOfType(key)); |
There was a problem hiding this comment.
This is quite brutal, for instance if we delete an app we have to refetch all apps from all cf's on the app wall.
* v2-master: (30 commits) Fixed issue where we failed to store response from 1 or more endpoints - Ensure that we process the repsonse from all endpoints that haven't failed - Caused by minor bug mixed with not checking if the error object had a false error property - Renamed `errors` to `errorsCheck` to avoid future confusion Revert triggers Revert sync-official release Remove legacy release pipeline Update dev pipeline Remove BOWER Fix repo string Make pipeline manually invoked Remove unused artifacts Delete unused variable Remove commented tasks Tweaks Update CAP pipeline Short status update Ensure remove user confirmation modal contains correct role prefix - permission name can now be null at org/space levels - ensure we treat role pill label and confirmation label the same (handling nulls) Fix heading level on some release titles Remove duplicate SSO line Fix formatting Remove doc issue Final tweak ...
Fixes #1626