Add hosted databases management page with delete and reset password d…#1684
Merged
lyubov-voloshko merged 2 commits intomainfrom Mar 25, 2026
Merged
Add hosted databases management page with delete and reset password d…#1684lyubov-voloshko merged 2 commits intomainfrom
lyubov-voloshko merged 2 commits intomainfrom
Conversation
…ialogs Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
📝 WalkthroughWalkthroughThis pull request adds a complete "Hosted Databases" management feature to the frontend, including a new top-level route, a main component with list view, two dialog components for password resets and database deletion, corresponding service methods, model types, and navigation sidebar integration. Changes
Sequence DiagramssequenceDiagram
participant User
participant HostedDatabasesComponent
participant HostedDatabaseService
participant Backend API
User->>HostedDatabasesComponent: Navigate to hosted-databases
HostedDatabasesComponent->>HostedDatabaseService: listHostedDatabases(companyId)
HostedDatabaseService->>Backend API: GET /saas/hosted-database/{companyId}
Backend API-->>HostedDatabaseService: FoundHostedDatabase[]
HostedDatabaseService-->>HostedDatabasesComponent: Database list
HostedDatabasesComponent->>HostedDatabasesComponent: Update databases signal
HostedDatabasesComponent->>User: Display database cards
sequenceDiagram
participant User
participant HostedDatabasesComponent
participant DeleteDialogComponent
participant HostedDatabaseService
participant Backend API
participant NotificationsService
User->>HostedDatabasesComponent: Click delete button
HostedDatabasesComponent->>DeleteDialogComponent: Open dialog with db data
User->>DeleteDialogComponent: Confirm deletion
DeleteDialogComponent->>HostedDatabaseService: deleteHostedDatabase(companyId, id)
HostedDatabaseService->>Backend API: DELETE /saas/hosted-database/delete/{companyId}/{id}
Backend API-->>HostedDatabaseService: { success: boolean }
DeleteDialogComponent->>NotificationsService: Show success notification
DeleteDialogComponent->>HostedDatabasesComponent: Close with 'delete' action
HostedDatabasesComponent->>HostedDatabaseService: Reload databases
HostedDatabasesComponent->>User: Refresh list
sequenceDiagram
participant User
participant HostedDatabasesComponent
participant ResetPasswordDialogComponent
participant HostedDatabaseService
participant Backend API
User->>HostedDatabasesComponent: Click reset password button
HostedDatabasesComponent->>ResetPasswordDialogComponent: Open dialog with db data
User->>ResetPasswordDialogComponent: Confirm password reset
ResetPasswordDialogComponent->>HostedDatabaseService: resetHostedDatabasePassword(companyId, id)
HostedDatabaseService->>Backend API: POST /saas/hosted-database/reset-password/{companyId}/{id}
Backend API-->>HostedDatabaseService: CreatedHostedDatabase (with new credentials)
ResetPasswordDialogComponent->>ResetPasswordDialogComponent: Update result signal, switch to credentials phase
ResetPasswordDialogComponent->>User: Display new credentials
User->>ResetPasswordDialogComponent: Copy credentials to clipboard
ResetPasswordDialogComponent->>User: Show copy success snackbar
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds a new SaaS-only “Hosted Databases” management page in the frontend, enabling users to view hosted PostgreSQL instances and perform destructive actions (delete, reset password) via confirmation dialogs.
Changes:
- Extended
HostedDatabaseServiceand models to support listing, deletion, and password reset operations. - Added a new
/hosted-databasesrouted page with UI for listing databases and invoking delete/reset-password dialogs. - Updated the profile sidebar to include navigation to the Hosted Databases page (SaaS only).
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| frontend/src/app/services/hosted-database.service.ts | Adds API methods for list/delete/reset-password. |
| frontend/src/app/models/hosted-database.ts | Introduces FoundHostedDatabase model for list results. |
| frontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.ts | Extends activeTab union to include hosted-databases. |
| frontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.html | Adds SaaS-only sidebar link to Hosted Databases. |
| frontend/src/app/components/hosted-databases/hosted-databases.component.ts | New page logic: loads databases and opens dialogs. |
| frontend/src/app/components/hosted-databases/hosted-databases.component.html | New page UI: loading/empty/list states and action buttons. |
| frontend/src/app/components/hosted-databases/hosted-databases.component.css | Styling for the new management page. |
| frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts | New reset-password dialog logic + clipboard copy. |
| frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.html | Reset-password dialog confirm/result phases. |
| frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.css | Styling for reset-password dialog. |
| frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.ts | New delete dialog logic calling delete endpoint. |
| frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.html | Delete confirmation dialog UI. |
| frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.css | Minor dialog spacing tweak. |
| frontend/src/app/app-routing.module.ts | Registers new /hosted-databases route (AuthGuard-protected). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if (result?.success) { | ||
| posthog.capture('Hosted Databases: database deleted', { databaseName: this.data.databaseName }); | ||
| this._notifications.showSuccessSnackbar('Hosted database deleted successfully.'); | ||
| this._dialogRef.close('delete'); |
There was a problem hiding this comment.
HostedDatabaseDeleteDialogComponent only closes the dialog when result?.success is truthy; if the API returns { success: false } (a valid response per the method’s return type), the dialog stays open with no feedback to the user. Consider handling the non-success case explicitly (e.g., show an error snackbar/alert and keep the dialog actionable, or close with a failure result).
Suggested change
| this._dialogRef.close('delete'); | |
| this._dialogRef.close('delete'); | |
| } else { | |
| this._notifications.showErrorSnackbar('Failed to delete hosted database. Please try again.'); |
Comment on lines
+37
to
+47
| ngOnInit(): void { | ||
| this._company.getCurrentTabTitle().subscribe((tabTitle) => { | ||
| this._title.setTitle(`Hosted Databases | ${tabTitle || 'Rocketadmin'}`); | ||
| }); | ||
|
|
||
| this._userService.cast.subscribe((user) => { | ||
| if (user?.company?.id && user.company.id !== this._companyId) { | ||
| this._companyId = user.company.id; | ||
| this._loadDatabases(); | ||
| } | ||
| }); |
There was a problem hiding this comment.
This component subscribes to getCurrentTabTitle() and userService.cast but never unsubscribes. Both are backed by BehaviorSubjects and do not complete, so navigating away from the route will leak subscriptions and may trigger _loadDatabases() after destruction. Prefer takeUntilDestroyed(inject(DestroyRef)) (used elsewhere in the codebase) or convert these streams to signals via toSignal/effects.
Comment on lines
+56
to
+60
| dialogRef.afterClosed().subscribe(async (action) => { | ||
| if (action === 'delete') { | ||
| posthog.capture('Hosted Databases: database deleted successfully'); | ||
| await this._loadDatabases(); | ||
| } |
There was a problem hiding this comment.
Deleting a hosted database currently emits two PostHog events: one in HostedDatabaseDeleteDialogComponent on successful deletion and another here after the dialog closes. This likely double-counts deletes. Consider capturing the success event in only one place (typically the dialog that performs the action) or use distinct event names for “confirmed” vs “completed”.
| get credentialsText(): string { | ||
| const r = this.result(); | ||
| if (!r) return ''; | ||
| return `postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}`; |
There was a problem hiding this comment.
credentialsText interpolates username and password directly into a postgres:// URI. If either contains reserved URI characters (e.g. @, :, /, #), the copied connection string becomes invalid. Consider percent-encoding the username/password portion (or copying a structured set of fields instead of a URI).
Suggested change
| return `postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}`; | |
| const username = encodeURIComponent(r.username); | |
| const password = encodeURIComponent(r.password); | |
| return `postgres://${username}:${password}@${r.hostname}:${r.port}/${r.databaseName}`; |
Comment on lines
+47
to
+53
| matTooltip="Reset password" | ||
| (click)="resetPassword(db)"> | ||
| <mat-icon>lock_reset</mat-icon> | ||
| </button> | ||
| <button type="button" mat-icon-button | ||
| color="warn" | ||
| matTooltip="Delete database" |
There was a problem hiding this comment.
The icon-only action buttons rely on tooltips for meaning, but tooltips aren’t a substitute for accessible names. Add explicit aria-label (or visually hidden text) to the reset/delete buttons so screen readers can announce their purpose.
Suggested change
| matTooltip="Reset password" | |
| (click)="resetPassword(db)"> | |
| <mat-icon>lock_reset</mat-icon> | |
| </button> | |
| <button type="button" mat-icon-button | |
| color="warn" | |
| matTooltip="Delete database" | |
| matTooltip="Reset password" | |
| aria-label="Reset password" | |
| (click)="resetPassword(db)"> | |
| <mat-icon>lock_reset</mat-icon> | |
| </button> | |
| <button type="button" mat-icon-button | |
| color="warn" | |
| matTooltip="Delete database" | |
| aria-label="Delete database" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
frontend/src/app/models/hosted-database.ts (1)
12-20: Consider reducing duplication withOmitutility type.
FoundHostedDatabaseis identical toCreatedHostedDatabaseminus thepasswordfield. You could derive it to maintain DRY:♻️ Proposed refactor
-export interface FoundHostedDatabase { - id: string; - companyId: string; - databaseName: string; - hostname: string; - port: number; - username: string; - createdAt: string; -} +export type FoundHostedDatabase = Omit<CreatedHostedDatabase, 'password'>;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/models/hosted-database.ts` around lines 12 - 20, FoundHostedDatabase duplicates CreatedHostedDatabase except for the password field; change FoundHostedDatabase to be derived from CreatedHostedDatabase using TypeScript's Omit utility (i.e., replace the explicit interface with a type alias like Omit<CreatedHostedDatabase, 'password'>) so you only reference CreatedHostedDatabase and the 'password' key to keep the types DRY.frontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.html (1)
53-63: New code uses*ngIfinstead of@ifcontrol flow.Per coding guidelines, new code should use Angular's built-in control flow syntax (
@if) instead of structural directives (*ngIf). However, the existing template uses*ngIfthroughout, so migrating only this section would create inconsistency. Consider either keeping as-is for consistency, or migrating the entire template to built-in control flow.♻️ If migrating the new section only
- <a mat-list-item - *ngIf="isSaas" - routerLink="/hosted-databases" + `@if` (isSaas) { + <a mat-list-item + routerLink="/hosted-databases" [class.active-link]="activeTab() === 'hosted-databases'" ... <span *ngIf="!collapsed()" matListItemTitle>Hosted Databases</span> - </a> + </a> + }As per coding guidelines: "Use built-in control flow syntax (
@if,@for,@switch) instead of structural directives (*ngIf,*ngFor,*ngSwitch) in all new code".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.html` around lines 53 - 63, The new anchor uses the structural directive "*ngIf" which conflicts with the repo guideline requiring Angular built-in control flow ("@if") and also creates inconsistency with the rest of profile-sidebar.component.html; either revert this anchor to use the same pattern as the existing template (keep *ngIf) or migrate the entire template to built-in control flow. Locate the anchor element that checks isSaas and references collapsed() and activeTab(), then either change "*ngIf=\"isSaas\"" to the built-in "@if (isSaas) { ... }" block (and update any nested structural directives similarly) or revert this change so it matches existing "*ngIf" usage across the file to preserve consistency.frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.css (1)
1-3: Consider using dialog configuration instead of CSS override.Targeting
.mat-mdc-dialog-contentdirectly may break with future Angular Material updates. Consider using the dialog'spanelClassoption to apply custom styles, or adjust the dialog content padding through the component's own wrapper element.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.css` around lines 1 - 3, The CSS currently overrides the global Angular Material selector `.mat-mdc-dialog-content`; instead remove this global override and apply a custom panel class via the MatDialog open config (use the `panelClass` option when calling `MatDialog.open(...)`) or wrap the dialog template content in a local wrapper element (e.g., `<div class="hosted-db-delete-content">...</div>`) and move the styling there; update the dialog opener (where `MatDialog.open` is called) to include `panelClass: 'hosted-db-delete-panel'` and add corresponding styles in `hosted-database-delete-dialog.component.css` targeting `.hosted-db-delete-panel .hosted-db-delete-content` (or the wrapper) instead of `.mat-mdc-dialog-content`, and remove the original `.mat-mdc-dialog-content` rule.frontend/src/app/components/hosted-databases/hosted-databases.component.css (1)
12-16: Consider alternatives to::ng-deep(deprecated).While
::ng-deepstill works, it's deprecated in Angular. Consider using:host ::ng-deepfor clarity, or better yet, use component styles withViewEncapsulation.Nonefor the specific styles that need to pierce encapsulation, or move the styling to a global stylesheet.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/hosted-databases/hosted-databases.component.css` around lines 12 - 16, The CSS uses deprecated ::ng-deep in the selector "::ng-deep .profile-main > app-alert"; update the HostedDatabasesComponent to remove deprecated usage by either (a) moving this rule into a global stylesheet (e.g., styles.css) so no encapsulation piercing is needed, or (b) if it must stay with the component, set ViewEncapsulation.None on the HostedDatabasesComponent and remove ::ng-deep from the selector, or (c) as a minimal change change the selector to use ":host ::ng-deep .profile-main > app-alert" to narrow scope; pick one approach and adjust the selector and component decorator (Host component name: HostedDatabasesComponent) accordingly.frontend/src/app/components/hosted-databases/hosted-databases.component.ts (1)
56-61: Duplicate PostHog analytics event.The delete dialog already captures
'Hosted Databases: database deleted'on success. This parent component then captures'Hosted Databases: database deleted successfully'- a nearly identical event. This appears to be redundant tracking.Consider removing one of these events to avoid duplicate analytics:
dialogRef.afterClosed().subscribe(async (action) => { if (action === 'delete') { - posthog.capture('Hosted Databases: database deleted successfully'); await this._loadDatabases(); } });Alternatively, if both events serve different purposes, consider using distinct event names to clarify the tracking intent (e.g., dialog tracks the API action, parent tracks the UI flow completion).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/hosted-databases/hosted-databases.component.ts` around lines 56 - 61, Duplicate analytics: remove or differentiate the redundant posthog.capture call in the parent component that fires inside dialogRef.afterClosed().subscribe(async (action) => { ... }); specifically either delete the posthog.capture('Hosted Databases: database deleted successfully') call here (since the dialog already emits 'Hosted Databases: database deleted') or replace it with a distinct event name (e.g., 'Hosted Databases: delete flow completed') if you need parent-level tracking; keep the await this._loadDatabases() behavior unchanged so the UI refresh still occurs.frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts (1)
25-29: Consider usingcomputed()for derived state.The
credentialsTextgetter could be converted to acomputed()signal for consistency withphaseand alignment with the coding guidelines preference for signals.♻️ Proposed refactor
-get credentialsText(): string { - const r = this.result(); - if (!r) return ''; - return `postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}`; -} +protected credentialsText = computed(() => { + const r = this.result(); + if (!r) return ''; + return `postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}`; +});As per coding guidelines: "Use
computed()for derived state in components and services".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts` around lines 25 - 29, Replace the credentialsText getter with a computed() signal to follow the component's signal pattern: create a credentialsText = computed(() => { const r = this.result(); return r ? `postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}` : ''; }); remove the get credentialsText() accessor, ensure you import computed from `@angular/core` (consistent with the existing phase signal), and keep all usages of credentialsText as a property (not a function) in the template and code.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.ts`:
- Around line 23-37: The deleteDatabase() handler currently only acts when
result?.success is truthy, leaving users without feedback on failure; update
deleteDatabase() to handle falsy or error responses from
_hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id)
by catching errors and/or checking result?.success and calling
_notifications.showErrorSnackbar with a concise message (include result?.message
if available), ensure posthog.capture still fires for success only, and only
close the dialog via _dialogRef.close('delete') on success while always
resetting submitting.set(false) in the finally block.
In
`@frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts`:
- Around line 31-44: The resetPassword() method lacks user feedback when the API
returns a falsy response; update resetPassword() (and the branch around
_hostedDatabaseService.resetHostedDatabasePassword) to handle the else case:
when res is falsy, call the same UI notification mechanism used elsewhere (e.g.,
the delete dialog's error notifier) to show an error message to the user,
optionally capture a failure event with posthog.capture, and ensure this.result
is not set on failure; keep the existing submitting.set(false) in the finally
block.
In `@frontend/src/app/components/hosted-databases/hosted-databases.component.ts`:
- Around line 37-48: The component's subscriptions started in ngOnInit
(this._company.getCurrentTabTitle and this._userService.cast) need proper
cleanup to avoid leaks: implement OnDestroy, add a private Subject (e.g.,
destroy$), pipe both subscribes with takeUntil(this.destroy$) and replace direct
subscribe calls, and in ngOnDestroy call destroy$.next() and
destroy$.complete(); keep references to _companyId and _loadDatabases as-is so
logic remains unchanged.
---
Nitpick comments:
In
`@frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.css`:
- Around line 1-3: The CSS currently overrides the global Angular Material
selector `.mat-mdc-dialog-content`; instead remove this global override and
apply a custom panel class via the MatDialog open config (use the `panelClass`
option when calling `MatDialog.open(...)`) or wrap the dialog template content
in a local wrapper element (e.g., `<div
class="hosted-db-delete-content">...</div>`) and move the styling there; update
the dialog opener (where `MatDialog.open` is called) to include `panelClass:
'hosted-db-delete-panel'` and add corresponding styles in
`hosted-database-delete-dialog.component.css` targeting `.hosted-db-delete-panel
.hosted-db-delete-content` (or the wrapper) instead of
`.mat-mdc-dialog-content`, and remove the original `.mat-mdc-dialog-content`
rule.
In
`@frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts`:
- Around line 25-29: Replace the credentialsText getter with a computed() signal
to follow the component's signal pattern: create a credentialsText = computed(()
=> { const r = this.result(); return r ?
`postgres://${r.username}:${r.password}@${r.hostname}:${r.port}/${r.databaseName}`
: ''; }); remove the get credentialsText() accessor, ensure you import computed
from `@angular/core` (consistent with the existing phase signal), and keep all
usages of credentialsText as a property (not a function) in the template and
code.
In `@frontend/src/app/components/hosted-databases/hosted-databases.component.css`:
- Around line 12-16: The CSS uses deprecated ::ng-deep in the selector
"::ng-deep .profile-main > app-alert"; update the HostedDatabasesComponent to
remove deprecated usage by either (a) moving this rule into a global stylesheet
(e.g., styles.css) so no encapsulation piercing is needed, or (b) if it must
stay with the component, set ViewEncapsulation.None on the
HostedDatabasesComponent and remove ::ng-deep from the selector, or (c) as a
minimal change change the selector to use ":host ::ng-deep .profile-main >
app-alert" to narrow scope; pick one approach and adjust the selector and
component decorator (Host component name: HostedDatabasesComponent) accordingly.
In `@frontend/src/app/components/hosted-databases/hosted-databases.component.ts`:
- Around line 56-61: Duplicate analytics: remove or differentiate the redundant
posthog.capture call in the parent component that fires inside
dialogRef.afterClosed().subscribe(async (action) => { ... }); specifically
either delete the posthog.capture('Hosted Databases: database deleted
successfully') call here (since the dialog already emits 'Hosted Databases:
database deleted') or replace it with a distinct event name (e.g., 'Hosted
Databases: delete flow completed') if you need parent-level tracking; keep the
await this._loadDatabases() behavior unchanged so the UI refresh still occurs.
In
`@frontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.html`:
- Around line 53-63: The new anchor uses the structural directive "*ngIf" which
conflicts with the repo guideline requiring Angular built-in control flow
("@if") and also creates inconsistency with the rest of
profile-sidebar.component.html; either revert this anchor to use the same
pattern as the existing template (keep *ngIf) or migrate the entire template to
built-in control flow. Locate the anchor element that checks isSaas and
references collapsed() and activeTab(), then either change "*ngIf=\"isSaas\"" to
the built-in "@if (isSaas) { ... }" block (and update any nested structural
directives similarly) or revert this change so it matches existing "*ngIf" usage
across the file to preserve consistency.
In `@frontend/src/app/models/hosted-database.ts`:
- Around line 12-20: FoundHostedDatabase duplicates CreatedHostedDatabase except
for the password field; change FoundHostedDatabase to be derived from
CreatedHostedDatabase using TypeScript's Omit utility (i.e., replace the
explicit interface with a type alias like Omit<CreatedHostedDatabase,
'password'>) so you only reference CreatedHostedDatabase and the 'password' key
to keep the types DRY.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e51a70c2-ecb0-407f-9535-b81f0dabfb07
📒 Files selected for processing (14)
frontend/src/app/app-routing.module.tsfrontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.cssfrontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.htmlfrontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.tsfrontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.cssfrontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.htmlfrontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.tsfrontend/src/app/components/hosted-databases/hosted-databases.component.cssfrontend/src/app/components/hosted-databases/hosted-databases.component.htmlfrontend/src/app/components/hosted-databases/hosted-databases.component.tsfrontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.htmlfrontend/src/app/components/profile/profile-sidebar/profile-sidebar.component.tsfrontend/src/app/models/hosted-database.tsfrontend/src/app/services/hosted-database.service.ts
Comment on lines
+23
to
+37
| async deleteDatabase(): Promise<void> { | ||
| this.submitting.set(true); | ||
|
|
||
| try { | ||
| const result = await this._hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id); | ||
|
|
||
| if (result?.success) { | ||
| posthog.capture('Hosted Databases: database deleted', { databaseName: this.data.databaseName }); | ||
| this._notifications.showSuccessSnackbar('Hosted database deleted successfully.'); | ||
| this._dialogRef.close('delete'); | ||
| } | ||
| } finally { | ||
| this.submitting.set(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error feedback when deletion fails.
If result?.success is falsy (e.g., API returns { success: false } or an error occurs), the user receives no feedback and the dialog remains open with submitting reset to false. Consider showing an error notification.
🛡️ Proposed fix to add error handling
async deleteDatabase(): Promise<void> {
this.submitting.set(true);
try {
const result = await this._hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id);
if (result?.success) {
posthog.capture('Hosted Databases: database deleted', { databaseName: this.data.databaseName });
this._notifications.showSuccessSnackbar('Hosted database deleted successfully.');
this._dialogRef.close('delete');
+ } else {
+ this._notifications.showErrorSnackbar('Failed to delete database. Please try again.');
}
+ } catch {
+ this._notifications.showErrorSnackbar('Failed to delete database. Please try again.');
} finally {
this.submitting.set(false);
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| async deleteDatabase(): Promise<void> { | |
| this.submitting.set(true); | |
| try { | |
| const result = await this._hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id); | |
| if (result?.success) { | |
| posthog.capture('Hosted Databases: database deleted', { databaseName: this.data.databaseName }); | |
| this._notifications.showSuccessSnackbar('Hosted database deleted successfully.'); | |
| this._dialogRef.close('delete'); | |
| } | |
| } finally { | |
| this.submitting.set(false); | |
| } | |
| } | |
| async deleteDatabase(): Promise<void> { | |
| this.submitting.set(true); | |
| try { | |
| const result = await this._hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id); | |
| if (result?.success) { | |
| posthog.capture('Hosted Databases: database deleted', { databaseName: this.data.databaseName }); | |
| this._notifications.showSuccessSnackbar('Hosted database deleted successfully.'); | |
| this._dialogRef.close('delete'); | |
| } else { | |
| this._notifications.showErrorSnackbar('Failed to delete database. Please try again.'); | |
| } | |
| } catch { | |
| this._notifications.showErrorSnackbar('Failed to delete database. Please try again.'); | |
| } finally { | |
| this.submitting.set(false); | |
| } | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/hosted-databases/hosted-database-delete-dialog/hosted-database-delete-dialog.component.ts`
around lines 23 - 37, The deleteDatabase() handler currently only acts when
result?.success is truthy, leaving users without feedback on failure; update
deleteDatabase() to handle falsy or error responses from
_hostedDatabaseService.deleteHostedDatabase(this.data.companyId, this.data.id)
by catching errors and/or checking result?.success and calling
_notifications.showErrorSnackbar with a concise message (include result?.message
if available), ensure posthog.capture still fires for success only, and only
close the dialog via _dialogRef.close('delete') on success while always
resetting submitting.set(false) in the finally block.
Comment on lines
+31
to
+44
| async resetPassword(): Promise<void> { | ||
| this.submitting.set(true); | ||
|
|
||
| try { | ||
| const res = await this._hostedDatabaseService.resetHostedDatabasePassword(this.data.companyId, this.data.id); | ||
|
|
||
| if (res) { | ||
| posthog.capture('Hosted Databases: password reset', { databaseName: this.data.databaseName }); | ||
| this.result.set(res); | ||
| } | ||
| } finally { | ||
| this.submitting.set(false); | ||
| } | ||
| } |
There was a problem hiding this comment.
Missing error feedback when password reset fails.
Similar to the delete dialog, if res is falsy (API failure or error), the user receives no feedback. Consider adding error notification.
🛡️ Proposed fix to add error handling
async resetPassword(): Promise<void> {
this.submitting.set(true);
try {
const res = await this._hostedDatabaseService.resetHostedDatabasePassword(this.data.companyId, this.data.id);
if (res) {
posthog.capture('Hosted Databases: password reset', { databaseName: this.data.databaseName });
this.result.set(res);
+ } else {
+ this._notifications.showErrorSnackbar('Failed to reset password. Please try again.');
}
+ } catch {
+ this._notifications.showErrorSnackbar('Failed to reset password. Please try again.');
} finally {
this.submitting.set(false);
}
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@frontend/src/app/components/hosted-databases/hosted-database-reset-password-dialog/hosted-database-reset-password-dialog.component.ts`
around lines 31 - 44, The resetPassword() method lacks user feedback when the
API returns a falsy response; update resetPassword() (and the branch around
_hostedDatabaseService.resetHostedDatabasePassword) to handle the else case:
when res is falsy, call the same UI notification mechanism used elsewhere (e.g.,
the delete dialog's error notifier) to show an error message to the user,
optionally capture a failure event with posthog.capture, and ensure this.result
is not set on failure; keep the existing submitting.set(false) in the finally
block.
Comment on lines
+37
to
+48
| ngOnInit(): void { | ||
| this._company.getCurrentTabTitle().subscribe((tabTitle) => { | ||
| this._title.setTitle(`Hosted Databases | ${tabTitle || 'Rocketadmin'}`); | ||
| }); | ||
|
|
||
| this._userService.cast.subscribe((user) => { | ||
| if (user?.company?.id && user.company.id !== this._companyId) { | ||
| this._companyId = user.company.id; | ||
| this._loadDatabases(); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing subscription cleanup - potential memory leak.
The subscriptions in ngOnInit are not cleaned up when the component is destroyed. This could cause memory leaks if the component is navigated away from while subscriptions are still active.
🔧 Proposed fix using takeUntil pattern
+import { Subject } from 'rxjs';
+import { takeUntil } from 'rxjs/operators';
export class HostedDatabasesComponent implements OnInit {
private _hostedDatabaseService = inject(HostedDatabaseService);
private _userService = inject(UserService);
private _company = inject(CompanyService);
private _dialog = inject(MatDialog);
private _title = inject(Title);
+ private _destroy$ = new Subject<void>();
protected databases = signal<FoundHostedDatabase[] | null>(null);
protected loading = signal(true);
private _companyId: string | null = null;
ngOnInit(): void {
- this._company.getCurrentTabTitle().subscribe((tabTitle) => {
+ this._company.getCurrentTabTitle().pipe(takeUntil(this._destroy$)).subscribe((tabTitle) => {
this._title.setTitle(`Hosted Databases | ${tabTitle || 'Rocketadmin'}`);
});
- this._userService.cast.subscribe((user) => {
+ this._userService.cast.pipe(takeUntil(this._destroy$)).subscribe((user) => {
if (user?.company?.id && user.company.id !== this._companyId) {
this._companyId = user.company.id;
this._loadDatabases();
}
});
}
+
+ ngOnDestroy(): void {
+ this._destroy$.next();
+ this._destroy$.complete();
+ }Also add OnDestroy to the implements clause:
-export class HostedDatabasesComponent implements OnInit {
+export class HostedDatabasesComponent implements OnInit, OnDestroy {As per coding guidelines: "Use takeUntil pattern for memory leak prevention with proper subscription management".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ngOnInit(): void { | |
| this._company.getCurrentTabTitle().subscribe((tabTitle) => { | |
| this._title.setTitle(`Hosted Databases | ${tabTitle || 'Rocketadmin'}`); | |
| }); | |
| this._userService.cast.subscribe((user) => { | |
| if (user?.company?.id && user.company.id !== this._companyId) { | |
| this._companyId = user.company.id; | |
| this._loadDatabases(); | |
| } | |
| }); | |
| } | |
| import { Subject } from 'rxjs'; | |
| import { takeUntil } from 'rxjs/operators'; | |
| export class HostedDatabasesComponent implements OnInit, OnDestroy { | |
| private _hostedDatabaseService = inject(HostedDatabaseService); | |
| private _userService = inject(UserService); | |
| private _company = inject(CompanyService); | |
| private _dialog = inject(MatDialog); | |
| private _title = inject(Title); | |
| private _destroy$ = new Subject<void>(); | |
| protected databases = signal<FoundHostedDatabase[] | null>(null); | |
| protected loading = signal(true); | |
| private _companyId: string | null = null; | |
| ngOnInit(): void { | |
| this._company.getCurrentTabTitle().pipe(takeUntil(this._destroy$)).subscribe((tabTitle) => { | |
| this._title.setTitle(`Hosted Databases | ${tabTitle || 'Rocketadmin'}`); | |
| }); | |
| this._userService.cast.pipe(takeUntil(this._destroy$)).subscribe((user) => { | |
| if (user?.company?.id && user.company.id !== this._companyId) { | |
| this._companyId = user.company.id; | |
| this._loadDatabases(); | |
| } | |
| }); | |
| } | |
| ngOnDestroy(): void { | |
| this._destroy$.next(); | |
| this._destroy$.complete(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/app/components/hosted-databases/hosted-databases.component.ts`
around lines 37 - 48, The component's subscriptions started in ngOnInit
(this._company.getCurrentTabTitle and this._userService.cast) need proper
cleanup to avoid leaks: implement OnDestroy, add a private Subject (e.g.,
destroy$), pipe both subscribes with takeUntil(this.destroy$) and replace direct
subscribe calls, and in ngOnDestroy call destroy$.next() and
destroy$.complete(); keep references to _companyId and _loadDatabases as-is so
logic remains unchanged.
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.
…ialogs
Summary by CodeRabbit
Release Notes