Skip to content

Conversation

@GnaneshKunal
Copy link
Contributor

@GnaneshKunal GnaneshKunal commented Sep 5, 2024

Allow users to delete CA and client certificates.

@GnaneshKunal GnaneshKunal self-assigned this Sep 5, 2024
@GnaneshKunal GnaneshKunal force-pushed the feature/RI-6004-delete-certificates branch from fe9a88a to d5b2058 Compare September 5, 2024 17:16
@ArtemHoruzhenko
Copy link
Contributor

I don't see changes on backend. Was it implemented?
Could you have a look on FE part? @romansergeenkosofteq @kchepikava

@GnaneshKunal GnaneshKunal force-pushed the feature/RI-6004-delete-certificates branch 5 times, most recently from 2a98ff8 to 695ea84 Compare September 8, 2024 10:35
event: TelemetryEvent.CONFIG_DATABASES_CERTIFICATE_REMOVED,
eventData: {},
})
showPopover(id)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think showPopover we should call before deletion

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed this snippet as it's not required.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why its not required?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The popover component showPopover will show the popover. The telemetry event is sent after the certificate is removed, and we don't require showPopover because the popover is already available at that time. The Popover component's closePopover method closes the popover.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, you are right, thanks

rsergeenko
rsergeenko previously approved these changes Sep 9, 2024
ArtemHoruzhenko
ArtemHoruzhenko previously approved these changes Sep 9, 2024
Copy link
Contributor

@ArtemHoruzhenko ArtemHoruzhenko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

rsergeenko
rsergeenko previously approved these changes Sep 10, 2024
egor-zalenski
egor-zalenski previously approved these changes Sep 20, 2024
@GnaneshKunal GnaneshKunal force-pushed the feature/RI-6004-delete-certificates branch from 4a17391 to b09dd32 Compare September 25, 2024 08:07
@mariasergeenko mariasergeenko merged commit 5d4a069 into main Sep 25, 2024
@mariasergeenko mariasergeenko deleted the feature/RI-6004-delete-certificates branch September 25, 2024 08:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants