feat: delete connector instance and connector (#4642)#4654
Draft
MarineLeM wants to merge 3 commits into
Draft
Conversation
Signed-off-by: Marine LM <marine.lemezo@filigran.io>
Signed-off-by: Marine LM <marine.lemezo@filigran.io>
Signed-off-by: Marine LM <marine.lemezo@filigran.io>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## release/current #4654 +/- ##
=====================================================
+ Coverage 51.68% 51.74% +0.06%
- Complexity 3979 3995 +16
=====================================================
Files 942 943 +1
Lines 28363 28410 +47
Branches 2129 2135 +6
=====================================================
+ Hits 14659 14702 +43
Misses 12819 12819
- Partials 885 889 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR implements the ability to delete connector instances and their associated connectors (injectors, collectors, or executors). The deletion is orchestrated through XtmComposer, with safeguards to prevent deletion of instances that don't support it.
Key changes:
- Added a new
deletingrequested status type for connector instances - Implemented backend logic to delete connector instances and their associated connectors
- Added frontend UI to display deletion status and disable interactions during deletion
- Introduced
enableDeletionflag to control which instances can be deleted
Reviewed changes
Copilot reviewed 37 out of 38 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| openaev-model/src/main/java/io/openaev/database/model/ConnectorInstance.java | Added deleting status and enableDeletion flag to ConnectorInstance model |
| openaev-model/src/main/java/io/openaev/database/repository/ConnectorInstanceRepository.java | Updated query to exclude instances with deleting status |
| openaev-model/src/main/java/io/openaev/database/audit/BaseEvent.java | Refactored class hierarchy detection logic with new helper method |
| openaev-api/src/main/java/io/openaev/migration/V4_59__Add_deleting_requested_connector_status.java | Database migration to add deleting status type and enableDeletion column |
| openaev-api/src/main/java/io/openaev/api/xtm_composer/XtmComposerApi.java | Added DELETE endpoint for connector instance deletion |
| openaev-api/src/main/java/io/openaev/service/connectors/ConnectorOrchestrationService.java | Implemented deletion orchestration logic for instances and connectors |
| openaev-api/src/main/java/io/openaev/service/connector_instances/ConnectorInstanceService.java | Added validation for deletion requests and enableDeletion flag initialization |
| openaev-api/src/main/java/io/openaev/service/connectors/AbstractConnectorService.java | Made getConnectorIdFromInstance public and updated mapper signature |
| openaev-api/src/main/java/io/openaev/service/InjectorService.java | Added deleteById method |
| openaev-api/src/main/java/io/openaev/executors/ExecutorService.java | Added deleteById method |
| openaev-api/src/main/java/io/openaev/rest/collector/service/CollectorService.java | Added deleteById method |
| openaev-api/src/main/java/io/openaev/rest/connector_instance/ConnectorInstanceApi.java | Removed direct DELETE endpoint (deletion now via XtmComposer) |
| openaev-api/src/main/java/io/openaev/utils/mapper/*.java | Updated mappers to include ConnectorInstance in outputs |
| openaev-api/src/main/java/io/openaev/rest/*/form/*Output.java | Added connectorInstance field to output DTOs |
| openaev-front/src/utils/lang/*.json | Added translation for "Deletion is being processed" |
| openaev-front/src/components/common/ButtonPopover.tsx | Added style property to PopoverEntry interface |
| openaev-front/src/admin/components/integrations/common/ConnectorPopover.tsx | Implemented delete functionality with navigation after deletion |
| openaev-front/src/admin/components/integrations/common/ConnectorTitle.tsx | Added disabled state handling for deletion in progress |
| openaev-front/src/admin/components/integrations/common/ConnectorPage.tsx | Added logic to disable UI elements during deletion |
| openaev-front/src/admin/components/integrations/common/ConnectorList.tsx | Added disabled state for cards with deleting status |
| openaev-front/src/admin/components/integrations/common/ConnectorCard.tsx | Visual feedback for disabled state during deletion |
| openaev-front/src/admin/components/integrations/common/ConnectorContext.ts | Added instance field to ConnectorOutput interface |
| openaev-front/src/actions/connector_instances/connector-instance-actions.ts | Removed deleteConnectorInstance action |
| openaev-api/src/test/java/io/openaev/rest/ConnectorInstanceApiTest.java | Added test for deletion validation |
| openaev-api/src/test/java/io/openaev/api/xtm_composer/XtmComposerApiTest.java | Added tests for deletion endpoint |
| + "AND instance.catalogConnector.isManagerSupported = TRUE") | ||
| List<ConnectorInstance> findAllManagedByXtmComposerAndConfiguration(); | ||
| + "AND instance.catalogConnector.isManagerSupported = TRUE " | ||
| + "AND instance.requestedStatus != 'deleting'") |
There was a problem hiding this comment.
Extra space before '!=' operator. Should be single space for consistent formatting.
Suggested change
| + "AND instance.requestedStatus != 'deleting'") | |
| + "AND instance.requestedStatus != 'deleting'") |
Comment on lines
+58
to
+60
| connectorInstance -> { | ||
| connectorInstance.setEnableDeletion(true); | ||
| }); |
There was a problem hiding this comment.
Lambda body can be simplified to method reference: connectorInstance -> connectorInstance.setEnableDeletion(true) can use a more concise approach or avoid the intermediate variable in the forEach.
Suggested change
| connectorInstance -> { | |
| connectorInstance.setEnableDeletion(true); | |
| }); | |
| connectorInstance -> connectorInstance.setEnableDeletion(true)); |
Comment on lines
+42
to
+44
| dispatch(updateRequestedStatus(connectorInstanceId, { connector_instance_requested_status: 'deleting' })).then(() => { | ||
| const parentPath = location.pathname.split('/').slice(0, -1).join('/'); | ||
| navigate(parentPath); |
There was a problem hiding this comment.
The location.pathname is accessed without imports. Ensure that location is available in scope, typically from useLocation hook in react-router.
| name = "includeNext", | ||
| description = "Include collectors pending deployment", | ||
| required = false) | ||
| @Parameter(name = "includeNext", description = "Include collectors pending deployment") |
There was a problem hiding this comment.
The removal of required = false from the @parameter annotation is unnecessary since the corresponding @RequestParam already specifies required = false. This change doesn't add value and differs from the original explicit declaration.
Suggested change
| @Parameter(name = "includeNext", description = "Include collectors pending deployment") | |
| @Parameter( | |
| name = "includeNext", | |
| description = "Include collectors pending deployment", | |
| required = false) |
SamuelHassine
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Proposed changes
Related issues
Related issue